Re: [Tarantool-patches] [PATCH v2 01/11] applier: store instance_id in struct applier

[Serge Petrenko <sergepetrenko@tarantool.org>]

10.09.2020 02:16, Vladislav Shpilevoy пишет:
> Applier is going to need its numeric ID in order to tell the
> future Raft module who is a sender of a Raft message. An
> alternative would be to add sender ID to each Raft message, but
> this looks like a crutch. Moreover, applier still needs to know
> its numeric ID in order to notify Raft about heartbeats from the
> peer node.
>
> Needed for #1146
> ---
> In the previous version applier set its ID on its own. But that didn't work
> somewhy. It happened, that on master the ID was never set after bootstrap. Tbh
> I didn't find why, and just moved the ID assignment to another more suitable
> place. To the same where replica gets its ID and where an applier is associated
> with a replica.
>
>   src/box/applier.h      | 2 ++
>   src/box/replication.cc | 7 ++++++-
>   2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/src/box/applier.h b/src/box/applier.h
> index 6e979a806..15ca1fcfd 100644
> --- a/src/box/applier.h
> +++ b/src/box/applier.h
> @@ -95,6 +95,8 @@ struct applier {
>   	ev_tstamp lag;
>   	/** The last box_error_code() logged to avoid log flooding */
>   	uint32_t last_logged_errcode;
> +	/** Remote instance ID. */
> +	uint32_t instance_id;
>   	/** Remote instance UUID */
>   	struct tt_uuid uuid;
>   	/** Remote URI (string) */
> diff --git a/src/box/replication.cc b/src/box/replication.cc
> index ef0e2411d..744236b99 100644
> --- a/src/box/replication.cc
> +++ b/src/box/replication.cc
> @@ -247,7 +247,8 @@ replica_set_id(struct replica *replica, uint32_t replica_id)
>   						   tt_uuid_str(&replica->uuid));
>   	}
>   	replicaset.replica_by_id[replica_id] = replica;
> -
> +	if (replica->applier != NULL)
> +		replica->applier->instance_id = replica_id;
>   	say_info("assigned id %d to replica %s",
>   		 replica->id, tt_uuid_str(&replica->uuid));
>   	replica->anon = false;
> @@ -273,6 +274,8 @@ replica_clear_id(struct replica *replica)
>   		instance_id = REPLICA_ID_NIL;
>   	}
>   	replica->id = REPLICA_ID_NIL;
> +	if (replica->applier != NULL)
> +		replica->applier->instance_id = 0;
>   	say_info("removed replica %s", tt_uuid_str(&replica->uuid));
>   
>   	/*
> @@ -301,12 +304,14 @@ replica_set_applier(struct replica *replica, struct applier *applier)
>   	replica->applier = applier;
>   	trigger_add(&replica->applier->on_state,
>   		    &replica->on_applier_state);
> +	applier->instance_id = replica->id;
>   }
>   
>   void
>   replica_clear_applier(struct replica *replica)
>   {
>   	assert(replica->applier != NULL);
> +	replica->applier->instance_id = 0;
>   	replica->applier = NULL;
>   	trigger_clear(&replica->on_applier_state);
>   }
LGTM

-- 
Serge Petrenko