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

* Re: [Tarantool-patches] [PATCH 1/1] raft: introduce election_role configuration option
  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-12 20:16 ` Vladislav Shpilevoy
  1 sibling, 1 reply; 5+ messages in thread
From: Serge Petrenko @ 2020-10-07  9:30 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches


06.10.2020 23:36, Vladislav Shpilevoy пишет:
> 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
Hi! Thanks for the patch!

I propose to either rename `election_role` to `election_mode` or
`election_role="off"` to `election_role="none"`

Other than that, LGTM.
> ---
> 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,

-- 
Serge Petrenko

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/1] raft: introduce election_role configuration option
  2020-10-07  9:30 ` Serge Petrenko
@ 2020-10-07 12:20   ` Sergey Ostanevich
  2020-10-07 22:29     ` Vladislav Shpilevoy
  0 siblings, 1 reply; 5+ messages in thread
From: Sergey Ostanevich @ 2020-10-07 12:20 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: tarantool-patches, Vladislav Shpilevoy

Hi!

Thank you for the patch and review! 
I vote for 'election_mode' naming, since 'off' means we have election
turend off on this node. The 'voter' means this node behaves in a mode
it votes for others only, the 'candidate' turns into a mode of 
self-nomination. 

LGTM otherwise.

Regards,
Sergos

On 07 окт 12:30, Serge Petrenko wrote:
> 
> 06.10.2020 23:36, Vladislav Shpilevoy пишет:
> > 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
> Hi! Thanks for the patch!
> 
> I propose to either rename `election_role` to `election_mode` or
> `election_role="off"` to `election_role="none"`
> 
> Other than that, LGTM.
> > ---
> > 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,
> 
> -- 
> Serge Petrenko
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/1] raft: introduce election_role configuration option
  2020-10-07 12:20   ` Sergey Ostanevich
@ 2020-10-07 22:29     ` Vladislav Shpilevoy
  0 siblings, 0 replies; 5+ messages in thread
From: Vladislav Shpilevoy @ 2020-10-07 22:29 UTC (permalink / raw)
  To: Sergey Ostanevich, Serge Petrenko; +Cc: tarantool-patches

Hi! Thanks for the review!

I am fine with election_mode. Renamed on the branch.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/1] raft: introduce election_role configuration option
  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-12 20:16 ` Vladislav Shpilevoy
  1 sibling, 0 replies; 5+ messages in thread
From: Vladislav Shpilevoy @ 2020-10-12 20:16 UTC (permalink / raw)
  To: tarantool-patches, sergepetrenko

Pushed to master.

^ 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