Re: [Tarantool-patches] [PATCH 06/12] raft: make raft_request.vclock constant

[Serge Petrenko <sergepetrenko@tarantool.org>]

17.11.2020 03:02, Vladislav Shpilevoy пишет:
> Raft is never supposed to change vclock. Not the stored one, nor
> the received ones. The patch makes it checked during compilation.
>
> The patch is mostly motivated by a next patch making Raft use an
> externally configured vclock which can't be changed. Since Raft
> uses raft_request to carry the vclock in a few places, the
> request's vclock also must become const.
>
> Part of #5303


LGTM.


> ---
>   src/box/box.cc    | 2 +-
>   src/box/raftlib.c | 9 +++------
>   src/box/raftlib.h | 3 +--
>   src/box/xrow.c    | 2 +-
>   src/box/xrow.h    | 2 +-
>   5 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 8f5f3558e..78fca928e 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -2142,7 +2142,7 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
>   		 * should be 0.
>   		 */
>   		struct raft_request req;
> -		raft_serialize_for_network(box_raft(), &req, &vclock);
> +		raft_serialize_for_network(box_raft(), &req);
>   		xrow_encode_raft(&row, &fiber()->gc, &req);
>   		coio_write_xrow(io, &row);
>   	}
> diff --git a/src/box/raftlib.c b/src/box/raftlib.c
> index ca1940ba6..78164bf91 100644
> --- a/src/box/raftlib.c
> +++ b/src/box/raftlib.c
> @@ -850,8 +850,7 @@ raft_sm_stop(struct raft *raft)
>   }
>   
>   void
> -raft_serialize_for_network(const struct raft *raft, struct raft_request *req,
> -			   struct vclock *vclock)
> +raft_serialize_for_network(const struct raft *raft, struct raft_request *req)
>   {
>   	memset(req, 0, sizeof(*req));
>   	/*
> @@ -865,10 +864,8 @@ raft_serialize_for_network(const struct raft *raft, struct raft_request *req,
>   	 * Raft does not own vclock, so it always expects it passed externally.
>   	 * Vclock is sent out only by candidate instances.
>   	 */
> -	if (req->state == RAFT_STATE_CANDIDATE) {
> -		req->vclock = vclock;
> -		vclock_copy(vclock, &replicaset.vclock);
> -	}
> +	if (req->state == RAFT_STATE_CANDIDATE)
> +		req->vclock = &replicaset.vclock;
>   }
>   
>   void
> diff --git a/src/box/raftlib.h b/src/box/raftlib.h
> index f75ed2567..2da3cec86 100644
> --- a/src/box/raftlib.h
> +++ b/src/box/raftlib.h
> @@ -263,8 +263,7 @@ raft_new_term(struct raft *raft);
>    * cluster. It is allowed to save anything here, not only persistent state.
>    */
>   void
> -raft_serialize_for_network(const struct raft *raft, struct raft_request *req,
> -			   struct vclock *vclock);
> +raft_serialize_for_network(const struct raft *raft, struct raft_request *req);
>   
>   /**
>    * Save complete Raft state into a request to be persisted on disk. Only term
> diff --git a/src/box/xrow.c b/src/box/xrow.c
> index 165a00a16..bc06738ad 100644
> --- a/src/box/xrow.c
> +++ b/src/box/xrow.c
> @@ -1057,7 +1057,7 @@ xrow_decode_raft(const struct xrow_header *row, struct raft_request *r,
>   			r->vclock = vclock;
>   			if (r->vclock == NULL)
>   				mp_next(&pos);
> -			else if (mp_decode_vclock_ignore0(&pos, r->vclock) != 0)
> +			else if (mp_decode_vclock_ignore0(&pos, vclock) != 0)
>   				goto bad_msgpack;
>   			break;
>   		default:
> diff --git a/src/box/xrow.h b/src/box/xrow.h
> index 095911239..3d68c1268 100644
> --- a/src/box/xrow.h
> +++ b/src/box/xrow.h
> @@ -268,7 +268,7 @@ struct raft_request {
>   	uint64_t term;
>   	uint32_t vote;
>   	uint32_t state;
> -	struct vclock *vclock;
> +	const struct vclock *vclock;
>   };
>   
>   int

-- 
Serge Petrenko