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 B54556EC55; Thu, 10 Jun 2021 16:36:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B54556EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1623332192; bh=K0vbQ5ffN3xd8aPiwjD/Q2GaVm8qQkKT3OiGgx5H6ug=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=EIz6SUZaddnnRKkRLwZFR+DH/Xy6knjz/4uyhZEajSvi3rvCvy2OLauccQJ9d9QTo QdYurULlofpTSDDLGxmBORwaWWW+0loqu8wSzqIJlfgMubcesW/XWhXLZl6SXyCghx 2u5rtEf4y+usR21e0/Eq+o0bzHafeDL6wMRR6zuw= Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (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 AA15D6EC70 for ; Thu, 10 Jun 2021 16:33:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AA15D6EC70 Received: by smtp63.i.mail.ru with esmtpa (envelope-from ) id 1lrKo2-0007EB-0B; Thu, 10 Jun 2021 16:33:10 +0300 To: v.shpilevoy@tarantool.org, gorcunov@gmail.com Date: Thu, 10 Jun 2021 16:32:57 +0300 Message-Id: X-Mailer: git-send-email 2.30.1 (Apple Git-130) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9D5B0DA836B685C549A9F97C297FFF2C725C7934AD8E7B4B9182A05F5380850406C47199B169F84516E6603F54C8E7C3DC76C51482EFB35B15D563E286E6EA0C9 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE74E940C28E5656A39EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637DC5FF0CF1FFF13268638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D84BE70FC4FC14EFF03ED7E4F757B9B0F1117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE0AC5B80A05675ACD45B9AC499A3C791CD8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE35FF72824B19451C603F1AB874ED89028C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F79006371F24DFF1B2961425731C566533BA786AA5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A24209795067102C07E8F7B195E1C97831B61BC0640769A5B1E78E5B5B57A3F822 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8183A4AFAF3EA6BDC44C234C8B12C006B7AFC014AD2D9E6C96F5C233F111ED9AA3F3B7CBBE7B7839779B1881A6453793CE9C32612AADDFBE061C61BE10805914D3804EBA3D8E7E5B87ABF8C51168CD8EBDBAB5495298CADBD05DC48ACC2A39D04F89CDFB48F4795C241BDAD6C7F3747799A X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3475AE8838CC1BF58E959A9A70CC29030D682C67D0E763E3DBF6B3DFF01998DEE3061A153CC62F23EE1D7E09C32AA3244C22995EB8BC9D99054E369ACF7FAD4372795D98D676DD64D0927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXiiA81jxGMgNpWwanvUC3R8 X-Mailru-Sender: 583F1D7ACE8F49BD9DF7A8DAE6E2B08A8104EB92363019CF48C6D1E840BAAC7777D43CB88E2C6613424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: [Tarantool-patches] [PATCH 7/7] box: make promote/demote 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 Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" When called without elections, promote and demote resulted in multiple PROMOTE/DEMOTE entries for the same term. This is not right, because all the promotions for the same term except the first one would be ignored as already seen. Follow-up #6034 --- src/box/box.cc | 14 +- src/lib/raft/raft.c | 3 +- .../gh-6034-limbo-ownership.result | 186 ++++++++++++++++++ .../gh-6034-limbo-ownership.test.lua | 68 +++++++ .../gh-6034-promote-bump-term.result | 51 +++++ .../gh-6034-promote-bump-term.test.lua | 20 ++ test/replication/suite.cfg | 2 + 7 files changed, 336 insertions(+), 8 deletions(-) create mode 100644 test/replication/gh-6034-limbo-ownership.result create mode 100644 test/replication/gh-6034-limbo-ownership.test.lua create mode 100644 test/replication/gh-6034-promote-bump-term.result create mode 100644 test/replication/gh-6034-promote-bump-term.test.lua diff --git a/src/box/box.cc b/src/box/box.cc index d3908b4cd..59f8dddae 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -1663,14 +1663,16 @@ box_clear_synchro_queue(bool demote) rc = -1; } else { promote: - /* We cannot possibly get here in a volatile state. */ - assert(box_raft()->volatile_term == box_raft()->term); + if (try_wait) + raft_new_term(box_raft()); + + uint64_t term = box_raft()->volatile_term; if (demote) { txn_limbo_write_demote(&txn_limbo, wait_lsn, - box_raft()->term); + term); } else { txn_limbo_write_promote(&txn_limbo, wait_lsn, - box_raft()->term); + term); } uint16_t type = demote ? IPROTO_DEMOTE : IPROTO_PROMOTE; struct synchro_request req = { @@ -1678,7 +1680,7 @@ 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)); @@ -3511,7 +3513,7 @@ box_cfg_xc(void) if (!is_bootstrap_leader) { replicaset_sync(); - } else { + } else if (raft_is_enabled(box_raft())) { /* * When the cluster is just bootstrapped and this instance is a * leader, it makes no sense to wait for a leader appearance. diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c index 247c7a71a..1de05727f 100644 --- a/src/lib/raft/raft.c +++ b/src/lib/raft/raft.c @@ -983,8 +983,7 @@ raft_cfg_vclock(struct raft *raft, const struct vclock *vclock) void raft_new_term(struct raft *raft) { - if (raft->is_enabled) - raft_sm_schedule_new_term(raft, raft->volatile_term + 1); + raft_sm_schedule_new_term(raft, raft->volatile_term + 1); } static void diff --git a/test/replication/gh-6034-limbo-ownership.result b/test/replication/gh-6034-limbo-ownership.result new file mode 100644 index 000000000..ebedc2b17 --- /dev/null +++ b/test/replication/gh-6034-limbo-ownership.result @@ -0,0 +1,186 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... +fiber = require('fiber') + | --- + | ... + +-- +-- gh-6034: test that transactional limbo isn't accessible without a promotion. +-- +synchro_quorum = box.cfg.replication_synchro_quorum + | --- + | ... +election_mode = box.cfg.election_mode + | --- + | ... +box.cfg{replication_synchro_quorum = 1, election_mode='off'} + | --- + | ... + +_ = box.schema.space.create('async'):create_index('pk') + | --- + | ... +_ = box.schema.space.create('sync', {is_sync=true}):create_index('pk') + | --- + | ... + +-- Limbo is initially unclaimed, everyone is writeable. +assert(not box.info.ro) + | --- + | - true + | ... +assert(box.info.synchro.queue.owner == 0) + | --- + | - true + | ... +box.space.async:insert{1} -- success. + | --- + | - [1] + | ... +-- Synchro spaces aren't writeable +box.space.sync:insert{1} -- error. + | --- + | - error: Synchronous transaction limbo doesn't belong to any instance + | ... + +box.ctl.promote() + | --- + | ... +assert(not box.info.ro) + | --- + | - true + | ... +assert(box.info.synchro.queue.owner == box.info.id) + | --- + | - true + | ... +box.space.sync:insert{1} -- success. + | --- + | - [1] + | ... + +-- Everyone but the limbo owner is read-only. +box.schema.user.grant('guest', 'replication') + | --- + | ... +test_run:cmd('create server replica with rpl_master=default,\ + script="replication/replica.lua"') + | --- + | - true + | ... +test_run:cmd('start server replica with wait=True, wait_load=True') + | --- + | - true + | ... +test_run:cmd('set variable rpl_listen to "replica.listen"') + | --- + | - true + | ... +orig_replication = box.cfg.replication + | --- + | ... +box.cfg{replication={box.info.listen, rpl_listen}} + | --- + | ... + +test_run:switch('replica') + | --- + | - true + | ... +assert(box.info.ro) + | --- + | - true + | ... +assert(box.info.synchro.queue.owner == test_run:eval('default', 'return box.info.id')[1]) + | --- + | - 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]) + | --- + | - true + | ... +box.space.sync:insert{3} -- failure. + | --- + | - error: Can't modify data because this instance is in read-only mode. + | ... + +box.ctl.demote() + | --- + | ... +assert(not box.info.ro) + | --- + | - true + | ... +box.space.sync:insert{3} -- still fails. + | --- + | - error: Synchronous transaction limbo doesn't belong to any instance + | ... +assert(box.info.synchro.queue.owner == 0) + | --- + | - true + | ... +box.space.async:insert{3} -- success. + | --- + | - [3] + | ... + +-- Cleanup. +box.ctl.demote() + | --- + | ... +test_run:cmd('stop server replica') + | --- + | - true + | ... +test_run:cmd('delete server replica') + | --- + | - true + | ... +box.schema.user.revoke('guest', 'replication') + | --- + | ... +box.space.sync:drop() + | --- + | ... +box.space.async:drop() + | --- + | ... +box.cfg{\ + replication_synchro_quorum = synchro_quorum,\ + election_mode = election_mode,\ + replication = orig_replication,\ +} + | --- + | ... diff --git a/test/replication/gh-6034-limbo-ownership.test.lua b/test/replication/gh-6034-limbo-ownership.test.lua new file mode 100644 index 000000000..0e1586566 --- /dev/null +++ b/test/replication/gh-6034-limbo-ownership.test.lua @@ -0,0 +1,68 @@ +test_run = require('test_run').new() +fiber = require('fiber') + +-- +-- gh-6034: test that transactional limbo isn't accessible without a promotion. +-- +synchro_quorum = box.cfg.replication_synchro_quorum +election_mode = box.cfg.election_mode +box.cfg{replication_synchro_quorum = 1, election_mode='off'} + +_ = box.schema.space.create('async'):create_index('pk') +_ = box.schema.space.create('sync', {is_sync=true}):create_index('pk') + +-- Limbo is initially unclaimed, everyone is writeable. +assert(not box.info.ro) +assert(box.info.synchro.queue.owner == 0) +box.space.async:insert{1} -- success. +-- Synchro spaces aren't writeable +box.space.sync:insert{1} -- error. + +box.ctl.promote() +assert(not box.info.ro) +assert(box.info.synchro.queue.owner == box.info.id) +box.space.sync:insert{1} -- success. + +-- Everyone but the limbo owner is read-only. +box.schema.user.grant('guest', 'replication') +test_run:cmd('create server replica with rpl_master=default,\ + script="replication/replica.lua"') +test_run:cmd('start server replica with wait=True, wait_load=True') +test_run:cmd('set variable rpl_listen to "replica.listen"') +orig_replication = box.cfg.replication +box.cfg{replication={box.info.listen, rpl_listen}} + +test_run:switch('replica') +assert(box.info.ro) +assert(box.info.synchro.queue.owner == test_run:eval('default', 'return box.info.id')[1]) +box.space.async:insert{2} -- failure. + +-- Promotion on the other node. Default should become ro. +box.ctl.promote() +assert(not box.info.ro) +assert(box.info.synchro.queue.owner == box.info.id) +box.space.sync:insert{2} -- success. + +test_run:switch('default') +assert(box.info.ro) +assert(box.info.synchro.queue.owner == test_run:eval('replica', 'return box.info.id')[1]) +box.space.sync:insert{3} -- failure. + +box.ctl.demote() +assert(not box.info.ro) +box.space.sync:insert{3} -- still fails. +assert(box.info.synchro.queue.owner == 0) +box.space.async:insert{3} -- success. + +-- Cleanup. +box.ctl.demote() +test_run:cmd('stop server replica') +test_run:cmd('delete server replica') +box.schema.user.revoke('guest', 'replication') +box.space.sync:drop() +box.space.async:drop() +box.cfg{\ + replication_synchro_quorum = synchro_quorum,\ + election_mode = election_mode,\ + replication = orig_replication,\ +} diff --git a/test/replication/gh-6034-promote-bump-term.result b/test/replication/gh-6034-promote-bump-term.result new file mode 100644 index 000000000..d3cbbc1c3 --- /dev/null +++ b/test/replication/gh-6034-promote-bump-term.result @@ -0,0 +1,51 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... + +-- gh-6034 follow-up: test that every box.ctl.promote()/box.ctl.demote() bumps +-- the instance's term. Even when elections are disabled. Even for consequent +-- promotes/demotes on the same instance. +election_mode = box.cfg.election_mode + | --- + | ... +box.cfg{election_mode='off'} + | --- + | ... + +term = box.info.election.term + | --- + | ... +box.ctl.promote() + | --- + | ... +assert(box.info.election.term == term + 1) + | --- + | - true + | ... +box.ctl.promote() + | --- + | ... +assert(box.info.election.term == term + 2) + | --- + | - true + | ... +box.ctl.demote() + | --- + | ... +assert(box.info.election.term == term + 3) + | --- + | - true + | ... +box.ctl.demote() + | --- + | ... +assert(box.info.election.term == term + 4) + | --- + | - true + | ... + +-- Cleanup. +box.cfg{election_mode=election_mode} + | --- + | ... diff --git a/test/replication/gh-6034-promote-bump-term.test.lua b/test/replication/gh-6034-promote-bump-term.test.lua new file mode 100644 index 000000000..a6a082ed5 --- /dev/null +++ b/test/replication/gh-6034-promote-bump-term.test.lua @@ -0,0 +1,20 @@ +test_run = require('test_run').new() + +-- gh-6034 follow-up: test that every box.ctl.promote()/box.ctl.demote() bumps +-- the instance's term. Even when elections are disabled. Even for consequent +-- promotes/demotes on the same instance. +election_mode = box.cfg.election_mode +box.cfg{election_mode='off'} + +term = box.info.election.term +box.ctl.promote() +assert(box.info.election.term == term + 1) +box.ctl.promote() +assert(box.info.election.term == term + 2) +box.ctl.demote() +assert(box.info.election.term == term + 3) +box.ctl.demote() +assert(box.info.election.term == term + 4) + +-- Cleanup. +box.cfg{election_mode=election_mode} diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg index 46de2e6c4..9444ec49a 100644 --- a/test/replication/suite.cfg +++ b/test/replication/suite.cfg @@ -46,6 +46,8 @@ "gh-5536-wal-limit.test.lua": {}, "gh-5566-final-join-synchro.test.lua": {}, "gh-6032-promote-wal-write.test.lua": {}, + "gh-6034-limbo-ownership.test.lua": {}, + "gh-6034-promote-bump-term.test.lua": {}, "gh-6057-qsync-confirm-async-no-wal.test.lua": {}, "gh-6094-rs-uuid-mismatch.test.lua": {}, "*": { -- 2.30.1 (Apple Git-130)