[Tarantool-patches] [PATCH v3] raft: change request state to uint64_t

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Thu, Jul 08, 2021 at 01:05:00AM +0300, Cyrill Gorcunov wrote:
> On Thu, Jul 08, 2021 at 12:59:33AM +0300, Cyrill Gorcunov wrote:
> > > 2. You deleted the state decode. I assume not a single replication
> > > test passes now, correct?
> > 
> > Nope :) I write it back a bit later once verification is complete.
> > I ran the test locally before sending the patch.
> 
> Vlad, I think I know what happened: there was a typo which I fixed
> and commited locally but didn't force pushed out. Just realized it.
> I updated repo. But since we gonna switch to uintX instead of enum
> I think it doesn't worth to resend the patch.

I force pushed an update
---
From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Fri, 25 Jun 2021 13:03:12 +0300
Subject: [PATCH v3] raft: change request state to uint64_t

When new raft message comes in from the network we need
to be sure that the payload is suitable for processing,
in particular `raft_msg::state` must be valid because
our code logic depends on it.

For this sake make `raft_msg::state` being uint64_t
which allows to an easier processing of the state
field verification.

Same time use panic() instead of unreacheable() macro
because the test for valid state must be enabled all
the time.

Closes #6067

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/xrow.h        |  2 +-
 src/lib/raft/raft.c   |  5 +++--
 src/lib/raft/raft.h   |  4 ++--
 test/unit/raft.c      | 10 +++++++++-
 test/unit/raft.result |  4 +++-
 5 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/box/xrow.h b/src/box/xrow.h
index 055fd31e1..9f61bfd47 100644
--- a/src/box/xrow.h
+++ b/src/box/xrow.h
@@ -288,7 +288,7 @@ xrow_decode_synchro(const struct xrow_header *row, struct synchro_request *req);
 struct raft_request {
 	uint64_t term;
 	uint32_t vote;
-	uint32_t state;
+	uint64_t state;
 	const struct vclock *vclock;
 };
 
diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c
index eacdddb7e..ef11ef89f 100644
--- a/src/lib/raft/raft.c
+++ b/src/lib/raft/raft.c
@@ -44,7 +44,7 @@
  * a valid data incomes.
  */
 const char *
-raft_state_str(uint32_t state)
+raft_state_str(uint64_t state)
 {
 	static const char *str[] = {
 		[0]			= "invalid (0)",
@@ -406,7 +406,8 @@ raft_process_msg(struct raft *raft, const struct raft_msg *req, uint32_t source)
 			raft_sm_become_leader(raft);
 			break;
 		default:
-			unreachable();
+			panic("RAFT: unreacheable state hit");
+			break;
 		}
 	}
 	if (req->state != RAFT_STATE_LEADER) {
diff --git a/src/lib/raft/raft.h b/src/lib/raft/raft.h
index fae30b03d..223e0509f 100644
--- a/src/lib/raft/raft.h
+++ b/src/lib/raft/raft.h
@@ -92,7 +92,7 @@ enum raft_state {
  * Decode raft state into string representation.
  */
 const char *
-raft_state_str(uint32_t state);
+raft_state_str(uint64_t state);
 
 /**
  * Basic Raft communication unit for talking to other nodes, and even to other
@@ -110,7 +110,7 @@ struct raft_msg {
 	 * State of the instance. Can be 0 if the state does not matter for the
 	 * message. For instance, when the message is sent to disk.
 	 */
-	enum raft_state state;
+	uint64_t state;
 	/**
 	 * Vclock of the instance. Can be NULL, if the node is not a candidate.
 	 * Also is omitted when does not matter (when the message is for disk).
diff --git a/test/unit/raft.c b/test/unit/raft.c
index 6369c42d3..b9bc49b78 100644
--- a/test/unit/raft.c
+++ b/test/unit/raft.c
@@ -247,7 +247,7 @@ raft_test_recovery(void)
 static void
 raft_test_bad_msg(void)
 {
-	raft_start_test(9);
+	raft_start_test(11);
 	struct raft_msg msg;
 	struct raft_node node;
 	struct vclock vclock;
@@ -294,6 +294,14 @@ raft_test_bad_msg(void)
 	is(raft_node_process_msg(&node, &msg, 2), -1, "bad state");
 	is(node.raft.term, 1, "term from the bad message wasn't used");
 
+	msg = (struct raft_msg){
+		.state = -1,
+		.term = 10,
+		.vote = 2,
+	};
+	is(raft_node_process_msg(&node, &msg, 2), -1, "bad negative state");
+	is(node.raft.term, 1, "term from the bad message wasn't used");
+
 	raft_node_destroy(&node);
 	raft_finish_test();
 }
diff --git a/test/unit/raft.result b/test/unit/raft.result
index 598a7e760..f89cd1658 100644
--- a/test/unit/raft.result
+++ b/test/unit/raft.result
@@ -45,7 +45,7 @@ ok 1 - subtests
 ok 2 - subtests
 	*** raft_test_recovery: done ***
 	*** raft_test_bad_msg ***
-    1..9
+    1..11
     ok 1 - state can't be 0
     ok 2 - term from the bad message wasn't used
     ok 3 - node can't be a candidate but vote for another node
@@ -55,6 +55,8 @@ ok 2 - subtests
     ok 7 - term can't be 0
     ok 8 - bad state
     ok 9 - term from the bad message wasn't used
+    ok 10 - bad negative state
+    ok 11 - term from the bad message wasn't used
 ok 3 - subtests
 	*** raft_test_bad_msg: done ***
 	*** raft_test_vote ***
-- 
2.31.1