[Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Serge Petrenko]

---
https://github.com/tarantool/tarantool/tree/sp/feedback_daemon_synchro

 src/box/lua/feedback_daemon.lua       | 12 ++++++++++++
 test/box-tap/feedback_daemon.test.lua | 21 ++++++++++++++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
index 179e8fb47..aa296c445 100644
--- a/src/box/lua/feedback_daemon.lua
+++ b/src/box/lua/feedback_daemon.lua
@@ -150,6 +150,7 @@ local function fill_in_schema_stats_impl(schema)
         vinyl     = 0,
         temporary = 0,
         ['local'] = 0,
+        sync      = 0,
     }
 
     local indices = {
@@ -188,6 +189,9 @@ local function fill_in_schema_stats_impl(schema)
         if space.is_local then
             spaces['local'] = spaces['local'] + 1
         end
+        if space.is_sync then
+            spaces.sync = spaces.sync + 1
+        end
         fill_in_indices_stats(space, indices)
 
         fiber.yield()
@@ -222,10 +226,18 @@ local function fill_in_features(feedback)
     fill_in_schema_stats(feedback.features)
 end
 
+local function fill_in_options(feedback)
+    local options = {}
+    options.election_mode = box.cfg.election_mode
+    options.synchro_quorum = box.cfg.replication_synchro_quorum
+    feedback.options = options
+end
+
 local function fill_in_feedback(feedback)
     fill_in_base_info(feedback)
     fill_in_platform_info(feedback)
     fill_in_features(feedback)
+    fill_in_options(feedback)
 
     return feedback
 end
diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
index a04995d8a..c1ae5f81b 100755
--- a/test/box-tap/feedback_daemon.test.lua
+++ b/test/box-tap/feedback_daemon.test.lua
@@ -67,7 +67,7 @@ if not ok then
     os.exit(0)
 end
 
-test:plan(19)
+test:plan(21)
 
 local function check(message)
     while feedback_count < 1 do
@@ -152,6 +152,7 @@ local expected = {
     vinyl_spaces = 0,
     temporary_spaces = 0,
     local_spaces = 0,
+    sync_spaces = 0,
     tree_indices = 0,
     rtree_indices = 0,
     hash_indices = 0,
@@ -168,6 +169,7 @@ box.schema.create_space('features_vinyl', {engine = 'vinyl'})
 box.schema.create_space('features_memtx_empty')
 box.schema.create_space('features_memtx',
         {engine = 'memtx', is_local = true, temporary = true})
+box.schema.create_space('features_sync', {is_sync=true})
 box.space.features_vinyl:create_index('vinyl_pk', {type = 'tree'})
 box.space.features_memtx:create_index('memtx_pk', {type = 'tree'})
 box.space.features_memtx:create_index('memtx_hash', {type = 'hash'})
@@ -195,11 +197,12 @@ box.space.features_memtx:create_index('functional_multikey',
 actual = daemon.generate_feedback()
 local schema_stats = actual.features.schema
 test:test('features.schema', function(t)
-    t:plan(12)
-    t:is(schema_stats.memtx_spaces, 2, 'memtx engine usage gathered')
+    t:plan(13)
+    t:is(schema_stats.memtx_spaces, 3, 'memtx engine usage gathered')
     t:is(schema_stats.vinyl_spaces, 1, 'vinyl engine usage gathered')
     t:is(schema_stats.temporary_spaces, 1, 'temporary space usage gathered')
     t:is(schema_stats.local_spaces, 1, 'local space usage gathered')
+    t:is(schema_stats.sync_spaces, 1, 'sync space  usage gathered')
     t:is(schema_stats.tree_indices, 6, 'tree index gathered')
     t:is(schema_stats.hash_indices, 1, 'hash index gathered')
     t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
@@ -216,9 +219,21 @@ actual = daemon.generate_feedback()
 test:is(actual.features.schema.hash_indices, 2,
         'internal cache invalidates when schema changes')
 
+--
+-- collect synchronous replication and election box.cfg options.
+--
+em = box.cfg.election_mode
+quorum = box.cfg.replication_synchro_quorum
+box.cfg{election_mode='candidate', replication_synchro_quorum=2}
+actual = daemon.generate_feedback()
+test:is(actual.options.election_mode, 'candidate', 'election_mode option reported')
+test:is(actual.options.synchro_quorum, 2, 'replication_synchro_quorum option reported')
+box.cfg{election_mode=em, replication_synchro_quorum=quorum}
+
 box.space.features_vinyl:drop()
 box.space.features_memtx_empty:drop()
 box.space.features_memtx:drop()
+box.space.features_sync:drop()
 
 test:check()
 os.exit(0)
-- 
2.24.3 (Apple Git-128)

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Serge Petrenko]

13.10.2020 16:34, Serge Petrenko пишет:
> ---
> https://github.com/tarantool/tarantool/tree/sp/feedback_daemon_synchro

A small update: add txm usage reporting & rename synchro_quorum to
replication_synchro_quorum so that each option in daemon's report is
named the same way it's done in box.cfg.

The only value to report for txm is box.cfg.memtx_use_mvcc_engine

The diff's below:

diff --git a/src/box/lua/feedback_daemon.lua 
b/src/box/lua/feedback_daemon.lua
index aa296c445..026eef641 100644
--- a/src/box/lua/feedback_daemon.lua
+++ b/src/box/lua/feedback_daemon.lua
@@ -229,7 +229,8 @@ end
  local function fill_in_options(feedback)
      local options = {}
      options.election_mode = box.cfg.election_mode
-    options.synchro_quorum = box.cfg.replication_synchro_quorum
+    options.replication_synchro_quorum = box.cfg.replication_synchro_quorum
+    options.memtx_use_mvcc_engine = box.cfg.memtx_use_mvcc_engine
      feedback.options = options
  end

diff --git a/test/box-tap/feedback_daemon.test.lua 
b/test/box-tap/feedback_daemon.test.lua
index c1ae5f81b..abf7b9576 100755
--- a/test/box-tap/feedback_daemon.test.lua
+++ b/test/box-tap/feedback_daemon.test.lua
@@ -67,7 +67,7 @@ if not ok then
      os.exit(0)
  end

-test:plan(21)
+test:plan(22)

  local function check(message)
      while feedback_count < 1 do
@@ -220,14 +220,16 @@ test:is(actual.features.schema.hash_indices, 2,
          'internal cache invalidates when schema changes')

  --
--- collect synchronous replication and election box.cfg options.
+-- collect box.cfg options: election mode, synchronous replication and tx
+-- manager.
  --
  em = box.cfg.election_mode
  quorum = box.cfg.replication_synchro_quorum
  box.cfg{election_mode='candidate', replication_synchro_quorum=2}
  actual = daemon.generate_feedback()
  test:is(actual.options.election_mode, 'candidate', 'election_mode 
option reported')
-test:is(actual.options.synchro_quorum, 2, 'replication_synchro_quorum 
option reported')
+test:is(actual.options.replication_synchro_quorum, 2, 
'replication_synchro_quorum option reported')
+test:is(actual.options.memtx_use_mvcc_engine, false, 
'memtx_use_mvcc_engine option reported')
  box.cfg{election_mode=em, replication_synchro_quorum=quorum}

  box.space.features_vinyl:drop()

>
>   src/box/lua/feedback_daemon.lua       | 12 ++++++++++++
>   test/box-tap/feedback_daemon.test.lua | 21 ++++++++++++++++++---
>   2 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
> index 179e8fb47..aa296c445 100644
> --- a/src/box/lua/feedback_daemon.lua
> +++ b/src/box/lua/feedback_daemon.lua
> @@ -150,6 +150,7 @@ local function fill_in_schema_stats_impl(schema)
>           vinyl     = 0,
>           temporary = 0,
>           ['local'] = 0,
> +        sync      = 0,
>       }
>   
>       local indices = {
> @@ -188,6 +189,9 @@ local function fill_in_schema_stats_impl(schema)
>           if space.is_local then
>               spaces['local'] = spaces['local'] + 1
>           end
> +        if space.is_sync then
> +            spaces.sync = spaces.sync + 1
> +        end
>           fill_in_indices_stats(space, indices)
>   
>           fiber.yield()
> @@ -222,10 +226,18 @@ local function fill_in_features(feedback)
>       fill_in_schema_stats(feedback.features)
>   end
>   
> +local function fill_in_options(feedback)
> +    local options = {}
> +    options.election_mode = box.cfg.election_mode
> +    options.synchro_quorum = box.cfg.replication_synchro_quorum
> +    feedback.options = options
> +end
> +
>   local function fill_in_feedback(feedback)
>       fill_in_base_info(feedback)
>       fill_in_platform_info(feedback)
>       fill_in_features(feedback)
> +    fill_in_options(feedback)
>   
>       return feedback
>   end
> diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
> index a04995d8a..c1ae5f81b 100755
> --- a/test/box-tap/feedback_daemon.test.lua
> +++ b/test/box-tap/feedback_daemon.test.lua
> @@ -67,7 +67,7 @@ if not ok then
>       os.exit(0)
>   end
>   
> -test:plan(19)
> +test:plan(21)
>   
>   local function check(message)
>       while feedback_count < 1 do
> @@ -152,6 +152,7 @@ local expected = {
>       vinyl_spaces = 0,
>       temporary_spaces = 0,
>       local_spaces = 0,
> +    sync_spaces = 0,
>       tree_indices = 0,
>       rtree_indices = 0,
>       hash_indices = 0,
> @@ -168,6 +169,7 @@ box.schema.create_space('features_vinyl', {engine = 'vinyl'})
>   box.schema.create_space('features_memtx_empty')
>   box.schema.create_space('features_memtx',
>           {engine = 'memtx', is_local = true, temporary = true})
> +box.schema.create_space('features_sync', {is_sync=true})
>   box.space.features_vinyl:create_index('vinyl_pk', {type = 'tree'})
>   box.space.features_memtx:create_index('memtx_pk', {type = 'tree'})
>   box.space.features_memtx:create_index('memtx_hash', {type = 'hash'})
> @@ -195,11 +197,12 @@ box.space.features_memtx:create_index('functional_multikey',
>   actual = daemon.generate_feedback()
>   local schema_stats = actual.features.schema
>   test:test('features.schema', function(t)
> -    t:plan(12)
> -    t:is(schema_stats.memtx_spaces, 2, 'memtx engine usage gathered')
> +    t:plan(13)
> +    t:is(schema_stats.memtx_spaces, 3, 'memtx engine usage gathered')
>       t:is(schema_stats.vinyl_spaces, 1, 'vinyl engine usage gathered')
>       t:is(schema_stats.temporary_spaces, 1, 'temporary space usage gathered')
>       t:is(schema_stats.local_spaces, 1, 'local space usage gathered')
> +    t:is(schema_stats.sync_spaces, 1, 'sync space  usage gathered')
>       t:is(schema_stats.tree_indices, 6, 'tree index gathered')
>       t:is(schema_stats.hash_indices, 1, 'hash index gathered')
>       t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
> @@ -216,9 +219,21 @@ actual = daemon.generate_feedback()
>   test:is(actual.features.schema.hash_indices, 2,
>           'internal cache invalidates when schema changes')
>   
> +--
> +-- collect synchronous replication and election box.cfg options.
> +--
> +em = box.cfg.election_mode
> +quorum = box.cfg.replication_synchro_quorum
> +box.cfg{election_mode='candidate', replication_synchro_quorum=2}
> +actual = daemon.generate_feedback()
> +test:is(actual.options.election_mode, 'candidate', 'election_mode option reported')
> +test:is(actual.options.synchro_quorum, 2, 'replication_synchro_quorum option reported')
> +box.cfg{election_mode=em, replication_synchro_quorum=quorum}
> +
>   box.space.features_vinyl:drop()
>   box.space.features_memtx_empty:drop()
>   box.space.features_memtx:drop()
> +box.space.features_sync:drop()
>   
>   test:check()
>   os.exit(0)

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Cyrill Gorcunov]

On Tue, Oct 13, 2020 at 04:34:18PM +0300, Serge Petrenko wrote:
>  
> +local function fill_in_options(feedback)
> +    local options = {}
> +    options.election_mode = box.cfg.election_mode
> +    options.synchro_quorum = box.cfg.replication_synchro_quorum
> +    feedback.options = options
> +end

Maybe be we could just get rid of local variable here?

local function fill_in_options(feedback)
    feedback.options = {
        ['election_mode']   = box.cfg.election_mode,
        ['synchro_quorum']  = box.cfg.replication_synchro_quorum,
    }
end

Not insisting at all just a thought.

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Serge Petrenko]

13.10.2020 17:01, Cyrill Gorcunov пишет:
> On Tue, Oct 13, 2020 at 04:34:18PM +0300, Serge Petrenko wrote:
>>   
>> +local function fill_in_options(feedback)
>> +    local options = {}
>> +    options.election_mode = box.cfg.election_mode
>> +    options.synchro_quorum = box.cfg.replication_synchro_quorum
>> +    feedback.options = options
>> +end
> Maybe be we could just get rid of local variable here?
Yes, sure. I'll do it together with other fixes, if any.
>
> local function fill_in_options(feedback)
>      feedback.options = {
>          ['election_mode']   = box.cfg.election_mode,
>          ['synchro_quorum']  = box.cfg.replication_synchro_quorum,
>      }
> end
>
> Not insisting at all just a thought.

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Serge Petrenko]

13.10.2020 16:34, Serge Petrenko пишет:
> ---
> https://github.com/tarantool/tarantool/tree/sp/feedback_daemon_synchro
>
>   src/box/lua/feedback_daemon.lua       | 12 ++++++++++++
>   test/box-tap/feedback_daemon.test.lua | 21 ++++++++++++++++++---
>   2 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
> index 179e8fb47..aa296c445 100644
> --- a/src/box/lua/feedback_daemon.lua
> +++ b/src/box/lua/feedback_daemon.lua
> @@ -150,6 +150,7 @@ local function fill_in_schema_stats_impl(schema)
>           vinyl     = 0,
>           temporary = 0,
>           ['local'] = 0,
> +        sync      = 0,
>       }

New patch on top:

 From 9de473601d9643fbb1b36cc56d80a09dc3989106 Mon Sep 17 00:00:00 2001
From: Serge Petrenko <sergepetrenko@tarantool.org>
Date: Tue, 13 Oct 2020 18:48:48 +0300
Subject: [PATCH] feedback_daemon: report box.on_reload_configuration usage

---
  src/box/lua/feedback_daemon.lua       | 2 ++
  test/box-tap/feedback_daemon.test.lua | 4 +++-
  2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/box/lua/feedback_daemon.lua 
b/src/box/lua/feedback_daemon.lua
index 026eef641..4779448ca 100644
--- a/src/box/lua/feedback_daemon.lua
+++ b/src/box/lua/feedback_daemon.lua
@@ -224,6 +224,8 @@ end
  local function fill_in_features(feedback)
      feedback.features = {}
      fill_in_schema_stats(feedback.features)
+    feedback.features.on_reload_configuration_used =
+        type(box.on_reload_configuration) == 'function'
  end

  local function fill_in_options(feedback)
diff --git a/test/box-tap/feedback_daemon.test.lua 
b/test/box-tap/feedback_daemon.test.lua
index abf7b9576..4a1f636b7 100755
--- a/test/box-tap/feedback_daemon.test.lua
+++ b/test/box-tap/feedback_daemon.test.lua
@@ -67,7 +67,7 @@ if not ok then
      os.exit(0)
  end

-test:plan(22)
+test:plan(23)

  local function check(message)
      while feedback_count < 1 do
@@ -226,7 +226,9 @@ test:is(actual.features.schema.hash_indices, 2,
  em = box.cfg.election_mode
  quorum = box.cfg.replication_synchro_quorum
  box.cfg{election_mode='candidate', replication_synchro_quorum=2}
+box.on_reload_configuration = function() end
  actual = daemon.generate_feedback()
+test:is(actual.features.on_reload_configuration_used, true, 
'on_reload_configuration reported')
  test:is(actual.options.election_mode, 'candidate', 'election_mode 
option reported')
  test:is(actual.options.replication_synchro_quorum, 2, 
'replication_synchro_quorum option reported')
  test:is(actual.options.memtx_use_mvcc_engine, false, 
'memtx_use_mvcc_engine option reported')
-- 
2.24.3 (Apple Git-128)

>   
>       local indices = {
> @@ -188,6 +189,9 @@ local function fill_in_schema_stats_impl(schema)
>           if space.is_local then
>               spaces['local'] = spaces['local'] + 1
>           end
> +        if space.is_sync then
> +            spaces.sync = spaces.sync + 1
> +        end
>           fill_in_indices_stats(space, indices)
>   
>           fiber.yield()
> @@ -222,10 +226,18 @@ local function fill_in_features(feedback)
>       fill_in_schema_stats(feedback.features)
>   end
>   
> +local function fill_in_options(feedback)
> +    local options = {}
> +    options.election_mode = box.cfg.election_mode
> +    options.synchro_quorum = box.cfg.replication_synchro_quorum
> +    feedback.options = options
> +end
> +
>   local function fill_in_feedback(feedback)
>       fill_in_base_info(feedback)
>       fill_in_platform_info(feedback)
>       fill_in_features(feedback)
> +    fill_in_options(feedback)
>   
>       return feedback
>   end
> diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
> index a04995d8a..c1ae5f81b 100755
> --- a/test/box-tap/feedback_daemon.test.lua
> +++ b/test/box-tap/feedback_daemon.test.lua
> @@ -67,7 +67,7 @@ if not ok then
>       os.exit(0)
>   end
>   
> -test:plan(19)
> +test:plan(21)
>   
>   local function check(message)
>       while feedback_count < 1 do
> @@ -152,6 +152,7 @@ local expected = {
>       vinyl_spaces = 0,
>       temporary_spaces = 0,
>       local_spaces = 0,
> +    sync_spaces = 0,
>       tree_indices = 0,
>       rtree_indices = 0,
>       hash_indices = 0,
> @@ -168,6 +169,7 @@ box.schema.create_space('features_vinyl', {engine = 'vinyl'})
>   box.schema.create_space('features_memtx_empty')
>   box.schema.create_space('features_memtx',
>           {engine = 'memtx', is_local = true, temporary = true})
> +box.schema.create_space('features_sync', {is_sync=true})
>   box.space.features_vinyl:create_index('vinyl_pk', {type = 'tree'})
>   box.space.features_memtx:create_index('memtx_pk', {type = 'tree'})
>   box.space.features_memtx:create_index('memtx_hash', {type = 'hash'})
> @@ -195,11 +197,12 @@ box.space.features_memtx:create_index('functional_multikey',
>   actual = daemon.generate_feedback()
>   local schema_stats = actual.features.schema
>   test:test('features.schema', function(t)
> -    t:plan(12)
> -    t:is(schema_stats.memtx_spaces, 2, 'memtx engine usage gathered')
> +    t:plan(13)
> +    t:is(schema_stats.memtx_spaces, 3, 'memtx engine usage gathered')
>       t:is(schema_stats.vinyl_spaces, 1, 'vinyl engine usage gathered')
>       t:is(schema_stats.temporary_spaces, 1, 'temporary space usage gathered')
>       t:is(schema_stats.local_spaces, 1, 'local space usage gathered')
> +    t:is(schema_stats.sync_spaces, 1, 'sync space  usage gathered')
>       t:is(schema_stats.tree_indices, 6, 'tree index gathered')
>       t:is(schema_stats.hash_indices, 1, 'hash index gathered')
>       t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
> @@ -216,9 +219,21 @@ actual = daemon.generate_feedback()
>   test:is(actual.features.schema.hash_indices, 2,
>           'internal cache invalidates when schema changes')
>   
> +--
> +-- collect synchronous replication and election box.cfg options.
> +--
> +em = box.cfg.election_mode
> +quorum = box.cfg.replication_synchro_quorum
> +box.cfg{election_mode='candidate', replication_synchro_quorum=2}
> +actual = daemon.generate_feedback()
> +test:is(actual.options.election_mode, 'candidate', 'election_mode option reported')
> +test:is(actual.options.synchro_quorum, 2, 'replication_synchro_quorum option reported')
> +box.cfg{election_mode=em, replication_synchro_quorum=quorum}
> +
>   box.space.features_vinyl:drop()
>   box.space.features_memtx_empty:drop()
>   box.space.features_memtx:drop()
> +box.space.features_sync:drop()
>   
>   test:check()
>   os.exit(0)

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

See 2 comments below.

> diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
> index a04995d8a..c1ae5f81b 100755
> --- a/test/box-tap/feedback_daemon.test.lua
> +++ b/test/box-tap/feedback_daemon.test.lua
> @@ -195,11 +197,12 @@ box.space.features_memtx:create_index('functional_multikey',
>  actual = daemon.generate_feedback()
>  local schema_stats = actual.features.schema
>  test:test('features.schema', function(t)
> -    t:plan(12)
> -    t:is(schema_stats.memtx_spaces, 2, 'memtx engine usage gathered')
> +    t:plan(13)
> +    t:is(schema_stats.memtx_spaces, 3, 'memtx engine usage gathered')
>      t:is(schema_stats.vinyl_spaces, 1, 'vinyl engine usage gathered')
>      t:is(schema_stats.temporary_spaces, 1, 'temporary space usage gathered')
>      t:is(schema_stats.local_spaces, 1, 'local space usage gathered')
> +    t:is(schema_stats.sync_spaces, 1, 'sync space  usage gathered')

1. Double whitespace in 'space  usage'. At least the macbooks >= 2016
are stylish.

>      t:is(schema_stats.tree_indices, 6, 'tree index gathered')
>      t:is(schema_stats.hash_indices, 1, 'hash index gathered')
>      t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
2. I also see you pushed a new commit on top of this branch about reporting
box.on_reload_configuration. But why? It is not even documented anywhere.
We could just drop this function.

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Serge Petrenko]

15.10.2020 02:50, Vladislav Shpilevoy пишет:
> Hi! Thanks for the patch!
>
> See 2 comments below.
Thanks for the review!
>
>> diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
>> index a04995d8a..c1ae5f81b 100755
>> --- a/test/box-tap/feedback_daemon.test.lua
>> +++ b/test/box-tap/feedback_daemon.test.lua
>> @@ -195,11 +197,12 @@ box.space.features_memtx:create_index('functional_multikey',
>>   actual = daemon.generate_feedback()
>>   local schema_stats = actual.features.schema
>>   test:test('features.schema', function(t)
>> -    t:plan(12)
>> -    t:is(schema_stats.memtx_spaces, 2, 'memtx engine usage gathered')
>> +    t:plan(13)
>> +    t:is(schema_stats.memtx_spaces, 3, 'memtx engine usage gathered')
>>       t:is(schema_stats.vinyl_spaces, 1, 'vinyl engine usage gathered')
>>       t:is(schema_stats.temporary_spaces, 1, 'temporary space usage gathered')
>>       t:is(schema_stats.local_spaces, 1, 'local space usage gathered')
>> +    t:is(schema_stats.sync_spaces, 1, 'sync space  usage gathered')
> 1. Double whitespace in 'space  usage'. At least the macbooks >= 2016
> are stylish.
Yep, they look nice. Fixed, thanks!
>>       t:is(schema_stats.tree_indices, 6, 'tree index gathered')
>>       t:is(schema_stats.hash_indices, 1, 'hash index gathered')
>>       t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
> 2. I also see you pushed a new commit on top of this branch about reporting
> box.on_reload_configuration. But why? It is not even documented anywhere.
> We could just drop this function.
It was Mons's request. He wants to see whether anyone uses this feature 
or not.
If you're against this, let's discuss.

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Vladislav Shpilevoy]

>>>       t:is(schema_stats.tree_indices, 6, 'tree index gathered')
>>>       t:is(schema_stats.hash_indices, 1, 'hash index gathered')
>>>       t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
>> 2. I also see you pushed a new commit on top of this branch about reporting
>> box.on_reload_configuration. But why? It is not even documented anywhere.
>> We could just drop this function.
> It was Mons's request. He wants to see whether anyone uses this feature or not.
> If you're against this, let's discuss.

Ok, leave it then, I don't mind. Still looks strange though. We really
could just drop it. It is not a feature. It is an undocumented piece of
code. Same as box.internal.

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Илья Конюхов]

Hi,

Please increment "feedback_version" each time you patch feedback daemon report. You can do it here - https://github.com/tarantool/tarantool/blob/cf6a9c64dea6a4385f3fd4719e44d0d9dc9ffee9/src/box/lua/feedback_daemon.lua#L359

> 13 окт. 2020 г., в 16:34, Serge Petrenko <sergepetrenko@tarantool.org> написал(а):
> 
> ---
> https://github.com/tarantool/tarantool/tree/sp/feedback_daemon_synchro
> 
> src/box/lua/feedback_daemon.lua       | 12 ++++++++++++
> test/box-tap/feedback_daemon.test.lua | 21 ++++++++++++++++++---
> 2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
> index 179e8fb47..aa296c445 100644
> --- a/src/box/lua/feedback_daemon.lua
> +++ b/src/box/lua/feedback_daemon.lua
> @@ -150,6 +150,7 @@ local function fill_in_schema_stats_impl(schema)
>         vinyl     = 0,
>         temporary = 0,
>         ['local'] = 0,
> +        sync      = 0,
>     }
> 
>     local indices = {
> @@ -188,6 +189,9 @@ local function fill_in_schema_stats_impl(schema)
>         if space.is_local then
>             spaces['local'] = spaces['local'] + 1
>         end
> +        if space.is_sync then
> +            spaces.sync = spaces.sync + 1
> +        end
>         fill_in_indices_stats(space, indices)
> 
>         fiber.yield()
> @@ -222,10 +226,18 @@ local function fill_in_features(feedback)
>     fill_in_schema_stats(feedback.features)
> end
> 
> +local function fill_in_options(feedback)
> +    local options = {}
> +    options.election_mode = box.cfg.election_mode
> +    options.synchro_quorum = box.cfg.replication_synchro_quorum
> +    feedback.options = options
> +end
> +
> local function fill_in_feedback(feedback)
>     fill_in_base_info(feedback)
>     fill_in_platform_info(feedback)
>     fill_in_features(feedback)
> +    fill_in_options(feedback)
> 
>     return feedback
> end
> diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
> index a04995d8a..c1ae5f81b 100755
> --- a/test/box-tap/feedback_daemon.test.lua
> +++ b/test/box-tap/feedback_daemon.test.lua
> @@ -67,7 +67,7 @@ if not ok then
>     os.exit(0)
> end
> 
> -test:plan(19)
> +test:plan(21)
> 
> local function check(message)
>     while feedback_count < 1 do
> @@ -152,6 +152,7 @@ local expected = {
>     vinyl_spaces = 0,
>     temporary_spaces = 0,
>     local_spaces = 0,
> +    sync_spaces = 0,
>     tree_indices = 0,
>     rtree_indices = 0,
>     hash_indices = 0,
> @@ -168,6 +169,7 @@ box.schema.create_space('features_vinyl', {engine = 'vinyl'})
> box.schema.create_space('features_memtx_empty')
> box.schema.create_space('features_memtx',
>         {engine = 'memtx', is_local = true, temporary = true})
> +box.schema.create_space('features_sync', {is_sync=true})
> box.space.features_vinyl:create_index('vinyl_pk', {type = 'tree'})
> box.space.features_memtx:create_index('memtx_pk', {type = 'tree'})
> box.space.features_memtx:create_index('memtx_hash', {type = 'hash'})
> @@ -195,11 +197,12 @@ box.space.features_memtx:create_index('functional_multikey',
> actual = daemon.generate_feedback()
> local schema_stats = actual.features.schema
> test:test('features.schema', function(t)
> -    t:plan(12)
> -    t:is(schema_stats.memtx_spaces, 2, 'memtx engine usage gathered')
> +    t:plan(13)
> +    t:is(schema_stats.memtx_spaces, 3, 'memtx engine usage gathered')
>     t:is(schema_stats.vinyl_spaces, 1, 'vinyl engine usage gathered')
>     t:is(schema_stats.temporary_spaces, 1, 'temporary space usage gathered')
>     t:is(schema_stats.local_spaces, 1, 'local space usage gathered')
> +    t:is(schema_stats.sync_spaces, 1, 'sync space  usage gathered')
>     t:is(schema_stats.tree_indices, 6, 'tree index gathered')
>     t:is(schema_stats.hash_indices, 1, 'hash index gathered')
>     t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
> @@ -216,9 +219,21 @@ actual = daemon.generate_feedback()
> test:is(actual.features.schema.hash_indices, 2,
>         'internal cache invalidates when schema changes')
> 
> +--
> +-- collect synchronous replication and election box.cfg options.
> +--
> +em = box.cfg.election_mode
> +quorum = box.cfg.replication_synchro_quorum
> +box.cfg{election_mode='candidate', replication_synchro_quorum=2}
> +actual = daemon.generate_feedback()
> +test:is(actual.options.election_mode, 'candidate', 'election_mode option reported')
> +test:is(actual.options.synchro_quorum, 2, 'replication_synchro_quorum option reported')
> +box.cfg{election_mode=em, replication_synchro_quorum=quorum}
> +
> box.space.features_vinyl:drop()
> box.space.features_memtx_empty:drop()
> box.space.features_memtx:drop()
> +box.space.features_sync:drop()
> 
> test:check()
> os.exit(0)
> -- 
> 2.24.3 (Apple Git-128)
> 

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Serge Petrenko]

20.10.2020 20:09, Илья Конюхов пишет:
> Hi,
>
> Please increment "feedback_version" each time you patch feedback daemon report. You can do it here - https://github.com/tarantool/tarantool/blob/cf6a9c64dea6a4385f3fd4719e44d0d9dc9ffee9/src/box/lua/feedback_daemon.lua#L359
Hi! Thanks for your answer, done.
>> 13 окт. 2020 г., в 16:34, Serge Petrenko <sergepetrenko@tarantool.org> написал(а):
>>
>> ---
>> https://github.com/tarantool/tarantool/tree/sp/feedback_daemon_synchro
>>
>> src/box/lua/feedback_daemon.lua       | 12 ++++++++++++
>> test/box-tap/feedback_daemon.test.lua | 21 ++++++++++++++++++---
>> 2 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
>> index 179e8fb47..aa296c445 100644
>> --- a/src/box/lua/feedback_daemon.lua
>> +++ b/src/box/lua/feedback_daemon.lua
>> @@ -150,6 +150,7 @@ local function fill_in_schema_stats_impl(schema)
>>          vinyl     = 0,
>>          temporary = 0,
>>          ['local'] = 0,
>> +        sync      = 0,
>>      }
>>
>>      local indices = {
>> @@ -188,6 +189,9 @@ local function fill_in_schema_stats_impl(schema)
>>          if space.is_local then
>>              spaces['local'] = spaces['local'] + 1
>>          end
>> +        if space.is_sync then
>> +            spaces.sync = spaces.sync + 1
>> +        end
>>          fill_in_indices_stats(space, indices)
>>
>>          fiber.yield()
>> @@ -222,10 +226,18 @@ local function fill_in_features(feedback)
>>      fill_in_schema_stats(feedback.features)
>> end
>>
>> +local function fill_in_options(feedback)
>> +    local options = {}
>> +    options.election_mode = box.cfg.election_mode
>> +    options.synchro_quorum = box.cfg.replication_synchro_quorum
>> +    feedback.options = options
>> +end
>> +
>> local function fill_in_feedback(feedback)
>>      fill_in_base_info(feedback)
>>      fill_in_platform_info(feedback)
>>      fill_in_features(feedback)
>> +    fill_in_options(feedback)
>>
>>      return feedback
>> end
>> diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
>> index a04995d8a..c1ae5f81b 100755
>> --- a/test/box-tap/feedback_daemon.test.lua
>> +++ b/test/box-tap/feedback_daemon.test.lua
>> @@ -67,7 +67,7 @@ if not ok then
>>      os.exit(0)
>> end
>>
>> -test:plan(19)
>> +test:plan(21)
>>
>> local function check(message)
>>      while feedback_count < 1 do
>> @@ -152,6 +152,7 @@ local expected = {
>>      vinyl_spaces = 0,
>>      temporary_spaces = 0,
>>      local_spaces = 0,
>> +    sync_spaces = 0,
>>      tree_indices = 0,
>>      rtree_indices = 0,
>>      hash_indices = 0,
>> @@ -168,6 +169,7 @@ box.schema.create_space('features_vinyl', {engine = 'vinyl'})
>> box.schema.create_space('features_memtx_empty')
>> box.schema.create_space('features_memtx',
>>          {engine = 'memtx', is_local = true, temporary = true})
>> +box.schema.create_space('features_sync', {is_sync=true})
>> box.space.features_vinyl:create_index('vinyl_pk', {type = 'tree'})
>> box.space.features_memtx:create_index('memtx_pk', {type = 'tree'})
>> box.space.features_memtx:create_index('memtx_hash', {type = 'hash'})
>> @@ -195,11 +197,12 @@ box.space.features_memtx:create_index('functional_multikey',
>> actual = daemon.generate_feedback()
>> local schema_stats = actual.features.schema
>> test:test('features.schema', function(t)
>> -    t:plan(12)
>> -    t:is(schema_stats.memtx_spaces, 2, 'memtx engine usage gathered')
>> +    t:plan(13)
>> +    t:is(schema_stats.memtx_spaces, 3, 'memtx engine usage gathered')
>>      t:is(schema_stats.vinyl_spaces, 1, 'vinyl engine usage gathered')
>>      t:is(schema_stats.temporary_spaces, 1, 'temporary space usage gathered')
>>      t:is(schema_stats.local_spaces, 1, 'local space usage gathered')
>> +    t:is(schema_stats.sync_spaces, 1, 'sync space  usage gathered')
>>      t:is(schema_stats.tree_indices, 6, 'tree index gathered')
>>      t:is(schema_stats.hash_indices, 1, 'hash index gathered')
>>      t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
>> @@ -216,9 +219,21 @@ actual = daemon.generate_feedback()
>> test:is(actual.features.schema.hash_indices, 2,
>>          'internal cache invalidates when schema changes')
>>
>> +--
>> +-- collect synchronous replication and election box.cfg options.
>> +--
>> +em = box.cfg.election_mode
>> +quorum = box.cfg.replication_synchro_quorum
>> +box.cfg{election_mode='candidate', replication_synchro_quorum=2}
>> +actual = daemon.generate_feedback()
>> +test:is(actual.options.election_mode, 'candidate', 'election_mode option reported')
>> +test:is(actual.options.synchro_quorum, 2, 'replication_synchro_quorum option reported')
>> +box.cfg{election_mode=em, replication_synchro_quorum=quorum}
>> +
>> box.space.features_vinyl:drop()
>> box.space.features_memtx_empty:drop()
>> box.space.features_memtx:drop()
>> +box.space.features_sync:drop()
>>
>> test:check()
>> os.exit(0)
>> -- 
>> 2.24.3 (Apple Git-128)
>>
-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] feedback_daemon: add synchro replication usage reporting

[Alexander V. Tikhonov]

Hi Serge, I've checked results in gitlab [1], there no any new
degradation found. Patch LGTM.

[1] - https://gitlab.com/tarantool/tarantool/-/pipelines/205365404

On Tue, Oct 20, 2020 at 10:16:15PM +0300, Serge Petrenko wrote:
> 
> 20.10.2020 20:09, Илья Конюхов пишет:
> > Hi,
> > 
> > Please increment "feedback_version" each time you patch feedback daemon report. You can do it here - https://github.com/tarantool/tarantool/blob/cf6a9c64dea6a4385f3fd4719e44d0d9dc9ffee9/src/box/lua/feedback_daemon.lua#L359
> Hi! Thanks for your answer, done.
> > > 13 окт. 2020 г., в 16:34, Serge Petrenko <sergepetrenko@tarantool.org> написал(а):
> > > 
> > > ---
> > > https://github.com/tarantool/tarantool/tree/sp/feedback_daemon_synchro
> > > 
> > > src/box/lua/feedback_daemon.lua       | 12 ++++++++++++
> > > test/box-tap/feedback_daemon.test.lua | 21 ++++++++++++++++++---
> > > 2 files changed, 30 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
> > > index 179e8fb47..aa296c445 100644
> > > --- a/src/box/lua/feedback_daemon.lua
> > > +++ b/src/box/lua/feedback_daemon.lua
> > > @@ -150,6 +150,7 @@ local function fill_in_schema_stats_impl(schema)
> > >          vinyl     = 0,
> > >          temporary = 0,
> > >          ['local'] = 0,
> > > +        sync      = 0,
> > >      }
> > > 
> > >      local indices = {
> > > @@ -188,6 +189,9 @@ local function fill_in_schema_stats_impl(schema)
> > >          if space.is_local then
> > >              spaces['local'] = spaces['local'] + 1
> > >          end
> > > +        if space.is_sync then
> > > +            spaces.sync = spaces.sync + 1
> > > +        end
> > >          fill_in_indices_stats(space, indices)
> > > 
> > >          fiber.yield()
> > > @@ -222,10 +226,18 @@ local function fill_in_features(feedback)
> > >      fill_in_schema_stats(feedback.features)
> > > end
> > > 
> > > +local function fill_in_options(feedback)
> > > +    local options = {}
> > > +    options.election_mode = box.cfg.election_mode
> > > +    options.synchro_quorum = box.cfg.replication_synchro_quorum
> > > +    feedback.options = options
> > > +end
> > > +
> > > local function fill_in_feedback(feedback)
> > >      fill_in_base_info(feedback)
> > >      fill_in_platform_info(feedback)
> > >      fill_in_features(feedback)
> > > +    fill_in_options(feedback)
> > > 
> > >      return feedback
> > > end
> > > diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
> > > index a04995d8a..c1ae5f81b 100755
> > > --- a/test/box-tap/feedback_daemon.test.lua
> > > +++ b/test/box-tap/feedback_daemon.test.lua
> > > @@ -67,7 +67,7 @@ if not ok then
> > >      os.exit(0)
> > > end
> > > 
> > > -test:plan(19)
> > > +test:plan(21)
> > > 
> > > local function check(message)
> > >      while feedback_count < 1 do
> > > @@ -152,6 +152,7 @@ local expected = {
> > >      vinyl_spaces = 0,
> > >      temporary_spaces = 0,
> > >      local_spaces = 0,
> > > +    sync_spaces = 0,
> > >      tree_indices = 0,
> > >      rtree_indices = 0,
> > >      hash_indices = 0,
> > > @@ -168,6 +169,7 @@ box.schema.create_space('features_vinyl', {engine = 'vinyl'})
> > > box.schema.create_space('features_memtx_empty')
> > > box.schema.create_space('features_memtx',
> > >          {engine = 'memtx', is_local = true, temporary = true})
> > > +box.schema.create_space('features_sync', {is_sync=true})
> > > box.space.features_vinyl:create_index('vinyl_pk', {type = 'tree'})
> > > box.space.features_memtx:create_index('memtx_pk', {type = 'tree'})
> > > box.space.features_memtx:create_index('memtx_hash', {type = 'hash'})
> > > @@ -195,11 +197,12 @@ box.space.features_memtx:create_index('functional_multikey',
> > > actual = daemon.generate_feedback()
> > > local schema_stats = actual.features.schema
> > > test:test('features.schema', function(t)
> > > -    t:plan(12)
> > > -    t:is(schema_stats.memtx_spaces, 2, 'memtx engine usage gathered')
> > > +    t:plan(13)
> > > +    t:is(schema_stats.memtx_spaces, 3, 'memtx engine usage gathered')
> > >      t:is(schema_stats.vinyl_spaces, 1, 'vinyl engine usage gathered')
> > >      t:is(schema_stats.temporary_spaces, 1, 'temporary space usage gathered')
> > >      t:is(schema_stats.local_spaces, 1, 'local space usage gathered')
> > > +    t:is(schema_stats.sync_spaces, 1, 'sync space  usage gathered')
> > >      t:is(schema_stats.tree_indices, 6, 'tree index gathered')
> > >      t:is(schema_stats.hash_indices, 1, 'hash index gathered')
> > >      t:is(schema_stats.rtree_indices, 1, 'rtree index gathered')
> > > @@ -216,9 +219,21 @@ actual = daemon.generate_feedback()
> > > test:is(actual.features.schema.hash_indices, 2,
> > >          'internal cache invalidates when schema changes')
> > > 
> > > +--
> > > +-- collect synchronous replication and election box.cfg options.
> > > +--
> > > +em = box.cfg.election_mode
> > > +quorum = box.cfg.replication_synchro_quorum
> > > +box.cfg{election_mode='candidate', replication_synchro_quorum=2}
> > > +actual = daemon.generate_feedback()
> > > +test:is(actual.options.election_mode, 'candidate', 'election_mode option reported')
> > > +test:is(actual.options.synchro_quorum, 2, 'replication_synchro_quorum option reported')
> > > +box.cfg{election_mode=em, replication_synchro_quorum=quorum}
> > > +
> > > box.space.features_vinyl:drop()
> > > box.space.features_memtx_empty:drop()
> > > box.space.features_memtx:drop()
> > > +box.space.features_sync:drop()
> > > 
> > > test:check()
> > > os.exit(0)
> > > -- 
> > > 2.24.3 (Apple Git-128)
> > > 
> -- 
> Serge Petrenko
>