From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 0D3F26ECE3; Thu, 20 Jan 2022 03:44:21 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0D3F26ECE3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1642639461; bh=Fdfc/e5XSLVOnutzilZnEge5F17/2WcCyeLegc5DNSM=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Yo5jLjHo6700MbSykI3FyOFDX4Tzm9cgCeA2EuWukopxv3DPtCaGWe3xaIiVVC0JI bnuE+3qEwmbou7OO6bZWkuiyh7gm+D41kEvFctUpTce8uW06r+zLbIUlnY6CepYNm1 c7eNrAyIPAuTD3lS5fy0iK6lJpHAAaehsquHSSoo= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [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 8C2166ECE3 for ; Thu, 20 Jan 2022 03:43:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8C2166ECE3 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1nALYK-0006wT-SW; Thu, 20 Jan 2022 03:43:49 +0300 To: tarantool-patches@dev.tarantool.org, sergepetrenko@tarantool.org Date: Thu, 20 Jan 2022 01:43:43 +0100 Message-Id: <4cde0a624f2f716e868653030048771643e84bc7.1642639079.git.v.shpilevoy@tarantool.org> X-Mailer: git-send-email 2.24.3 (Apple Git-128) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD98A33503A0B8627DB9E78F06BE902B0B8946864876BAF670B182A05F53808504091991DE314BBBDCEACE5EFDEE0B8612847B8C43CF74606D033338EC53707B85A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE76256B078082242EEEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006373C9FC9F3BACECB908638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8474CB3DA61FFF7D4668720835B576F50117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC2EE5AD8F952D28FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735201E561CDFBCA1751FF04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6B1CFA6D474D4A6A4089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C4C7A0BC55FA0FE5FCBEEB5B7715175159397E5776AB1445B49C0947B63BC522E6B1881A6453793CE9C32612AADDFBE061C61BE10805914D3804EBA3D8E7E5B87ABF8C51168CD8EBDB049C5FA07E1E73E1DC48ACC2A39D04F89CDFB48F4795C241BDAD6C7F3747799A X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D346840168BCAD8054E6B8220AE0D3CA17FF1F8FCCBBCF4EBB966B03DF2A8B6B108E85B70BBAF76A2241D7E09C32AA3244C62944D0223F0A916D60F6B029D4AEDA3E646F07CC2D4F3D8729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojPeoZbWa28mxqrs/AKTTa/g== X-Mailru-Sender: 689FA8AB762F739339CABD9B3CA9A7D693FDC60DFC4F0E3DC8DDDD8D92648A7F3841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E25FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: [Tarantool-patches] [PATCH v2 1/5] raft: fix crash on election_timeout reconfig X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" It used to crash if done during election on a node voted for anybody, it is a candidate, it doesn't know a leader yet, but has a WAL write in progress. Thus it could only happen if the term was bumped by a message from a non-leader node and wasn't flushed to the disk yet. The patch makes the reconfig check if there is a WAL write in progress. Then don't do anything. Could also check for volatile vote instead of persistent, but it would create the same problem for the case when started writing vote for self and didn't finish yet. Reconfig would crash. --- .../unreleased/election-timeout-cfg-crash.md | 5 ++ src/lib/raft/raft.c | 3 +- test/unit/raft.c | 75 ++++++++++++++++++- test/unit/raft.result | 16 +++- 4 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/election-timeout-cfg-crash.md diff --git a/changelogs/unreleased/election-timeout-cfg-crash.md b/changelogs/unreleased/election-timeout-cfg-crash.md new file mode 100644 index 000000000..e870e3665 --- /dev/null +++ b/changelogs/unreleased/election-timeout-cfg-crash.md @@ -0,0 +1,5 @@ +## bugfix/raft + +* Reconfiguration of `box.cfg.election_timeout` could lead to a crash or + undefined behaviour if done during an ongoing election with a special WAL + write in progress. diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c index 0e6dc6155..1ae8fe87f 100644 --- a/src/lib/raft/raft.c +++ b/src/lib/raft/raft.c @@ -935,7 +935,8 @@ raft_cfg_election_timeout(struct raft *raft, double timeout) return; raft->election_timeout = timeout; - if (raft->vote != 0 && raft->leader == 0 && raft->is_candidate) { + if (raft->vote != 0 && raft->leader == 0 && raft->is_candidate && + !raft->is_write_in_progress) { assert(raft_ev_is_active(&raft->timer)); struct ev_loop *loop = raft_loop(); double timeout = raft_ev_timer_remaining(loop, &raft->timer) - diff --git a/test/unit/raft.c b/test/unit/raft.c index 8f4d2dd9a..520b94466 100644 --- a/test/unit/raft.c +++ b/test/unit/raft.c @@ -1296,7 +1296,7 @@ raft_test_enable_disable(void) static void raft_test_too_long_wal_write(void) { - raft_start_test(8); + raft_start_test(22); struct raft_node node; raft_node_create(&node); @@ -1362,6 +1362,79 @@ raft_test_too_long_wal_write(void) ok(raft_time() - ts == node.cfg_death_timeout, "timer works again"); is(node.raft.state, RAFT_STATE_CANDIDATE, "became candidate"); + /* + * During WAL write it is possible to reconfigure election timeout. + * The dangerous case is when the timer is active already. It happens + * when the node voted and is a candidate, but leader is unknown. + */ + raft_node_destroy(&node); + raft_node_create(&node); + + raft_node_cfg_election_timeout(&node, 100); + raft_run_next_event(); + is(node.raft.term, 2, "term is bumped"); + + /* Bump term again but it is not written to WAL yet. */ + raft_node_block(&node); + is(raft_node_send_vote_response(&node, + 3 /* Term. */, + 3 /* Vote. */, + 2 /* Source. */ + ), 0, "2 votes for 3 in a new term"); + raft_run_next_event(); + is(node.raft.term, 2, "term is old"); + is(node.raft.vote, 1, "vote is used for self"); + is(node.raft.volatile_term, 3, "volatile term is new"); + is(node.raft.volatile_vote, 0, "volatile vote is unused"); + + raft_node_cfg_election_timeout(&node, 50); + raft_node_unblock(&node); + ts = raft_time(); + raft_run_next_event(); + ts = raft_time() - ts; + /* 50 + <= 10% random delay. */ + ok(ts >= 50 && ts <= 55, "new election timeout works"); + ok(raft_node_check_full_state(&node, + RAFT_STATE_CANDIDATE /* State. */, + 0 /* Leader. */, + 4 /* Term. */, + 1 /* Vote. */, + 4 /* Volatile term. */, + 1 /* Volatile vote. */, + "{0: 4}" /* Vclock. */ + ), "new term is started with vote for self"); + + /* + * Similar case when a vote is being written but not finished yet. + */ + raft_node_destroy(&node); + raft_node_create(&node); + + raft_node_cfg_election_timeout(&node, 100); + raft_node_block(&node); + raft_run_next_event(); + is(node.raft.term, 1, "term is old"); + is(node.raft.vote, 0, "vote is unused"); + is(node.raft.volatile_term, 2, "volatile term is new"); + is(node.raft.volatile_vote, 1, "volatile vote is self"); + + raft_node_cfg_election_timeout(&node, 50); + raft_node_unblock(&node); + ts = raft_time(); + raft_run_next_event(); + ts = raft_time() - ts; + /* 50 + <= 10% random delay. */ + ok(ts >= 50 && ts <= 55, "new election timeout works"); + ok(raft_node_check_full_state(&node, + RAFT_STATE_CANDIDATE /* State. */, + 0 /* Leader. */, + 3 /* Term. */, + 1 /* Vote. */, + 3 /* Volatile term. */, + 1 /* Volatile vote. */, + "{0: 2}" /* Vclock. */ + ), "new term is started with vote for self"); + raft_node_destroy(&node); raft_finish_test(); } diff --git a/test/unit/raft.result b/test/unit/raft.result index 14689ea81..764d82969 100644 --- a/test/unit/raft.result +++ b/test/unit/raft.result @@ -220,7 +220,7 @@ ok 11 - subtests ok 12 - subtests *** raft_test_enable_disable: done *** *** raft_test_too_long_wal_write *** - 1..8 + 1..22 ok 1 - vote for 2 ok 2 - vote is volatile ok 3 - message from leader @@ -229,6 +229,20 @@ ok 12 - subtests ok 6 - wal write is finished ok 7 - timer works again ok 8 - became candidate + ok 9 - term is bumped + ok 10 - 2 votes for 3 in a new term + ok 11 - term is old + ok 12 - vote is used for self + ok 13 - volatile term is new + ok 14 - volatile vote is unused + ok 15 - new election timeout works + ok 16 - new term is started with vote for self + ok 17 - term is old + ok 18 - vote is unused + ok 19 - volatile term is new + ok 20 - volatile vote is self + ok 21 - new election timeout works + ok 22 - new term is started with vote for self ok 13 - subtests *** raft_test_too_long_wal_write: done *** *** raft_test_promote_restore *** -- 2.24.3 (Apple Git-128)