Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com,
	sergepetrenko@tarantool.org
Subject: [Tarantool-patches] [PATCH 2/4] raft: initialize raft completely at runtime
Date: Sun,  8 Nov 2020 19:03:55 +0100	[thread overview]
Message-ID: <5226a32c8783879dbf1b3714730b87147767466f.1604858551.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1604858551.git.v.shpilevoy@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 <stdint.h>
 #include <stdbool.h>
+#include <assert.h>
 #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)

  parent reply	other threads:[~2020-11-08 18:04 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-08 18:03 [Tarantool-patches] [PATCH 0/4] Raft module, part 1 - explicit argument Vladislav Shpilevoy
2020-11-08 18:03 ` [Tarantool-patches] [PATCH 1/4] fiber: introduce fiber.arg Vladislav Shpilevoy
2020-11-09 12:49   ` Serge Petrenko
2020-11-09 14:27   ` Cyrill Gorcunov
2020-11-09 23:59     ` Vladislav Shpilevoy
2020-11-08 18:03 ` Vladislav Shpilevoy [this message]
2020-11-09 12:50   ` [Tarantool-patches] [PATCH 2/4] raft: initialize raft completely at runtime Serge Petrenko
2020-11-09 14:59   ` Cyrill Gorcunov
2020-11-09 23:59     ` Vladislav Shpilevoy
2020-11-08 18:03 ` [Tarantool-patches] [PATCH 3/4] raft: add explicit raft argument to all functions Vladislav Shpilevoy
2020-11-09 13:46   ` Serge Petrenko
2020-11-10  0:00     ` Vladislav Shpilevoy
2020-11-08 18:03 ` [Tarantool-patches] [PATCH 4/4] vclock: move to src/lib Vladislav Shpilevoy
2020-11-09 14:12   ` Serge Petrenko
2020-11-09 15:01   ` Cyrill Gorcunov
2020-11-10 21:06 ` [Tarantool-patches] [PATCH 0/4] Raft module, part 1 - explicit argument Alexander V. Tikhonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5226a32c8783879dbf1b3714730b87147767466f.1604858551.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergepetrenko@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2/4] raft: initialize raft completely at runtime' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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