From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 459AD469719 for ; Mon, 16 Nov 2020 17:44:28 +0300 (MSK) References: <07e4c4aa-0360-d2d4-8c80-40a61c150d14@tarantool.org> From: Serge Petrenko Message-ID: <46909a70-c0ae-a191-425c-3218238c6ebb@tarantool.org> Date: Mon, 16 Nov 2020 17:44:26 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: ru 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: Sergey Bronnikov , tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org 16.11.2020 14:11, Sergey Bronnikov пишет: > Hi, Serge! > > thanks for review! Thanks for the fixes! LGTM. > > 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') >> -- Serge Petrenko