Re: [Tarantool-patches] [PATCH v2 04/10] replication: track registered replica count

[Sergey Ostanevich <sergos@tarantool.org>]

Hi!

Thanks for the patch, just one comment below.

Regards,
Sergos


On 04 сен 00:51, Vladislav Shpilevoy wrote:
> Struct replicaset didn't store a number of registered replicas.
> Only an array, which was necessary to fullscan each time when want
> to find the count.
> 
> That is going to be needed in Raft to calculate election quorum.
> The patch makes the count tracked so as it could be found for
> constant time by simply reading an integer.
> 
> Needed for #1146
> ---
>  src/box/replication.cc | 3 +++
>  src/box/replication.h  | 7 +++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/src/box/replication.cc b/src/box/replication.cc
> index ef0e2411d..20f16206a 100644
> --- a/src/box/replication.cc
> +++ b/src/box/replication.cc
> @@ -247,6 +247,7 @@ replica_set_id(struct replica *replica, uint32_t replica_id)
>  						   tt_uuid_str(&replica->uuid));
>  	}
>  	replicaset.replica_by_id[replica_id] = replica;
> +	++replicaset.size;

There's another use of the replica_set_id() inside the  register_replica()
of alter.cc. Apparently, it's just for re-assign the ID without adding of
a new node. I would propose to move this counter into replicaset_add()

>  
>  	say_info("assigned id %d to replica %s",
>  		 replica->id, tt_uuid_str(&replica->uuid));
> @@ -267,6 +268,8 @@ replica_clear_id(struct replica *replica)
>  	 * replication.
>  	 */
>  	replicaset.replica_by_id[replica->id] = NULL;
> +	assert(replicaset.size > 0);
> +	--replicaset.size;
>  	if (replica->id == instance_id) {
>  		/* See replica_check_id(). */
>  		assert(replicaset.is_joining);
> diff --git a/src/box/replication.h b/src/box/replication.h
> index ddc2bddf4..69cc820c9 100644
> --- a/src/box/replication.h
> +++ b/src/box/replication.h
> @@ -217,6 +217,13 @@ struct replicaset {
>  	bool is_joining;
>  	/* A number of anonymous replicas following this instance. */
>  	int anon_count;
> +	/**
> +	 * Number of registered replicas. That includes all of them - connected,
> +	 * disconnected, connected not directly, just present in _cluster. If an
> +	 * instance has an ID, has the same replicaset UUID, then it is
> +	 * accounted here.
> +	 */
> +	int size;
>  	/** Applier state. */
>  	struct {
>  		/**
> -- 
> 2.21.1 (Apple Git-122.3)
>