From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 27F79469719 for ; Tue, 10 Nov 2020 01:42:15 +0300 (MSK) References: <68f35f200bdeb4fa5195c2d767ebc74bfec9c8da.1604767356.git.v.shpilevoy@tarantool.org> From: Vladislav Shpilevoy Message-ID: <043d73e1-9a87-12e6-c09f-f977beffc6f8@tarantool.org> Date: Mon, 9 Nov 2020 23:42:12 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit 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: Serge Petrenko , tarantool-patches@dev.tarantool.org, avtikhon@tarantool.org 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. >> + | --- >> + | ... >> +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')