[PATCH v3/3] replication: handle replication shutdown correctly.
Vladimir Davydov
vdavydov.dev at gmail.com
Wed Aug 15 21:47:58 MSK 2018
On Wed, Aug 15, 2018 at 07:13:28PM +0300, Serge Petrenko wrote:
> diff --git a/src/box/replication.cc b/src/box/replication.cc
> index 48956d2ed..083ae6407 100644
> --- a/src/box/replication.cc
> +++ b/src/box/replication.cc
> @@ -91,13 +91,6 @@ replication_init(void)
> latch_create(&replicaset.applier.order_latch);
> }
>
> -void
> -replication_free(void)
> -{
> - free(replicaset.replica_by_id);
> - fiber_cond_destroy(&replicaset.applier.cond);
> -}
> -
> void
> replica_check_id(uint32_t replica_id)
> {
> @@ -242,6 +235,42 @@ replica_clear_applier(struct replica *replica)
> trigger_clear(&replica->on_applier_state);
> }
>
> +void
> +replication_free(void)
> +{
> + struct replica *replica, *next;
> +
> + replica_hash_foreach_safe(&replicaset.hash, replica, next) {
> + if (replica->id == instance_id) {
> + replica_hash_remove(&replicaset.hash, replica);
> + /*
> + * Local replica doesn't have neither applier
> + * nor relay, so ignore it.
> + */
> + continue;
> + }
> + if (replica->applier != NULL) {
> + replica_clear_applier(replica);
> + /*
> + * We're exiting, so control won't be passed
> + * to appliers and we don't need to stop them.
> + */
> + }
You don't need this code either. I want this loop to be as simple as
/*
* <explain why>
*/
replicaset_foreach(replica)
relay_cancel(replica->relay);
Then you wouldn't even need to move the definition of replication_free.
> + if (replica->id != REPLICA_ID_NIL) {
> + /*
> + * Relay threads keep sending messages
> + * to tx via cbus upon shutdown, which
> + * could lead to segfaults. So cancel
> + * them.
> + */
> + relay_cancel(replica->relay);
> + }
> + }
> +
> + free(replicaset.replica_by_id);
> + fiber_cond_destroy(&replicaset.applier.cond);
> +}
More information about the Tarantool-patches
mailing list