Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH 1/1] raft: introduce election_role configuration option
@ 2020-10-06 20:36 Vladislav Shpilevoy
  2020-10-07  9:30 ` Serge Petrenko
  2020-10-12 20:16 ` Vladislav Shpilevoy
  0 siblings, 2 replies; 5+ messages in thread
From: Vladislav Shpilevoy @ 2020-10-06 20:36 UTC (permalink / raw)
  To: tarantool-patches, sergepetrenko

The new option can be one of 3 values: 'off', 'candidate',
'voter'. It replaces 2 old options: election_is_enabled and
election_is_candidate. These flags looked strange, that it was
possible to set candidate true, but disable election at the same
time. Also it would not look good if we would ever decide to
introduce another mode like a data-less sentinel node, for
example. Just for voting.

Anyway, the single option approach looks easier to configure and
to extend.

- 'off' means the election is disabled on the node. It is the same
  as election_is_enabled = false in the old config;

- 'voter' means the node can vote and is never writable. The same
  as election_is_enabled = true + election_is_candidate = false in
  the old config;

- 'candidate' means the node is a full-featured cluster member,
  which eventually may become a leader. The same as
  election_is_enabled = true + election_is_candidate = true in the
  old config.

Part of #1146
---
Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-1146-raft-options
Issue: https://github.com/tarantool/tarantool/issues/1146

 src/box/box.cc                           | 54 +++++++-----------------
 src/box/box.h                            |  3 +-
 src/box/lua/cfg.cc                       | 15 ++-----
 src/box/lua/load_cfg.lua                 | 15 +++----
 test/app-tap/init_script.result          |  3 +-
 test/box/admin.result                    |  6 +--
 test/box/cfg.result                      | 12 ++----
 test/replication/election_basic.result   | 28 ++++--------
 test/replication/election_basic.test.lua | 17 +++-----
 test/replication/election_replica.lua    |  3 +-
 10 files changed, 48 insertions(+), 108 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index 6ec813c12..079fa8b4b 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -477,26 +477,17 @@ box_check_uri(const char *source, const char *option_name)
 	}
 }
 
-static int
-box_check_election_is_enabled(void)
+static const char *
+box_check_election_role(void)
 {
-	int b = cfg_getb("election_is_enabled");
-	if (b < 0) {
-		diag_set(ClientError, ER_CFG, "election_is_enabled",
-			 "the value must be a boolean");
+	const char *role = cfg_gets("election_role");
+	if (role == NULL || (strcmp(role, "off") != 0 &&
+	    strcmp(role, "voter") != 0 && strcmp(role, "candidate") != 0)) {
+		diag_set(ClientError, ER_CFG, "election_role", "the value must "
+			 "be a string 'off' or 'voter' or 'candidate'");
+		return NULL;
 	}
-	return b;
-}
-
-static int
-box_check_election_is_candidate(void)
-{
-	int b = cfg_getb("election_is_candidate");
-	if (b < 0) {
-		diag_set(ClientError, ER_CFG, "election_is_candidate",
-			 "the value must be a boolean");
-	}
-	return b;
+	return role;
 }
 
 static double
@@ -768,9 +759,7 @@ box_check_config(void)
 	box_check_uri(cfg_gets("listen"), "listen");
 	box_check_instance_uuid(&uuid);
 	box_check_replicaset_uuid(&uuid);
-	if (box_check_election_is_enabled() < 0)
-		diag_raise();
-	if (box_check_election_is_candidate() < 0)
+	if (box_check_election_role() == NULL)
 		diag_raise();
 	if (box_check_election_timeout() < 0)
 		diag_raise();
@@ -797,22 +786,13 @@ box_check_config(void)
 }
 
 int
-box_set_election_is_enabled(void)
+box_set_election_role(void)
 {
-	int b = box_check_election_is_enabled();
-	if (b < 0)
+	const char *role = box_check_election_role();
+	if (role == NULL)
 		return -1;
-	raft_cfg_is_enabled(b);
-	return 0;
-}
-
-int
-box_set_election_is_candidate(void)
-{
-	int b = box_check_election_is_candidate();
-	if (b < 0)
-		return -1;
-	raft_cfg_is_candidate(b);
+	raft_cfg_is_candidate(strcmp(role, "candidate") == 0);
+	raft_cfg_is_enabled(strcmp(role, "off") != 0);
 	return 0;
 }
 
