Re: [Tarantool-patches] [PATCH 3/5] applier: split join processing into two stages

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

See 2 comments below.

On 15/12/2019 21:58, sergepetrenko wrote:
> From: Serge Petrenko <sergepetrenko@tarantool.org>
> 
> We already have 'initial join' and 'final join' stages in applier logic.
> The first actually means fetching master's snapshot, and the second one
> -- receiving the rows which should contain replica's registration in
> _cluster.
> These stages will be used separately once anonymous replica is
> implemented, so split them as a preparation.
> 
> Prerequisite #3186
> ---
>  src/box/applier.cc | 61 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 48 insertions(+), 13 deletions(-)
> 
> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index 42374f886..357369025 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -388,18 +388,12 @@ done:
>  	applier_set_state(applier, APPLIER_READY);
>  }
>  
> -/**
> - * Execute and process JOIN request (bootstrap the instance).
> - */
> -static void
> -applier_join(struct applier *applier)
> +static uint64_t
> +applier_do_fetch_snapshot(struct applier *applier)
>  {
> -	/* Send JOIN request */
>  	struct ev_io *coio = &applier->io;
>  	struct ibuf *ibuf = &applier->ibuf;
>  	struct xrow_header row;
> -	xrow_encode_join_xc(&row, &INSTANCE_UUID);
> -	coio_write_xrow(coio, &row);
>  
>  	/**
>  	 * Tarantool < 1.7.0: if JOIN is successful, there is no "OK"

1. applier_do_fetch_snapshot() still uses JOIN in some comments
and in apply_initial_join_row() function name. The function can
be renamed right here. The comments should be fixed when you add
REQUEST_SNAPSHOT.

Also I propose you to rename it to just applier_fetch_snapshot().
And the applier_fetch_snapshot() which you add later rename to
applier_request_and_fetch_snapshot(). But it is arguable. Lets
discuss if you disagree. I don't have a strong opinion here.

Another option - rename it to applier_wait_snapshot(). It will be
consistent with what I propose below.

> @@ -456,9 +448,16 @@ applier_join(struct applier *applier)
>  				  (uint32_t) row.type);
>  		}
>  	}
> -	say_info("initial data received");
>  
> -	applier_set_state(applier, APPLIER_FINAL_JOIN);
> +	return row_count;
> +}
> +
> +static uint64_t
> +applier_do_register(struct applier *applier, uint64_t row_count)

2. This looks rather like 'wait_register' because the only thing it
does is waiting for register response.

> +{
> +	struct ev_io *coio = &applier->io;
> +	struct ibuf *ibuf = &applier->ibuf;
> +	struct xrow_header row;
>  
>  	/*
>  	 * Tarantool < 1.7.0: there is no "final join" stage.