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 B388A6EC5F; Sun, 18 Apr 2021 12:26:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B388A6EC5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1618738003; bh=fTZZN5dYKZTnswyXQunzDPo0hgvoVvDmnT+6u49VRf0=; 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=Stjsd0pFiwSxRVEE3ndS3lUjRJ8S0YAsif4UcoBDhY4fJOPu5W4qZ5/z1n0YPn8wa 5fIo7eo0XqdJydJKcx9T78ioV9WprdKqUXulaExzeQeaqrbugz/wl3w68ofwbHP0ST JLCVYiFfiLKY1xMnbojJ2oMEgrEi+9eRsaHGe110= 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 B1CB16EC5F for ; Sun, 18 Apr 2021 12:26:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B1CB16EC5F Received: by smtp52.i.mail.ru with esmtpa (envelope-from ) id 1lY3hR-0003DU-Tv; Sun, 18 Apr 2021 12:26:42 +0300 To: Vladislav Shpilevoy , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org References: <05b14638-53d8-51db-4a85-3adafd611db6@tarantool.org> Message-ID: <5b3ec61c-5790-b27e-9a8c-9964bbcb6b4f@tarantool.org> Date: Sun, 18 Apr 2021 12:26:41 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: <05b14638-53d8-51db-4a85-3adafd611db6@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E7480BE79914FF86F9151AC38CC435EA4A654182A05F5380850404ACF104019E58BBBF07BE2B53A800C897B50D36238C55BEFAF234942929F564A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE74D62681FFDF80F84EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006372BE3E2E75E3847F48638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B2C9B8FDFA39A459D0F55089CBCF97F71C8CCF7685675CC1A7D2E47CDBA5A96583C09775C1D3CA48CFE478A468B35FE767117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE79AE9BAF3542BD4619FA2833FD35BB23DF004C90652538430302FCEF25BFAB3454AD6D5ED66289B5278DA827A17800CE7302FCEF25BFAB3454AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C3456F507CAE568110BA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637A7EFCB0EB5ACB161EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A24209795067102C07E8F7B195E1C9783114BFD9D79657871AFCB540272E22000E X-C1DE0DAB: 0D63561A33F958A5D71E1B279335C3B2786A163F0A40E938E85A2F6DB1B5DBC1D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34087368DEEB028F17AE247D8A69EE1A28558DF02F71456CA48C1ED55A6EF108582454ADE68A9EF3A61D7E09C32AA3244C475FCFF7B21985E670AC00D4F4530DF860759606DA2E136AFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj1t4H7vLuVFVU72HRyVa5og== X-Mailru-Sender: 583F1D7ACE8F49BDD2846D59FC20E9F828F59AAB003B88376E737B3F4EFDB72A55886B8987D112E3424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v4 10/12] election: support manual elections in clear_synchro_queue() 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" 17.04.2021 01:24, Vladislav Shpilevoy пишет: > Thanks for the patch! > > See 1 comment below. > >> diff --git a/src/box/box.cc b/src/box/box.cc >> index d5a55a30a..fcd812c09 100644 >> --- a/src/box/box.cc >> +++ b/src/box/box.cc >> @@ -1521,12 +1521,75 @@ box_clear_synchro_queue(bool try_wait) >> if (!is_box_configured || >> raft_node_term(box_raft(), instance_id) == box_raft()->term) >> return 0; >> + >> + bool run_elections = false; >> + >> + switch (box_election_mode) { >> + case ELECTION_MODE_OFF: >> + break; >> + case ELECTION_MODE_VOTER: >> + assert(box_raft()->state == RAFT_STATE_FOLLOWER); >> + diag_set(ClientError, ER_UNSUPPORTED, "election_mode='voter'", >> + "manual elections"); >> + return -1; >> + case ELECTION_MODE_MANUAL: >> + assert(box_raft()->state == RAFT_STATE_FOLLOWER); >> + run_elections = true; >> + try_wait = false; >> + break; >> + case ELECTION_MODE_CANDIDATE: >> + /* >> + * Leader elections are enabled, and this instance is allowed to >> + * promote only if it's already an elected leader. No manual >> + * elections. >> + */ >> + if (box_raft()->state != RAFT_STATE_LEADER) { >> + diag_set(ClientError, ER_UNSUPPORTED, "election_mode=" >> + "'candidate'", "manual elections"); >> + return -1; >> + } >> + break; >> + default: >> + unreachable(); >> + } >> + >> uint32_t former_leader_id = txn_limbo.owner_id; >> int64_t wait_lsn = txn_limbo.confirmed_lsn; >> int rc = 0; >> int quorum = replication_synchro_quorum; >> in_clear_synchro_queue = true; >> >> + if (run_elections) { >> + /* >> + * Make this instance a candidate and run until some leader, not >> + * necessarily this instance, emerges. >> + */ >> + raft_start_candidate(box_raft()); >> + /* >> + * Trigger new elections without waiting for an old leader to >> + * disappear. >> + */ >> + raft_new_term(box_raft()); >> + box_raft_wait_leader_found(); > Shouldn't we wait for election_timeout? I think not. Let's wait for however long it takes to elect a leader. Several terms may pass before the leader is finally elected. I mean, IMO it would be simpler for the user to do: ``` box.ctl.promote()    -- term1, split vote    -- term2, split vote    -- term3, leader found -- success ``` rather than ``` box.ctl.promote() -- error, split vote box.ctl.promote() -- error, split vote box.ctl.promote() -- success ``` > > Also what if the fiber is canceled before the leader is found? It > seems box_raft_wait_leader_found() would fail on an assertion because > raft is still enabled, but leader_id is nil. Thanks for noticing! Will fix. Diff: ================================== diff --git a/src/box/box.cc b/src/box/box.cc index 962f649c3..797aa86b5 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -1572,13 +1572,17 @@ box_clear_synchro_queue(bool try_wait)                  * disappear.                  */                 raft_new_term(box_raft()); -               box_raft_wait_leader_found(); +               rc = box_raft_wait_leader_found();                 /*                  * Do not reset raft mode if it was changed while running the                  * elections.                  */                 if (box_election_mode == ELECTION_MODE_MANUAL)                         raft_stop_candidate(box_raft(), false); +               if (rc != 0) { +                       in_clear_synchro_queue = false; +                       return -1; +               }                 if (!box_raft()->is_enabled) {                         diag_set(ClientError, ER_RAFT_DISABLED);                         in_clear_synchro_queue = false; diff --git a/src/box/raft.c b/src/box/raft.c index 425353207..61fa9f91b 100644 --- a/src/box/raft.c +++ b/src/box/raft.c @@ -347,15 +347,20 @@ box_raft_wait_leader_found_f(struct trigger *trig, void *event)         return 0;  } -void +int  box_raft_wait_leader_found(void)  {         struct trigger trig;         trigger_create(&trig, box_raft_wait_leader_found_f, fiber(), NULL);         raft_on_update(box_raft(), &trig);         fiber_yield(); -       assert(box_raft()->leader != REPLICA_ID_NIL || !box_raft()->is_enabled);         trigger_clear(&trig); +       if (fiber_is_cancelled()) { +               diag_set(FiberIsCancelled); +               return -1; +       } +       assert(box_raft()->leader != REPLICA_ID_NIL || !box_raft()->is_enabled); +       return 0;  }  void diff --git a/src/box/raft.h b/src/box/raft.h index 8fce423e1..6b6136510 100644 --- a/src/box/raft.h +++ b/src/box/raft.h @@ -97,7 +97,8 @@ box_raft_checkpoint_remote(struct raft_request *req);  int  box_raft_process(struct raft_request *req, uint32_t source); -void +/** Block this fiber until Raft leader is known. */ +int  box_raft_wait_leader_found();  void > >> + /* >> + * Do not reset raft mode if it was changed while running the >> + * elections. >> + */ >> + if (box_election_mode == ELECTION_MODE_MANUAL) >> + raft_stop_candidate(box_raft(), false); >> + if (!box_raft()->is_enabled) { >> + diag_set(ClientError, ER_RAFT_DISABLED); >> + in_clear_synchro_queue = false; >> + return -1; >> + } >> + if (box_raft()->state != RAFT_STATE_LEADER) { >> + diag_set(ClientError, ER_INTERFERING_PROMOTE, >> + box_raft()->leader); >> + in_clear_synchro_queue = false; >> + return -1; >> + } >> + } >> + >> if (txn_limbo_is_empty(&txn_limbo)) >> goto promote; >> -- Serge Petrenko