@@ -2786,15 +2766,13 @@ box_cfg_xc(void)
 	 * election-enabled node without election actually enabled leading to
 	 * disconnect.
 	 */
-	if (box_set_election_is_candidate() != 0)
-		diag_raise();
 	if (box_set_election_timeout() != 0)
 		diag_raise();
 	/*
 	 * Election is enabled last. So as all the parameters are installed by
 	 * that time.
 	 */
-	if (box_set_election_is_enabled() != 0)
+	if (box_set_election_role() != 0)
 		diag_raise();
 
 	title("running");
diff --git a/src/box/box.h b/src/box/box.h
index 45ff8bbbf..ca66ecff6 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -245,8 +245,7 @@ void box_set_vinyl_memory(void);
 void box_set_vinyl_max_tuple_size(void);
 void box_set_vinyl_cache(void);
 void box_set_vinyl_timeout(void);
-int box_set_election_is_enabled(void);
-int box_set_election_is_candidate(void);
+int box_set_election_role(void);
 int box_set_election_timeout(void);
 void box_set_replication_timeout(void);
 void box_set_replication_connect_timeout(void);
diff --git a/src/box/lua/cfg.cc b/src/box/lua/cfg.cc
index bbb92f038..94a7eea90 100644
--- a/src/box/lua/cfg.cc
+++ b/src/box/lua/cfg.cc
@@ -270,17 +270,9 @@ lbox_cfg_set_worker_pool_threads(struct lua_State *L)
 }
 
 static int
-lbox_cfg_set_election_is_enabled(struct lua_State *L)
+lbox_cfg_set_election_role(struct lua_State *L)
 {
-	if (box_set_election_is_enabled() != 0)
-		luaT_error(L);
-	return 0;
-}
-
-static int
-lbox_cfg_set_election_is_candidate(struct lua_State *L)
-{
-	if (box_set_election_is_candidate() != 0)
+	if (box_set_election_role() != 0)
 		luaT_error(L);
 	return 0;
 }
@@ -406,8 +398,7 @@ box_lua_cfg_init(struct lua_State *L)
 		{"cfg_set_vinyl_max_tuple_size", lbox_cfg_set_vinyl_max_tuple_size},
 		{"cfg_set_vinyl_cache", lbox_cfg_set_vinyl_cache},
 		{"cfg_set_vinyl_timeout", lbox_cfg_set_vinyl_timeout},
