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 660A16EC55; Thu, 22 Jul 2021 02:28:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 660A16EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1626910117; bh=iYqGRiCy6UrUPRHSMOKLO7deaXS51GgHUm9xA85M8L4=; 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=HfbIZwB1+V8ricXLpI9FgtJu/xjxTnSauV23TVMzN+28ZwRV7iqSurgYqMfsN4eDq GA81MRdfz2Hze9/zgq86SqEppT4t5cj87L8KBCvIG7VNybp/V7FD1Q38wLrghfTR6g zOCU65RgDq8UElFd3GpUrrsOBBjADBWKJdmMI638= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [94.100.179.3]) (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 A4F176EC55 for ; Thu, 22 Jul 2021 02:28:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A4F176EC55 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1m6Ldh-000233-Sp; Thu, 22 Jul 2021 02:28:34 +0300 To: Serge Petrenko , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org References: <3b4f7752-f68b-afce-c83a-355bcfbc28cc@tarantool.org> Message-ID: <9ae5765f-ffcb-af66-469b-b0a99d71c331@tarantool.org> Date: Thu, 22 Jul 2021 01:28:32 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <3b4f7752-f68b-afce-c83a-355bcfbc28cc@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD941C43E597735A9C320FB367CC9CBEE800EEBD3117ABA9A5B182A05F538085040626CA481E07F393FE98DDB0E83AD96E0CBA91ED1131C55CC90585ADAC665B85E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE77F05ED334962579DEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063706922F90966A37BA8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D89BAFEB95C7822C53E16A3CA3611E7504117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC8C7ADC89C2F0B2A5A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18F04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B613439FA09F3DCB32089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A55BDBD43A48CA3CFAD9B421B2B8701FD13D491D45C38AEAE4D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7501A9DF589746230F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D346EDE9E12965E8CD53D1AFE34D156B9E37BEB92674869058464D2B5F23D097ED07878C65C8F6208A61D7E09C32AA3244CFB971DDA03F9F8779A48C8B86230DC878894E9C85370243EFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojaAaPr+N/4d2aBxESmfiqjw== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D92F39E83C47256296FB7DDF4989BF0DB3841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v3 07/12] box: introduce `box.ctl.demote` 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" Thanks for the fixes! >> 2. On top of this commit and on top of the branch too I tried to >> promote a candidate and got a strange error in the logs, although >> the promotion was successful: >> >> -- >> -- Instance 1 >> -- >> >> -- Step 1 >> box.cfg{ >>      listen = 3313, >>      election_mode = 'candidate', >>      replication_synchro_quorum = 2, >>      replication = {'localhost:3314', 'localhost:3313'} >> } >> box.schema.user.grant('guest', 'super') >> >> >> -- >> -- Instance 2 >> -- >> >> -- Step 2 >> box.cfg{ >>      listen = 3314, >>      election_mode = 'voter', >>      replication_synchro_quorum = 2, >>      replication = {'localhost:3314', 'localhost:3313'}, >>      read_only = true, >> } >> >> -- Step 3 >> box.cfg{read_only = false, election_mode = 'candidate'} >> >> -- Step 4 >> box.ctl.promote() >> >> main/112/raft_worker box.cc:1538 E> ER_UNSUPPORTED: box.ctl.promote does not support simultaneous invocations >> --- >> ... > > That's because once a candidate becomes the leader, it tries to issue `box.ctl.promote()`, and fails, > since we're already in `box.ctl.promote()` call. > I'm not sure how to handle that properly. This doesn't break anything though. Still, the error message looks really not good. There is an option - make box_promote() for candidate node just call raft_promote() and set box_in_promote = false. Then wait for the term outcome. Will it work? You would need to rebase your patchset on master branch then. It might be a little easier to do if you apply the diff below. (Warning: I didn't test it.) The motivation is that one of the main reasons why I wanted box_promote() simplified was because of the strange meaning of some flags. In particular, try_wait flag somewhy triggered elections before the waiting which is super not obvious why. How does 'wait' come to 'elections'? In the diff I tried to remove these flags entirely. And now you have a single place in the code of box_promote(), where ELECTION_MODE_CANDIDATE stuff is handled. Here you could try the proposal I gave above. ==================== diff --git a/src/box/box.cc b/src/box/box.cc index f68fffcab..e7765b657 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -1698,34 +1698,6 @@ box_issue_demote(uint32_t prev_leader_id, int64_t promote_lsn) assert(txn_limbo_is_empty(&txn_limbo)); } -/** - * Check whether this instance may run a promote() and set promote parameters - * according to its election mode. - */ -static int -box_check_promote_election_mode(bool *try_wait, bool *run_elections) -{ - switch (box_election_mode) { - case ELECTION_MODE_OFF: - if (try_wait != NULL) - *try_wait = true; - 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: - case ELECTION_MODE_CANDIDATE: - if (run_elections != NULL) - *run_elections = box_raft()->state != RAFT_STATE_LEADER; - break; - default: - unreachable(); - } - return 0; -} - /* A guard to block multiple simultaneous promote()/demote() invocations. */ static bool box_in_promote = false; @@ -1757,27 +1729,35 @@ box_promote(void) if (is_leader) return 0; - bool run_elections = false; - bool try_wait = false; - - if (box_check_promote_election_mode(&try_wait, &run_elections) < 0) - return -1; - - int64_t wait_lsn = -1; - - if (run_elections && box_run_elections() < 0) - return -1; - if (try_wait) { - if (box_try_wait_confirm(2 * replication_synchro_timeout) < 0) + switch (box_election_mode) { + case ELECTION_MODE_OFF: + if (box_try_wait_confirm(2 * replication_synchro_timeout) != 0) + return -1; + if (box_trigger_elections() != 0) return -1; - if (box_trigger_elections() < 0) + 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: + case ELECTION_MODE_CANDIDATE: + if (box_raft()->state != RAFT_STATE_LEADER && + box_run_elections() != 0) return -1; + break; + default: + unreachable(); } - if ((wait_lsn = box_wait_limbo_acked()) < 0) + if (box_check_promote_election_mode(&try_wait, &run_elections) < 0) return -1; - box_issue_promote(txn_limbo.owner_id, wait_lsn); + int64_t wait_lsn = box_wait_limbo_acked(); + if (wait_lsn < 0) + return -1; + box_issue_promote(txn_limbo.owner_id, wait_lsn); return 0; } @@ -1804,29 +1784,16 @@ box_demote(void) is_leader = is_leader && box_raft()->state == RAFT_STATE_LEADER; if (!is_leader) return 0; - - bool try_wait = false; - - if (box_check_promote_election_mode(&try_wait, NULL) < 0) - return -1; - - int64_t wait_lsn = -1; - if (box_trigger_elections() < 0) return -1; - if (box_election_mode != ELECTION_MODE_OFF) return 0; - - if (try_wait && - box_try_wait_confirm(2 * replication_synchro_timeout) < 0) + if (box_try_wait_confirm(2 * replication_synchro_timeout) != 0) return -1; - - if ((wait_lsn = box_wait_limbo_acked()) < 0) + int64_t wait_lsn = box_wait_limbo_acked(); + if (wait_lsn < 0) return -1; - box_issue_demote(txn_limbo.owner_id, wait_lsn); - return 0; }