Re: [Tarantool-patches] [PATCH 1/3 v2] replication: test clear_synchro_queue() function

[Sergey Bronnikov <sergeyb@tarantool.org>]

Hi, Serge!

thanks for review!

On 13.11.2020 16:12, Serge Petrenko wrote:
>
> 12.11.2020 12:10, sergeyb@tarantool.org пишет:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> Part of #5055
>> Part of #4849
>
> Hi! Thanks for the patch!
>
> Please see 2 comments below.
>
>> ---
>>   test/replication/qsync_basic.result   | 136 ++++++++++++++++++++++++++
>>   test/replication/qsync_basic.test.lua |  50 ++++++++++
>>   2 files changed, 186 insertions(+)
>>
>> diff --git a/test/replication/qsync_basic.result 
>> b/test/replication/qsync_basic.result
>> index bd3c3cce1..e848e305a 100644
>> --- a/test/replication/qsync_basic.result
>> +++ b/test/replication/qsync_basic.result
>> @@ -32,6 +32,13 @@ s2.is_sync
>>    | - false
>>    | ...
>>   +--
>> +-- gh-4849: clear synchro queue with unconfigured box
>> +--
>
> What do you mean by `unconfigured box`?
>
> box.cfg is called when the default instance (master.lua) is started, 
> before
> the first test line.

Missed it.

>
> If you really  want to test `box.ctl.clear_synchro_queue` before box.cfg,
> you need to do it in `app-tap/cfg.test.lua`. It has all the tests for 
> `box.ctl` options.

Removed testcase from qsync_basic.test.lua and added it to 
app-tap/cfg.test.lua:

--- a/test/replication/qsync_basic.test.lua
+++ b/test/replication/qsync_basic.test.lua
@@ -13,11 +13,6 @@ s1:select{}
  s2 = box.schema.create_space('test2')
  s2.is_sync

---
--- gh-4849: clear synchro queue with unconfigured box
---
-box.ctl.clear_synchro_queue()
-
  -- Net.box takes sync into account.
  box.schema.user.grant('guest', 'super')
  netbox = require('net.box')

--- a/test/app-tap/cfg.test.lua
+++ b/test/app-tap/cfg.test.lua
@@ -3,7 +3,13 @@ local fiber = require('fiber')
  local tap = require('tap')
  local test = tap.test("cfg")

-test:plan(11)
+test:plan(12)
+
+--
+-- gh-4849: clear synchro queue is null with unconfigured box
+--
+local ok, err = pcall(box.ctl.clear_synchro_queue(), nil)
+test:isnil(ok, 'execute clear_synchro_queue with unconfigured box')

  --
  -- gh-4282: box.cfg should not allow nor just ignore nil UUID.

>> +box.ctl.clear_synchro_queue()
>> + | ---
>> + | ...
>> +
>>   -- Net.box takes sync into account.
>>   box.schema.user.grant('guest', 'super')
>>    | ---
>> @@ -637,6 +644,135 @@ box.space.sync:count()
>>    | - 0
>>    | ...
>>   +--
>> +-- gh-4849: clear synchro queue on a master
>> +--
>> +test_run:switch('default')
>> + | ---
>> + | - true
>> + | ...
>> +box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout 
>> = 1000}
>> + | ---
>> + | ...
>> +ok, err = nil
>> + | ---
>> + | ...
>> +f = fiber.create(function()                            \
>> +    ok, err = pcall(box.space.sync.insert, box.space.sync, 
>> {10})        \
>> +end)
>> + | ---
>> + | ...
>> +f:status()
>> + | ---
>> + | - suspended
>> + | ...
>> +test_run:switch('replica')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:wait_cond(function() return box.space.sync:get{10} ~= nil end)
>> + | ---
>> + | - true
>> + | ...
>> +test_run:switch('default')
>> + | ---
>> + | - true
>> + | ...
>> +box.cfg{replication_synchro_timeout = 0.1}
>> + | ---
>> + | ...
>> +box.ctl.clear_synchro_queue()
>> + | ---
>> + | ...
>> +test_run:switch('replica')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:wait_cond(function() return box.space.sync:get{10} == nil end)
>> + | ---
>> + | - true
>> + | ...
>> +test_run:switch('default')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:wait_cond(function() return f:status() == 'dead' end)
>> + | ---
>> + | - true
>> + | ...
>> +ok, err
>> + | ---
>> + | - false
>> + | - Quorum collection for a synchronous transaction is timed out
>> + | ...
>> +test_run:wait_cond(function() return box.space.sync:get{10} == nil end)
>> + | ---
>> + | - true
>> + | ...
>> +
>> +--
>> +-- gh-4849: clear synchro queue on a replica, make sure no crashes
>> +--
>> +test_run:switch('default')
>> + | ---
>> + | - true
>> + | ...
>> +box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout 
>> = 1000}
>> + | ---
>> + | ...
>> +ok, err = nil
>> + | ---
>> + | ...
>> +f = fiber.create(function() \
>> +    ok, err = pcall(box.space.sync.insert, box.space.sync, 
>> {9})                 \
>> +end)
>> + | ---
>> + | ...
>> +f.status()
>> + | ---
>> + | - running
>> + | ...
>
>
> typo: should be `f:status()`

