From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id DE646469719 for ; Mon, 16 Nov 2020 14:11:52 +0300 (MSK) References: <07e4c4aa-0360-d2d4-8c80-40a61c150d14@tarantool.org> From: Sergey Bronnikov Message-ID: Date: Mon, 16 Nov 2020 14:11:51 +0300 MIME-Version: 1.0 In-Reply-To: <07e4c4aa-0360-d2d4-8c80-40a61c150d14@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH 1/3 v2] replication: test clear_synchro_queue() function List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Petrenko , tarantool-patches@dev.tarantool.org, v.shpilevoy@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 >> >> 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') >