From 05d03a1c58d9fb5b46078fefbe2a3049df39d052 Mon Sep 17 00:00:00 2001
From: Georgiy Lebedev <g.lebedev@tarantool.org>
Date: Fri, 24 May 2024 12:12:22 +0300
Subject: [PATCH] box: prevent demoted leader from being a candidate in the
next elections
Currently, the demoted leader sees that nobody has requested a vote in the
newly persisted term (because it has just written it without voting, and
nobody had time to see the new term yet), and hence votes for itself,
becoming the most probable winner of the next elections.
To prevent this from happening, let's forbid the demoted leader to be a
candidate in the next elections using `box_raft_leader_step_off`.
Closes #9855
NO_DOC=<bugfix>
Co-authored-by: Serge Petrenko <sergepetrenko@tarantool.org>
---
...-9855-box-ctl-demote-guarantee-demoting.md | 5 ++
src/box/box.cc | 5 +-
src/box/raft.c | 8 +-
src/box/raft.h | 9 +++
.../gh_6033_box_promote_demote_test.lua | 5 ++
.../gh_6860_election_off_demote_test.lua | 13 ++++
...box_ctl_demote_guarantee_demoting_test.lua | 77 +++++++++++++++++++
7 files changed, 112 insertions(+), 10 deletions(-)
create mode 100644 changelogs/unreleased/gh-9855-box-ctl-demote-guarantee-demoting.md
create mode 100644 test/replication-luatest/gh_9855_box_ctl_demote_guarantee_demoting_test.lua
diff --git a/changelogs/unreleased/gh-9855-box-ctl-demote-guarantee-demoting.md b/changelogs/unreleased/gh-9855-box-ctl-demote-guarantee-demoting.md
new file mode 100644
index 0000000000..44cdc4cfd8
--- /dev/null
+++ b/changelogs/unreleased/gh-9855-box-ctl-demote-guarantee-demoting.md
@@ -0,0 +1,5 @@
+## bugfix/replication
+
+* Fixed a bug that allowed the old leader in
+ `box.cfg{election_mode = 'candidate'` mode to get re-elected after resigning
+ himself through `box.ctl.demote` (gh-9855).
diff --git a/src/box/box.cc b/src/box/box.cc
index 22d957b81c..8a0a1386b3 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -3086,9 +3086,8 @@ box_demote(void)
return 0;
if (txn_limbo.owner_id != instance_id)
return 0;
- if (raft->state != RAFT_STATE_LEADER)
- return 0;
- return box_trigger_elections();
+ box_raft_leader_step_off();
+ return 0;
}
assert(raft->state == RAFT_STATE_FOLLOWER);
diff --git a/src/box/raft.c b/src/box/raft.c
index b5b97efe00..0f1282e7ad 100644
--- a/src/box/raft.c
+++ b/src/box/raft.c
@@ -302,13 +302,7 @@ box_raft_fence(void)
raft_resign(raft);
}
-/**
- * Resign RAFT leadership and freeze limbo regardless of
- * box_election_fencing_mode. It waits until the elections
- * begin. After the death-timeout expires, it starts a new
- * round of elections.
- */
-static void
+void
box_raft_leader_step_off(void)
{
struct raft *raft = box_raft();
diff --git a/src/box/raft.h b/src/box/raft.h
index 41611d27fc..edcdb165d1 100644
--- a/src/box/raft.h
+++ b/src/box/raft.h
@@ -152,6 +152,15 @@ box_raft_set_election_fencing_mode(enum election_fencing_mode mode);
void
box_raft_election_fencing_pause(void);
+/**
+ * Resign RAFT leadership and freeze limbo regardless of
+ * box_election_fencing_mode. It waits until the elections
+ * begin. After the death-timeout expires, it starts a new
+ * round of elections.
+ */
+void
+box_raft_leader_step_off(void);
+
void
box_raft_init(void);
diff --git a/test/replication-luatest/gh_6033_box_promote_demote_test.lua b/test/replication-luatest/gh_6033_box_promote_demote_test.lua
index fa1d0455b6..9b082f3097 100644
--- a/test/replication-luatest/gh_6033_box_promote_demote_test.lua
+++ b/test/replication-luatest/gh_6033_box_promote_demote_test.lua
@@ -108,6 +108,11 @@ end
local function cluster_reinit(g)
box_cfg_update(g.cluster.servers, g.box_cfg)
wait_sync(g.cluster.servers)
+ for _, server in ipairs(g.cluster.servers) do
+ server:exec(function()
+ box.ctl.demote()
+ end)
+ end
end
local function cluster_stop(g)
diff --git a/test/replication-luatest/gh_6860_election_off_demote_test.lua b/test/replication-luatest/gh_6860_election_off_demote_test.lua
index 791ab27ef4..51081044b6 100644
--- a/test/replication-luatest/gh_6860_election_off_demote_test.lua
+++ b/test/replication-luatest/gh_6860_election_off_demote_test.lua
@@ -15,6 +15,7 @@ local function replicaset_create(g)
server.build_listen_uri('server2', g.replicaset.id),
},
}
+ g.replication = box_cfg.replication
g.server1 = g.replicaset:build_and_add_server({
alias = 'server1', box_cfg = box_cfg
})
@@ -111,10 +112,22 @@ g.test_election_off_demote_other_no_leader = function(g)
end)
-- Server-2 sees that the queue is owned by server-1.
g.server2:wait_for_synchro_queue_term(g.server1:get_election_term())
+ g.server2:update_box_cfg{replication = ''}
g.server1:exec(function()
box.ctl.demote()
+ -- Bump the election term while leaving the synchro queue term intact.
+ local msg = "Not enough peers connected to start elections: 1 out " ..
+ "of minimal required 2"
+ t.assert_error_msg_content_equals(msg, box.ctl.promote)
+ t.assert_equals(box.info.election.term, box.info.synchro.queue.term + 1)
box.cfg{election_mode = 'off'}
end)
+ g.server2:update_box_cfg({replication = g.replication})
+ g.server2:exec(function()
+ t.helpers.retrying({timeout = 10, delay = 0.5}, function()
+ t.assert_not_equals(box.info.status, "orphan")
+ end)
+ end)
-- Server-2 sees that server-1 is no longer a leader. But the queue still
-- belongs to the latter.
g.server2:wait_for_election_term(g.server1:get_election_term())
diff --git a/test/replication-luatest/gh_9855_box_ctl_demote_guarantee_demoting_test.lua b/test/replication-luatest/gh_9855_box_ctl_demote_guarantee_demoting_test.lua
new file mode 100644
index 0000000000..cbac5824ca
--- /dev/null
+++ b/test/replication-luatest/gh_9855_box_ctl_demote_guarantee_demoting_test.lua
@@ -0,0 +1,77 @@
+local t = require('luatest')
+local replica_set = require('luatest.replica_set')
+local server = require('luatest.server')
+
+local g = t.group()
+
+g.before_each(function(cg)
+ cg.replica_set = replica_set:new{}
+ local box_cfg = {
+ replication = {
+ server.build_listen_uri('server1', cg.replica_set.id),
+ server.build_listen_uri('server2', cg.replica_set.id),
+ },
+ election_mode = 'candidate',
+ replication_timeout = 0.1,
+ election_timeout = 0.5,
+ }
+ cg.server1 = cg.replica_set:build_and_add_server{
+ alias = 'server1',
+ box_cfg = box_cfg,
+ }
+ box_cfg.election_mode = 'voter'
+ cg.server2 = cg.replica_set:build_and_add_server{
+ alias = 'server2',
+ box_cfg = box_cfg,
+ }
+ cg.replica_set:start()
+ cg.replica_set:wait_for_fullmesh()
+end)
+
+g.after_each(function(cg)
+ cg.replica_set:drop()
+end)
+
+-- Test that the demoted leader can still win in subsequent elections.
+local function test_subsequent_elections(demoted_leader, current_leader)
+ current_leader:exec(function()
+ box.ctl.demote()
+ end)
+ demoted_leader:wait_for_election_leader();
+ demoted_leader:exec(function()
+ t.assert_equals(box.info.election.vote, box.info.election.leader)
+ end)
+ current_leader:exec(function()
+ t.assert_equals(box.info.election.vote, box.info.election.leader)
+ end)
+end
+
+-- Test that the demoted leader cannot get elected after resigning himself
+-- through `box.ctl.demote`.
+g.test_demote_guarantee = function(cg)
+ cg.server1:wait_for_election_leader()
+ cg.server2:update_box_cfg({election_mode = 'candidate'})
+ cg.server1:exec(function()
+ box.ctl.demote()
+ end)
+ cg.server2:wait_for_election_leader();
+ cg.server1:exec(function()
+ t.assert_equals(box.info.election.vote, box.info.election.leader)
+ end)
+ test_subsequent_elections(cg.server1, cg.server2)
+end
+
+-- Test that the demoted leader waits for leader death timeout after resigning
+-- himself through `box.ctl.demote` and starts subsequent elections.
+g.test_demoted_leader_election_in_subsequent_elections = function(cg)
+ cg.server1:wait_for_election_leader()
+ local demoted_term = cg.server1:get_election_term()
+ cg.server1:exec(function()
+ box.ctl.demote()
+ end)
+ cg.server1:wait_for_election_leader()
+ cg.server1:exec(function(demoted_term)
+ t.assert_equals(box.info.election.term, demoted_term + 1)
+ t.assert_equals(box.info.election.vote, box.info.election.leader)
+ end, {demoted_term})
+end
--
GitLab