Re: [Tarantool-patches] [PATCH v3 10/12] replication: add META stage to JOIN

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the patch!

See 3 comments below.

On 29.06.2021 00:12, Serge Petrenko via Tarantool-patches wrote:
> The new META stage is part of server's response to a join request.
> It's marked by IPROTO_JOIN_META and IPROTO_JOIN_SNAPSHOT requests and goes
> before the actual snapshot data.
> 
> Follow-up #6034

1. You probably should document the new keys and their meaning. As well
as what is sent between them.

> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index 07fe7f5c7..7abad3a64 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -447,12 +447,26 @@ applier_wait_snapshot(struct applier *applier)
>  		xrow_decode_vclock_xc(&row, &replicaset.vclock);
>  	}
>  
> +	coio_read_xrow(coio, ibuf, &row);
> +	if (row.type == IPROTO_JOIN_META) {
> +		/* Read additional metadata. Empty at the moment. */
> +		do {
> +			coio_read_xrow(coio, ibuf, &row);
> +			if (iproto_type_is_error(row.type))
> +				xrow_decode_error_xc(&row);
> +			else if (row.type != IPROTO_JOIN_SNAPSHOT) {

2. If one branch of `if` got {}, then all branches should get it too.

> +				tnt_raise(ClientError, ER_UNKNOWN_REQUEST_TYPE,
> +					  (uint32_t)row.type);
> +			}
> +		} while (row.type != IPROTO_JOIN_SNAPSHOT);
> +		coio_read_xrow(coio, ibuf, &row);
> +	}
> diff --git a/src/box/box.cc b/src/box/box.cc
> index b2c52bc54..bc68ee4c8 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -2501,7 +2501,7 @@ box_process_fetch_snapshot(struct ev_io *io, struct xrow_header *header)
>  
>  	/* Send the snapshot data to the instance. */
>  	struct vclock start_vclock;
> -	relay_initial_join(io->fd, header->sync, &start_vclock);
> +	relay_initial_join(io->fd, header->sync, &start_vclock, 0);
>  	say_info("read-view sent.");
>  
>  	/* Remember master's vclock after the last request */
> @@ -2699,7 +2699,8 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
>  	 * Initial stream: feed replica with dirty data from engines.
>  	 */
>  	struct vclock start_vclock;
> -	relay_initial_join(io->fd, header->sync, &start_vclock);
> +	relay_initial_join(io->fd, header->sync, &start_vclock,
> +			   replica_version_id);

3. Shouldn't changes to this file be in the previous commit?