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 54B5C6EC40; Sun, 4 Jul 2021 15:27:23 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 54B5C6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1625401643; bh=ACPdvtveI+at1y1Mm560mn10qQLpDfjpTQzTXGAlwXI=; 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=noE2JovlmOb4z4khVNPmPIVqfKqcH7yZP18vqoLdnqbGatIUPYv/yqOU8A8huHWDs 3/RkFVu0r6iq41l99akjR3RBJs2ZYdJhfKLuaWfZJ4zG8Sod9LLYUObgumjN3EguVx gBu1Z3n2oM+ddIouAw9ARxDz8uhOSFj0pb1bX0KE= 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 B9F1D6EC40 for ; Sun, 4 Jul 2021 15:27:21 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B9F1D6EC40 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1m01DU-0008Mi-Ph; Sun, 04 Jul 2021 15:27:21 +0300 To: Serge Petrenko , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org References: Message-ID: Date: Sun, 4 Jul 2021 14:27:19 +0200 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 Content-Language: en-US Content-Transfer-Encoding: 7bit X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD954DFF1DC42D673FB703477AD6D36A6E3513B6A65BE508F5F182A05F538085040F2E37188B8146EFAA5167FDF894F6FE3D84E34DC73493B48AA57D8219AAB8788 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE736A5CCAC74C5E494EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637CFA2FDEB3954FDBC8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D823948B9744AD6156B2238FE48BFB64D4117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B67393CE827C55B5F775ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A5F082446F78C29CE22A4B15F50631441846C560F3274D306CD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7569E77FCA7B33833F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D342B8615F5CFAD9D0234CEF4806DF63CAC5F0582635B5E48EA611E9ACEE94F47010799C0EC80C044C61D7E09C32AA3244CAE59F6BB6145B4EB252DD19132A263D6C86C126E7119A0FEFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj5fH2RN9TpJmaR7sVbTT1Nw== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5DD4AEA23234453787BF7C780CC9BCE5483841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B 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 working on this! See 11 comments below. 1. On this commit a lot of tests fail. Why? 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 --- ... 3. On top of the branch I tried demote on a leader with election mode candidate 2 times. Demote didn't do anything on the second invocation. The test: -- -- 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') -- Step 3 -- I demote self but become leader again because I am a -- candidate. This is fine. box.ctl.demote() -- Step 4 tarantool> box.info.election --- - state: leader vote: 1 leader: 1 term: 3 ... -- Step 5 -- But now I try to demote again and nothing happens. -- I expected the same behaviour - bump term and become -- a leader again. tarantool> box.ctl.demote() --- ... tarantool> box.info.election --- - state: leader vote: 1 leader: 1 term: 3 <- the term is the same, so demote didn't do anything. ... -- -- Instance 2 -- -- Step 2 box.cfg{ listen = 3314, election_mode = 'voter', replication_synchro_quorum = 2, replication = {'localhost:3314', 'localhost:3313'}, read_only = true, } 4. I tried switching between candidate and manual, trying promote/demote in different combinations and got promote not doing anything. The test is the same as above but steps >= 3 are different: tarantool> box.ctl.demote() 2021-07-04 14:22:49.550 [588] main/103/interactive I> RAFT: bump term to 3, follow 2021-07-04 14:22:49.550 [588] main/113/raft_worker I> RAFT: persisted state {term: 3} 2021-07-04 14:22:49.550 [588] main/113/raft_worker I> RAFT: vote for 1, follow 2021-07-04 14:22:49.551 [588] main/113/raft_worker I> RAFT: persisted state {term: 3, vote: 1} 2021-07-04 14:22:49.551 [588] main/113/raft_worker I> RAFT: enter candidate state with 1 self vote --- ... tarantool> 2021-07-04 14:22:49.552 [588] main/110/applier/localhost:3314 I> RAFT: message {term: 3, state: follower} from 2 2021-07-04 14:22:49.552 [588] main/110/applier/localhost:3314 I> RAFT: message {term: 3, vote: 1, state: follower} from 2 2021-07-04 14:22:49.552 [588] main/110/applier/localhost:3314 I> RAFT: enter leader state with quorum 2 2021-07-04 14:22:49.552 [588] main/110/applier/localhost:3314 I> RAFT: message {term: 3, vote: 1, state: follower} from 2 2021-07-04 14:22:49.552 [588] main/110/applier/localhost:3314 I> RAFT: vote request is skipped - the leader is already known - 1 2021-07-04 14:22:49.553 [588] relay/[::1]:54006/101/main I> recover from `./00000000000000000000.xlog' --- ... tarantool> box.cfg{election_mode='manual'} 2021-07-04 14:22:55.391 [588] main/103/interactive I> set 'election_mode' configuration option to "manual" --- ... tarantool> box.info.election --- - state: follower vote: 1 leader: 0 term: 3 ... tarantool> box.ctl.promote() --- ... tarantool> box.info.election --- - state: follower vote: 1 leader: 0 term: 3 ... As you can see, the last promote() didn't do anything. The node stayed a follower, and no errors were raised. >From the comments 2-4 it seems to me the complexity of box_clear_synchro_queue()/box_promote() is getting out of control with the number of ifs and flags, sometimes having unexpected meaning (for example, try_wait flag causes term bump somewhy). It might be good to split it into separate functions and make promotion/demotion out of each election mode linear and straight. > @TarantoolBot document > Title: box.ctl.demote > > `box.ctl.demote()` is a new function, which works exactly like > `box.ctl.promote()`, with one exception that it results in the instance > writing DEMOTE request to WAL instead of a PROMOTE request. > > A DEMOTE request (DEMOTE = 32) copies PROMOTE behaviour (it clears the > limbo as well), but clears the synchronous transaction queue ownership instead > of assigning it to a new instance. 5. It is worth mentioning that demote can only be called on a master. > diff --git a/src/box/box.cc b/src/box/box.cc > index 1d894be97..86c5967b9 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -1674,15 +1680,22 @@ box_promote(void) > rc = -1; > } else { > promote: > - if (try_wait) { > + if (try_wait || demote) { > raft_new_term(box_raft()); > if (box_raft_wait_persisted() < 0) > return -1; 6. What if now some other node won the elections of the term you just bumped? Then you wouldn't have the right to perform DEMOTE as the term is not yours. > } > uint64_t term = box_raft()->term; > - txn_limbo_write_promote(&txn_limbo, wait_lsn, term); > + if (demote) { > + txn_limbo_write_demote(&txn_limbo, wait_lsn, > + term); > + } else { > + txn_limbo_write_promote(&txn_limbo, wait_lsn, > + term); > + } > + uint16_t type = demote ? IPROTO_DEMOTE : IPROTO_PROMOTE; > struct synchro_request req = { > - .type = IPROTO_PROMOTE, > + .type = type, > .replica_id = former_leader_id, > .origin_id = instance_id, > .lsn = wait_lsn, > diff --git a/test/replication/gh-5446-qsync-eval-quorum.test.lua b/test/replication/gh-5446-qsync-eval-quorum.test.lua > index 6b9e324ed..b969df836 100644 > --- a/test/replication/gh-5446-qsync-eval-quorum.test.lua > +++ b/test/replication/gh-5446-qsync-eval-quorum.test.lua > @@ -135,3 +136,5 @@ box.cfg{ > replication_synchro_quorum = old_synchro_quorum, \ > replication_synchro_timeout = old_synchro_timeout, \ > } > +box.ctl.demote() > + 7. Git diff highlights this new line with red as unnecessary. > diff --git a/test/replication/gh-6034-limbo-ownership.result b/test/replication/gh-6034-limbo-ownership.result > new file mode 100644 > index 000000000..e412b8d53 > --- /dev/null > +++ b/test/replication/gh-6034-limbo-ownership.result 8. Could you please write qsync tests with gh-####-qsync-... pattern? It is useful when you want to run all qsync tests and can do `python test-run.py qsync` then. > @@ -0,0 +1,189 @@ > +-- test-run result file version 2 > +test_run = require('test_run').new() > + | --- > + | ... > +fiber = require('fiber') 9. Fiber is not used in the test. > + | --- > + | ... > + <...> > +assert(box.info.synchro.queue.owner == test_run:eval('default', 'return box.info.id')[1]) 10. There is test_run:get_server_id(). > + | --- > + | - true > + | ... > +box.space.async:insert{2} -- failure. > + | --- > + | - error: Can't modify data because this instance is in read-only mode. > + | ... > + > +-- Promotion on the other node. Default should become ro. > +box.ctl.promote() > + | --- > + | ... > +assert(not box.info.ro) > + | --- > + | - true > + | ... > +assert(box.info.synchro.queue.owner == box.info.id) > + | --- > + | - true > + | ... > +box.space.sync:insert{2} -- success. > + | --- > + | - [2] > + | ... > + > +test_run:switch('default') > + | --- > + | - true > + | ... > +assert(box.info.ro) > + | --- > + | - true > + | ... > +assert(box.info.synchro.queue.owner == test_run:eval('replica', 'return box.info.id')[1]) 11. Ditto.