[Tarantool-patches] [PATCH 1/1] raft: introduce election_role configuration option
Serge Petrenko
sergepetrenko at tarantool.org
Wed Oct 7 12:30:08 MSK 2020
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
More information about the Tarantool-patches
mailing list