-		{"cfg_set_election_is_enabled", lbox_cfg_set_election_is_enabled},
-		{"cfg_set_election_is_candidate", lbox_cfg_set_election_is_candidate},
+		{"cfg_set_election_role", lbox_cfg_set_election_role},
 		{"cfg_set_election_timeout", lbox_cfg_set_election_timeout},
 		{"cfg_set_replication_timeout", lbox_cfg_set_replication_timeout},
 		{"cfg_set_replication_connect_quorum", lbox_cfg_set_replication_connect_quorum},
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index d558e7ac9..b2c3ce188 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -87,8 +87,7 @@ local default_cfg = {
     checkpoint_wal_threshold = 1e18,
     checkpoint_count    = 2,
     worker_pool_threads = 4,
-    election_is_enabled = false,
-    election_is_candidate = true,
+    election_role       = 'off',
     election_timeout    = 5,
     replication_timeout = 1,
     replication_sync_lag = 10,
@@ -168,8 +167,7 @@ local template_cfg = {
     hot_standby         = 'boolean',
     memtx_use_mvcc_engine = 'boolean',
     worker_pool_threads = 'number',
-    election_is_enabled = 'boolean',
-    election_is_candidate = 'boolean',
+    election_role       = 'string',
     election_timeout    = 'number',
     replication_timeout = 'number',
     replication_sync_lag = 'number',
@@ -287,8 +285,7 @@ local dynamic_cfg = {
         require('title').update(box.cfg.custom_proc_title)
     end,
     force_recovery          = function() end,
-    election_is_enabled     = private.cfg_set_election_is_enabled,
-    election_is_candidate   = private.cfg_set_election_is_candidate,
+    election_role           = private.cfg_set_election_role,
     election_timeout        = private.cfg_set_election_timeout,
     replication_timeout     = private.cfg_set_replication_timeout,
     replication_connect_timeout = private.cfg_set_replication_connect_timeout,
@@ -344,8 +341,7 @@ local dynamic_cfg_order = {
     -- the new one. This should be fixed when box.cfg is able to
     -- apply some parameters together and atomically.
     replication_anon        = 250,
-    election_is_enabled     = 300,
-    election_is_candidate   = 310,
+    election_role           = 300,
     election_timeout        = 320,
 }
 
@@ -364,8 +360,7 @@ local dynamic_cfg_skip_at_load = {
     vinyl_cache             = true,
     vinyl_timeout           = true,
     too_long_threshold      = true,
-    election_is_enabled     = true,
-    election_is_candidate   = true,
+    election_role           = true,
     election_timeout        = true,
     replication             = true,
     replication_timeout     = true,
diff --git a/test/app-tap/init_script.result b/test/app-tap/init_script.result
index d8969278b..55b739bc6 100644
--- a/test/app-tap/init_script.result
+++ b/test/app-tap/init_script.result
@@ -8,8 +8,7 @@ checkpoint_count:2
 checkpoint_interval:3600
 checkpoint_wal_threshold:1e+18
 coredump:false
-election_is_candidate:true
-election_is_enabled:false
+election_role:off
 election_timeout:5
 feedback_enabled:true
 feedback_host:https://feedback.tarantool.io
diff --git a/test/box/admin.result b/test/box/admin.result
index 52b62356f..383ffec83 100644
--- a/test/box/admin.result
+++ b/test/box/admin.result
@@ -37,10 +37,8 @@ cfg_filter(box.cfg)
     - 1000000000000000000
   - - coredump
     - false
-  - - election_is_candidate
-    - true
-  - - election_is_enabled
-    - false
+  - - election_role
+    - off
   - - election_timeout
     - 5
   - - feedback_enabled
diff --git a/test/box/cfg.result b/test/box/cfg.result
index f19f4bff7..10c505593 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -25,10 +25,8 @@ cfg_filter(box.cfg)
  |     - 1000000000000000000
  |   - - coredump
  |     - false
- |   - - election_is_candidate
- |     - true
- |   - - election_is_enabled
- |     - false
+ |   - - election_role
+ |     - off
  |   - - election_timeout
  |     - 5
  |   - - feedback_enabled
@@ -140,10 +138,8 @@ cfg_filter(box.cfg)
  |     - 1000000000000000000
  |   - - coredump
  |     - false
- |   - - election_is_candidate
- |     - true
- |   - - election_is_enabled
- |     - false
+ |   - - election_role
+ |     - off
  |   - - election_timeout
  |     - 5
  |   - - feedback_enabled
diff --git a/test/replication/election_basic.result b/test/replication/election_basic.result
index e59386f90..28143124b 100644
--- a/test/replication/election_basic.result
+++ b/test/replication/election_basic.result
@@ -11,25 +11,19 @@ old_election_timeout = box.cfg_election_timeout
  | ...
 
 -- Election is turned off by default.
-assert(not box.cfg.election_is_enabled)
- | ---
- | - true
- | ...
--- Is candidate by default. Although it does not matter, until election is
--- turned on.
-assert(box.cfg.election_is_candidate)
+assert(box.cfg.election_role == 'off')
  | ---
  | - true
  | ...
 -- Ensure election options are validated.
-box.cfg{election_is_enabled = 100}
+box.cfg{election_role = 100}
  | ---
- | - error: 'Incorrect value for option ''election_is_enabled'': should be of type boolean'
+ | - error: 'Incorrect value for option ''election_role'': should be of type string'
  | ...
-box.cfg{election_is_candidate = 100}
+box.cfg{election_role = '100'}
  | ---
- | - error: 'Incorrect value for option ''election_is_candidate'': should be of type
- |     boolean'
+ | - error: 'Incorrect value for option ''election_role'': the value must be a string
+ |     ''off'' or ''voter'' or ''candidate'''
  | ...
 box.cfg{election_timeout = -1}
  | ---
@@ -64,10 +58,7 @@ assert(not box.info.ro)
  | ...
 
 -- Turned on election blocks writes until the instance becomes a leader.
-box.cfg{election_is_candidate = false}
- | ---
- | ...
-box.cfg{election_is_enabled = true}
+box.cfg{election_role = 'voter'}
  | ---
  | ...
 assert(box.info.election.state == 'follower')
@@ -97,7 +88,7 @@ assert(box.info.election.leader == 0)
 box.cfg{election_timeout = 1000}
  | ---
  | ...
-box.cfg{election_is_candidate = true}
+box.cfg{election_role = 'candidate'}
  | ---
  | ...
 test_run:wait_cond(function() return box.info.election.state == 'leader' end)
@@ -118,8 +109,7 @@ assert(box.info.election.leader == box.info.id)
  | ...
 
 box.cfg{                                                                        \
-    election_is_enabled = false,                                                \
-    election_is_candidate = true,                                               \
+    election_role = 'off',                                                      \
     election_timeout = old_election_timeout                                     \
 }
  | ---
diff --git a/test/replication/election_basic.test.lua b/test/replication/election_basic.test.lua
index 506d5ec4e..81f587866 100644
--- a/test/replication/election_basic.test.lua
+++ b/test/replication/election_basic.test.lua
@@ -6,13 +6,10 @@ test_run = require('test_run').new()
 old_election_timeout = box.cfg_election_timeout
 
 -- Election is turned off by default.
-assert(not box.cfg.election_is_enabled)
--- Is candidate by default. Although it does not matter, until election is
--- turned on.
-assert(box.cfg.election_is_candidate)
+assert(box.cfg.election_role == 'off')
 -- Ensure election options are validated.
-box.cfg{election_is_enabled = 100}
-box.cfg{election_is_candidate = 100}
+box.cfg{election_role = 100}
+box.cfg{election_role = '100'}
 box.cfg{election_timeout = -1}
 box.cfg{election_timeout = 0}
 
@@ -25,8 +22,7 @@ assert(box.info.election.leader == 0)
 assert(not box.info.ro)
 
 -- Turned on election blocks writes until the instance becomes a leader.
-box.cfg{election_is_candidate = false}
-box.cfg{election_is_enabled = true}
+box.cfg{election_role = 'voter'}
 assert(box.info.election.state == 'follower')
 assert(box.info.ro)
 -- Term is not changed, because the instance can't be a candidate,
@@ -37,15 +33,14 @@ assert(box.info.election.leader == 0)
 
 -- Candidate instance votes immediately, if sees no leader.
 box.cfg{election_timeout = 1000}
-box.cfg{election_is_candidate = true}
+box.cfg{election_role = 'candidate'}
 test_run:wait_cond(function() return box.info.election.state == 'leader' end)
 assert(box.info.election.term > term)
 assert(box.info.election.vote == box.info.id)
 assert(box.info.election.leader == box.info.id)
 
 box.cfg{                                                                        \
-    election_is_enabled = false,                                                \
-    election_is_candidate = true,                                               \
+    election_role = 'off',                                                      \
     election_timeout = old_election_timeout                                     \
 }
 
diff --git a/test/replication/election_replica.lua b/test/replication/election_replica.lua
index b7d1aebe7..96a01245b 100644
--- a/test/replication/election_replica.lua
+++ b/test/replication/election_replica.lua
@@ -19,8 +19,7 @@ box.cfg({
         instance_uri(3),
     },
     replication_timeout = 0.1,
-    election_is_enabled = true,
-    election_is_candidate = true,
+    election_role = 'candidate',
     election_timeout = ELECTION_TIMEOUT,
     replication_synchro_quorum = SYNCHRO_QUORUM,
     replication_synchro_timeout = 0.1,
-- 
2.21.1 (Apple Git-122.3)

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-12 20:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 20:36 [Tarantool-patches] [PATCH 1/1] raft: introduce election_role configuration option Vladislav Shpilevoy
2020-10-07  9:30 ` Serge Petrenko
2020-10-07 12:20   ` Sergey Ostanevich
2020-10-07 22:29     ` Vladislav Shpilevoy
2020-10-12 20:16 ` Vladislav Shpilevoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox