From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 8255D446439 for ; Sun, 8 Nov 2020 21:04:01 +0300 (MSK) From: Vladislav Shpilevoy Date: Sun, 8 Nov 2020 19:03:55 +0100 Message-Id: <5226a32c8783879dbf1b3714730b87147767466f.1604858551.git.v.shpilevoy@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 2/4] raft: initialize raft completely at runtime List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergepetrenko@tarantool.org Global raft object was created mostly at compile time with constant values assigned to all raft fields. But it won't work soon because raft will be moved to a separate library, and it will provide methods for raft creation and destruction. This patch makes raft not rely on compile time initialization, and moves it entirely to raft_init(). Also there are added some validations via new raft_validate() function, which should ensure raft is not used before creation. This may be hard to support, since raft is used by recovery and replication, even before instance has an ID. The validation should help to catch bugs related to the initialization order of these subsystems. Part of #5303 --- src/box/raft.c | 37 ++++++++++++++++++++++--------------- src/box/raft.h | 14 ++++++++++++++ 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/box/raft.c b/src/box/raft.c index 8f8b59ba6..a6a893373 100644 --- a/src/box/raft.c +++ b/src/box/raft.c @@ -46,21 +46,11 @@ /** Raft state of this instance. */ struct raft raft = { - .leader = 0, - .state = RAFT_STATE_FOLLOWER, - .volatile_term = 1, - .volatile_vote = 0, - .is_enabled = false, - .is_candidate = false, - .is_cfg_candidate = false, - .is_write_in_progress = false, - .is_broadcast_scheduled = false, - .term = 1, - .vote = 0, - .vote_mask = 0, - .vote_count = 0, - .worker = NULL, - .election_timeout = 5, + /* + * Set an invalid state to validate in all raft functions they are not + * used before raft initialization. + */ + .state = 0, }; /** @@ -325,6 +315,7 @@ raft_request_to_string(const struct raft_request *req) void raft_process_recovery(const struct raft_request *req) { + raft_validate(); say_verbose("RAFT: recover %s", raft_request_to_string(req)); if (req->term != 0) { raft.term = req->term; @@ -353,6 +344,7 @@ raft_process_recovery(const struct raft_request *req) int raft_process_msg(const struct raft_request *req, uint32_t source) { + raft_validate(); say_info("RAFT: message %s from %u", raft_request_to_string(req), source); assert(source > 0); @@ -496,6 +488,7 @@ raft_process_msg(const struct raft_request *req, uint32_t source) void raft_process_heartbeat(uint32_t source) { + raft_validate(); /* * Raft handles heartbeats from all instances, including anon instances * which don't participate in Raft. @@ -905,6 +898,7 @@ raft_sm_stop(void) void raft_serialize_for_network(struct raft_request *req, struct vclock *vclock) { + raft_validate(); memset(req, 0, sizeof(*req)); /* * Volatile state is never used for any communications. @@ -926,6 +920,7 @@ raft_serialize_for_network(struct raft_request *req, struct vclock *vclock) void raft_serialize_for_disk(struct raft_request *req) { + raft_validate(); memset(req, 0, sizeof(*req)); req->term = raft.term; req->vote = raft.vote; @@ -934,12 +929,14 @@ raft_serialize_for_disk(struct raft_request *req) void raft_on_update(struct trigger *trigger) { + raft_validate(); trigger_add(&raft.on_update, trigger); } void raft_cfg_is_enabled(bool is_enabled) { + raft_validate(); if (is_enabled == raft.is_enabled) return; @@ -952,6 +949,7 @@ raft_cfg_is_enabled(bool is_enabled) void raft_cfg_is_candidate(bool is_candidate) { + raft_validate(); bool old_is_candidate = raft.is_candidate; raft.is_cfg_candidate = is_candidate; raft.is_candidate = is_candidate && raft.is_enabled; @@ -991,6 +989,7 @@ raft_cfg_is_candidate(bool is_candidate) void raft_cfg_election_timeout(double timeout) { + raft_validate(); if (timeout == raft.election_timeout) return; @@ -1008,6 +1007,7 @@ raft_cfg_election_timeout(double timeout) void raft_cfg_election_quorum(void) { + raft_validate(); if (raft.state != RAFT_STATE_CANDIDATE || raft.state == RAFT_STATE_LEADER) return; @@ -1019,6 +1019,7 @@ raft_cfg_election_quorum(void) void raft_cfg_death_timeout(void) { + raft_validate(); if (raft.state == RAFT_STATE_FOLLOWER && raft.is_candidate && raft.leader != 0) { assert(ev_is_active(&raft.timer)); @@ -1034,6 +1035,7 @@ raft_cfg_death_timeout(void) void raft_new_term(void) { + raft_validate(); if (raft.is_enabled) raft_sm_schedule_new_term(raft.volatile_term + 1); } @@ -1079,6 +1081,11 @@ raft_schedule_broadcast(void) void raft_init(void) { + memset(&raft, 0, sizeof(raft)); + raft.state = RAFT_STATE_FOLLOWER; + raft.volatile_term = 1; + raft.term = 1; + raft.election_timeout = 5; ev_timer_init(&raft.timer, raft_sm_schedule_new_election_cb, 0, 0); rlist_create(&raft.on_update); } diff --git a/src/box/raft.h b/src/box/raft.h index 8293d7410..0c60eccdf 100644 --- a/src/box/raft.h +++ b/src/box/raft.h @@ -31,6 +31,7 @@ */ #include #include +#include #include "tarantool_ev.h" #include "trigger.h" @@ -164,6 +165,16 @@ struct raft { extern struct raft raft; +/** + * Ensure the raft node can be used. I.e. that it is properly initialized. + * Entirely for debug purposes. + */ +static inline void +raft_validate(void) +{ + assert(raft.state != 0); +} + /** * A flag whether the instance is read-only according to Raft. Even if Raft * allows writes though, it does not mean the instance is writable. It can be @@ -172,6 +183,7 @@ extern struct raft raft; static inline bool raft_is_ro(void) { + raft_validate(); return raft.is_enabled && raft.state != RAFT_STATE_LEADER; } @@ -179,6 +191,7 @@ raft_is_ro(void) static inline bool raft_is_source_allowed(uint32_t source_id) { + raft_validate(); return !raft.is_enabled || raft.leader == source_id; } @@ -186,6 +199,7 @@ raft_is_source_allowed(uint32_t source_id) static inline bool raft_is_enabled(void) { + raft_validate(); return raft.is_enabled; } -- 2.21.1 (Apple Git-122.3)