Updated:

--- a/test/replication/qsync_basic.test.lua
+++ b/test/replication/qsync_basic.test.lua
@@ -284,7 +284,7 @@ ok, err = nil
  f = fiber.create(function() \
      ok, err = pcall(box.space.sync.insert, box.space.sync, 
{9})                 \
  end)
-f.status()
+f:status()
  test_run:wait_cond(function() return box.space.sync:get{9} ~= nil end)
  test_run:switch('replica')
  box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout=0.01}

>
>
>> +test_run:wait_cond(function() return box.space.sync:get{9} ~= nil end)
>> + | ---
>> + | - true
>> + | ...
>> +test_run:switch('replica')
>> + | ---
>> + | - true
>> + | ...
>> +box.cfg{replication_synchro_quorum = 3, 
>> replication_synchro_timeout=0.01}
>> + | ---
>> + | ...
>> +box.ctl.clear_synchro_queue()
>> + | ---
>> + | ...
>> +test_run:wait_cond(function() return box.space.sync:get{9} == nil end)
>> + | ---
>> + | - true
>> + | ...
>> +test_run:switch('default')
>> + | ---
>> + | - true
>> + | ...
>> +box.cfg{replication_synchro_timeout=0.01}
>> + | ---
>> + | ...
>> +test_run:wait_cond(function() return f:status() == 'dead' end)
>> + | ---
>> + | - true
>> + | ...
>> +ok, err
>> + | ---
>> + | - false
>> + | - Quorum collection for a synchronous transaction is timed out
>> + | ...
>> +test_run:wait_cond(function() return box.space.sync:get{9} == nil end)
>> + | ---
>> + | - true
>> + | ...
>> +
>> +-- Note: cluster may be in a broken state here due to nature of 
>> previous test.
>> +
>>   -- Cleanup.
>>   test_run:cmd('switch default')
>>    | ---
>> diff --git a/test/replication/qsync_basic.test.lua 
>> b/test/replication/qsync_basic.test.lua
>> index 94235547d..523dfa779 100644
>> --- a/test/replication/qsync_basic.test.lua
>> +++ b/test/replication/qsync_basic.test.lua
>> @@ -13,6 +13,11 @@ s1:select{}
>>   s2 = box.schema.create_space('test2')
>>   s2.is_sync
>>   +--
>> +-- gh-4849: clear synchro queue with unconfigured box
>> +--
>> +box.ctl.clear_synchro_queue()
>> +
>>   -- Net.box takes sync into account.
>>   box.schema.user.grant('guest', 'super')
>>   netbox = require('net.box')
>> @@ -248,6 +253,51 @@ for i = 1, 100 do box.space.sync:delete{i} end
>>   test_run:cmd('switch replica')
>>   box.space.sync:count()
>>   +--
>> +-- gh-4849: clear synchro queue on a master
>> +--
>> +test_run:switch('default')
>> +box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout 
>> = 1000}
>> +ok, err = nil
>> +f = fiber.create(function()                            \
>> +    ok, err = pcall(box.space.sync.insert, box.space.sync, 
>> {10})        \
>> +end)
>> +f:status()
>> +test_run:switch('replica')
>> +test_run:wait_cond(function() return box.space.sync:get{10} ~= nil end)
>> +test_run:switch('default')
>> +box.cfg{replication_synchro_timeout = 0.1}
>> +box.ctl.clear_synchro_queue()
>> +test_run:switch('replica')
>> +test_run:wait_cond(function() return box.space.sync:get{10} == nil end)
>> +test_run:switch('default')
>> +test_run:wait_cond(function() return f:status() == 'dead' end)
>> +ok, err
>> +test_run:wait_cond(function() return box.space.sync:get{10} == nil end)
>> +
>> +--
>> +-- gh-4849: clear synchro queue on a replica, make sure no crashes
>> +--
>> +test_run:switch('default')
>> +box.cfg{replication_synchro_quorum = 3, replication_synchro_timeout 
>> = 1000}
>> +ok, err = nil
>> +f = fiber.create(function() \
>> +    ok, err = pcall(box.space.sync.insert, box.space.sync, 
>> {9})                 \
>> +end)
>> +f.status()
>> +test_run:wait_cond(function() return box.space.sync:get{9} ~= nil end)
>> +test_run:switch('replica')
>> +box.cfg{replication_synchro_quorum = 3, 
>> replication_synchro_timeout=0.01}
>> +box.ctl.clear_synchro_queue()
>> +test_run:wait_cond(function() return box.space.sync:get{9} == nil end)
>> +test_run:switch('default')
>> +box.cfg{replication_synchro_timeout=0.01}
>> +test_run:wait_cond(function() return f:status() == 'dead' end)
>> +ok, err
>> +test_run:wait_cond(function() return box.space.sync:get{9} == nil end)
>> +
>> +-- Note: cluster may be in a broken state here due to nature of 
>> previous test.
>> +
>>   -- Cleanup.
>>   test_run:cmd('switch default')
>