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 9EA896EC57; Wed, 14 Jul 2021 21:34:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9EA896EC57 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1626287669; bh=wMCUuxUQO6fORlQ5fQOFnjBAjZ6JGK7Cjzp0wqecRpA=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=q1xAB5sPt9a0ovrPGpOSPxjAmGvtsyBGGBk6lMpBQzH4eqUbPQQCUcV+682xXI07y PkJQaeK1pf6GBIZwrj7Y1ScrAFOucHtKUzcFi3cBoQzjnf1Bs/M8xLTSZcVzcfYwD0 xY7rBFQ85DEUPc2TtcVfJU2z0vEFDXGosHO0YtQ4= Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (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 9D8246E222 for ; Wed, 14 Jul 2021 21:26:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9D8246E222 Received: by smtp45.i.mail.ru with esmtpa (envelope-from ) id 1m3jap-0005rO-W7; Wed, 14 Jul 2021 21:26:48 +0300 To: Vladislav Shpilevoy , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org References: <46c167f8c9bfe0e35370b0cbf10505d9d4a888ba.1624918077.git.sergepetrenko@tarantool.org> Message-ID: <27e527db-99b8-0e4e-1d83-b678336a8659@tarantool.org> Date: Wed, 14 Jul 2021 21:26:47 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD97BB0EF39AD2B33D54F26E6113A59F95A22EFF9DCA932A94B182A05F538085040BB4EA2D835D1050DF43614762B11A9DE6A62994E56E2E8B6F257484928766C04 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE754D4963CE5BA093BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375CC217B55A7C05578638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8D75B4A2D2109689800EBD12E36062EEB117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC974A882099E279BDA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735201E561CDFBCA1751FF04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6D0C9BB9AE6BD5D69089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5ADA874904662624D5FE91CB9A0687E64FE68FEBE04674BD9D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75E1D4179F59451251410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3454548929AF40B4281E0A4E4096DF528B43B297540A432CA4AE0C0069649538BC2A2DB941F1F721EB1D7E09C32AA3244C9BA4456202D188DCBA3642B1FDB488B8F94338140B71B8EEFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojDdSFIg49M1Q7QxqbDVylGA== X-Mailru-Sender: 3B9A0136629DC9125D61937A2360A446DCF67A6ACA5D040DA0A35BDF2E74F82A4A275DB40FA55CC2424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v3 04/12] box: make promote always bump the term 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: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 04.07.2021 15:14, Vladislav Shpilevoy пишет: > Thanks for the patch! Thanks for the review! > Did you think about making it NOP when the node is already a leader > (even in manual/off mode)? The current solution is all good except > that it makes the current leader temporary read-only until it wins > the election again, which looks strange. I would say "unexpected" for > users. Sure, why not. I'll address it in a separate commit. > See 4 comments below. > >> diff --git a/src/box/box.cc b/src/box/box.cc >> index 6a0950f44..ce37b307d 100644 >> --- a/src/box/box.cc >> +++ b/src/box/box.cc >> @@ -1687,16 +1687,19 @@ box_promote(void) >> rc = -1; >> } else { >> promote: >> - /* We cannot possibly get here in a volatile state. */ >> - assert(box_raft()->volatile_term == box_raft()->term); >> - txn_limbo_write_promote(&txn_limbo, wait_lsn, >> - box_raft()->term); >> + if (try_wait) { >> + raft_new_term(box_raft()); >> + if (box_raft_wait_persisted() < 0) > 1. What if during term WAL write another node also started promote, > won the elections and delivered the promote to us? I suppose after > the WAL write we will silently write PROMOTE for the term which > was won by somebody else, right? Can it be covered by a test? Yep, need to handle that. >> + return -1; >> + } >> + uint64_t term = box_raft()->term; >> + txn_limbo_write_promote(&txn_limbo, wait_lsn, term); >> struct synchro_request req = { >> .type = IPROTO_PROMOTE, >> .replica_id = former_leader_id, >> .origin_id = instance_id, >> .lsn = wait_lsn, >> - .term = box_raft()->term, >> + .term = term, >> }; >> txn_limbo_process(&txn_limbo, &req); >> assert(txn_limbo_is_empty(&txn_limbo)); >> diff --git a/src/box/raft.c b/src/box/raft.c >> index 7f787c0c5..17caf6f54 100644 >> --- a/src/box/raft.c >> +++ b/src/box/raft.c >> @@ -354,6 +354,42 @@ box_raft_wait_leader_found(void) >> return 0; >> } >> >> +struct raft_wait_persisted_data { >> + struct fiber *waiter; >> + uint64_t term; >> +}; >> + >> +static int >> +box_raft_wait_persisted_f(struct trigger *trig, void *event) >> +{ >> + struct raft *raft = event; >> + struct raft_wait_persisted_data *data = trig->data; >> + if (raft->term >= data->term) >> + fiber_wakeup(data->waiter); >> + return 0; >> +} >> + >> +int >> +box_raft_wait_persisted(void) >> +{ >> + if (box_raft()->term == box_raft()->volatile_term) > 2. Since it only waits for term being persisted, I would rather > call it 'wait_term_persisted'. Because there is also vote, and > you do not look at it. Ok. >> + return 0; >> + struct raft_wait_persisted_data data = { >> + .waiter = fiber(), >> + .term = box_raft()->volatile_term, >> + }; >> + struct trigger trig; >> + trigger_create(&trig, box_raft_wait_persisted_f, &data, NULL); >> + raft_on_update(box_raft(), &trig); >> + fiber_yield(); > 3. What about spurious wakeups? I could call fiber.wakeup() from > Lua on this fiber. Yep, need to handle that. I"ll do that for box_raft_wait_leader_found() as well. In a separate commit. Good catch! >> + trigger_clear(&trig); >> + if (fiber_is_cancelled()) { >> + diag_set(FiberIsCancelled); >> + return -1; >> + } >> + return 0; >> +} >> diff --git a/test/replication/gh-4114-local-space-replication.result b/test/replication/gh-4114-local-space-replication.result >> index 9b63a4b99..e71eb60a8 100644 >> --- a/test/replication/gh-4114-local-space-replication.result >> +++ b/test/replication/gh-4114-local-space-replication.result >> @@ -45,9 +45,8 @@ test_run:cmd('switch replica') >> | --- >> | - true >> | ... >> -box.info.vclock[0] >> +a = box.info.vclock[0] or 0 >> | --- >> - | - null >> | ... >> box.cfg{checkpoint_count=1} >> | --- >> @@ -77,9 +76,9 @@ box.space.test:insert{3} >> | - [3] >> | ... >> >> -box.info.vclock[0] >> +assert(box.info.vclock[0] == a + 3) >> | --- >> - | - 3 >> + | - true > 4. Why do you need these changes? I reverted this test and it passed. When the test's run after some election test, master has non-default raft term, and replica persists this term bumping vclock[0]. -- Serge Petrenko