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

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

04.07.2021 15:28, Vladislav Shpilevoy пишет:
> 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.

Yep, will do.

>> 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.

Fixed, thanks!

>> +				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?

I think not. I start using replica's version only in this commit.
Why start passing it to relay earlier?

-- 
Serge Petrenko