From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 C8E2B469719 for ; Tue, 10 Nov 2020 10:48:33 +0300 (MSK) References: <68f35f200bdeb4fa5195c2d767ebc74bfec9c8da.1604767356.git.v.shpilevoy@tarantool.org> <043d73e1-9a87-12e6-c09f-f977beffc6f8@tarantool.org> From: Serge Petrenko Message-ID: <5c4fd84d-c8a6-0814-4f05-f1838801c133@tarantool.org> Date: Tue, 10 Nov 2020 10:48:32 +0300 MIME-Version: 1.0 In-Reply-To: <043d73e1-9a87-12e6-c09f-f977beffc6f8@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH 2/2] raft: fix crash on candidate cfg during WAL write List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org, avtikhon@tarantool.org 10.11.2020 01:42, Vladislav Shpilevoy пишет: > Thanks for the review! > >>> diff --git a/test/replication/gh-5506-election-on-off.result b/test/replication/gh-5506-election-on-off.result >>> index 1a718396f..4fbc31986 100644 >>> --- a/test/replication/gh-5506-election-on-off.result >>> +++ b/test/replication/gh-5506-election-on-off.result >>> @@ -47,6 +47,91 @@ box.error.injection.set("ERRINJ_WAL_DELAY", false) >>>    | - ok >>>    | ... >>>   +box.cfg{                                                                        \ >>> +    election_mode = old_election_mode,                                          \ >>> +} >>> + | --- >>> + | ... >>> + >>> +-- >>> +-- Another crash could happen when election mode was configured to be >>> +-- 'candidate' with a known leader, but there was a not finished WAL write. >>> +-- The node tried to start waiting for the leader death, even though with an >>> +-- active WAL write it should wait for its end first. >>> +-- >>> +box.schema.user.grant('guest', 'super') >> >> box.schema.user.grant('guest', 'replication') is enough. > I know. But I prefer using super, because it works always. I don't > want to think of the weakest possible rights which would fit. So > I usually add 'super' and forget about it. Ok. > >>> + | --- >>> + | ... >>> +test_run:cmd('create server replica with rpl_master=default,\ >>> +              script="replication/replica.lua"') >>> + | --- >>> + | - true >>> + | ... >>> +test_run:cmd('start server replica with wait=True, wait_load=True') >>> + | --- >>> + | - true >>> + | ... >>> + >>> +test_run:switch('replica') >>> + | --- >>> + | - true >>> + | ... >>> +box.cfg{election_mode = 'voter'} >>> + | --- >>> + | ... >>> +box.error.injection.set("ERRINJ_WAL_DELAY_COUNTDOWN", 0) >>> + | --- >>> + | - ok >>> + | ... >>> + >>> +test_run:switch('default') >>> + | --- >>> + | - true >>> + | ... >>> +box.cfg{election_mode = 'candidate'} >>> + | --- >>> + | ... >>> +test_run:wait_cond(function()                                                   \ >>> +    return box.info.election.leader ~= 0                                        \ >>> +end) >> >> Wouldn't it be simpler to wait for `box.info.election.state == 'leader'`? > I don't mind. > > ==================== > diff --git a/test/replication/gh-5506-election-on-off.result b/test/replication/gh-5506-election-on-off.result > index 4fbc31986..b8abd7ecd 100644 > --- a/test/replication/gh-5506-election-on-off.result > +++ b/test/replication/gh-5506-election-on-off.result > @@ -92,7 +92,7 @@ box.cfg{election_mode = 'candidate'} > | --- > | ... > test_run:wait_cond(function() \ > - return box.info.election.leader ~= 0 \ > + return box.info.election.state == 'leader' \ > end) > | --- > | - true > diff --git a/test/replication/gh-5506-election-on-off.test.lua b/test/replication/gh-5506-election-on-off.test.lua > index bb89477d1..476b00ec0 100644 > --- a/test/replication/gh-5506-election-on-off.test.lua > +++ b/test/replication/gh-5506-election-on-off.test.lua > @@ -47,7 +47,7 @@ box.error.injection.set("ERRINJ_WAL_DELAY_COUNTDOWN", 0) > test_run:switch('default') > box.cfg{election_mode = 'candidate'} > test_run:wait_cond(function() \ > - return box.info.election.leader ~= 0 \ > + return box.info.election.state == 'leader' \ > end) > > test_run:switch('replica') Thanks for the changes! LGTM. -- Serge Petrenko