Re: [Tarantool-patches] [PATCH v2 09/11] raft: introduce state machine

[Serge Petrenko <sergepetrenko@tarantool.org>]

19.09.2020 18:50, Vladislav Shpilevoy пишет:
> And my new changes on top of it, in a separate commit.
>
> ====================
>
> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index c352faf5e..10186ab91 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -885,6 +885,11 @@ static int
>   applier_handle_raft(struct applier *applier, struct xrow_header *row)
>   {
>   	assert(iproto_type_is_raft_request(row->type));
> +	if (applier->instance_id == 0) {
> +		diag_set(ClientError, ER_PROTOCOL, "Can't apply a Raft request "
> +			 "from an instance without an ID");
> +		return -1;
> +	}
>   
>   	struct raft_request req;
>   	struct vclock candidate_clock;
> diff --git a/src/box/raft.c b/src/box/raft.c
> index 07a49351f..b3ab94bd7 100644
> --- a/src/box/raft.c
> +++ b/src/box/raft.c
> @@ -347,6 +347,7 @@ raft_process_msg(const struct raft_request *req, uint32_t source)
>   	if (req->vote != 0) {
>   		switch (raft.state) {
>   		case RAFT_STATE_FOLLOWER:
> +		case RAFT_STATE_LEADER:
>   			/*
>   			 * Can't respond on vote requests when Raft is disabled.
>   			 */
> @@ -386,6 +387,12 @@ raft_process_msg(const struct raft_request *req, uint32_t source)
>   					 "indirect votes are not allowed");
>   				break;
>   			}
> +			if (raft.leader != 0) {
> +				say_info("RAFT: vote request is skipped - the "
> +					 "leader is already known - %u",
> +					 raft.leader);
> +				break;
> +			}
>   			/*
>   			 * Either the term is new, or didn't vote in the current
>   			 * term yet. Anyway can vote now.
> @@ -415,16 +422,6 @@ raft_process_msg(const struct raft_request *req, uint32_t source)
>   			}
>   			raft_sm_become_leader();
>   			break;
> -		case RAFT_STATE_LEADER:
> -			/*
> -			 * If the node is still a leader, it ignores all votes.
> -			 * Indeed, if the received vote would be from a new
> -			 * term, the node would bump its own term and would
> -			 * enter the follower state by now. If the vote is from
> -			 * the current term, then the leader can freely ignore
> -			 * it.
> -			 */
> -			break;
>   		default:
>   			unreachable();
>   		}
> @@ -466,6 +463,12 @@ end:
>   void
>   raft_process_heartbeat(uint32_t source)
>   {
> +	/*
> +	 * Raft handles heartbeats from all instances, including anon instances
> +	 * which don't participate in Raft.
> +	 */
> +	if (source == 0)
> +		return;
>   	/*
>   	 * When not a candidate - don't wait for anything. Therefore do not care
>   	 * about the leader being dead.
> @@ -746,6 +749,7 @@ raft_sm_schedule_new_vote(uint32_t new_vote)
>   {
>   	say_info("RAFT: vote for %u, follow", new_vote, raft.volatile_term);
>   	assert(raft.volatile_vote == 0);
> +	assert(raft.leader == 0);
>   	assert(raft.state == RAFT_STATE_FOLLOWER);
>   	raft.volatile_vote = new_vote;
>   	raft_sm_pause_and_dump();
> @@ -929,15 +933,8 @@ raft_cfg_election_quorum(void)
>   		return;
>   	if (raft.vote_count < raft_election_quorum())
>   		return;
> -	/*
> -	 * The node is a candidate. It means its state if fully synced with
> -	 * disk. Otherwise it would be a follower.
> -	 */
> -	assert(!raft.is_write_in_progress);
> -	raft.state = RAFT_STATE_LEADER;
> -	raft.leader = instance_id;
> +	raft_sm_become_leader();
>   	raft_broadcast_new_state();
> -	box_update_ro_summary();
>   }
>   
>   void
Hi! Thanks for the fixes!
LGTM.

-- 
Serge Petrenko