Re: [Tarantool-patches] [PATCH 1/5] box: update comment describing join protocol

["Sergey Petrenko" <sergepetrenko@tarantool.org>]

>Воскресенье, 22 декабря 2019, 20:58 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:
>
>Thanks for the patch!

Hi! Thank you for review!

>
>
>See 1 question below.
>
>On 15/12/2019 21:58, sergepetrenko wrote:
>> From: Serge Petrenko < sergepetrenko@tarantool.org >
>> 
>> The comment states that relay sends the latest snapshot to replica
>> during initial join, however, this was changed in commit
>> 6332aca655ae7f95d391bdc0109e79915f6e6ad0 (relay: join new replicas off
>> read view).
>> Now relay sends rows from the read view created at the moment of join.
>> Update the comment to match.
>> 
>> Follow-up #1271
>> ---
>>  src/box/box.cc | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>> 
>> diff --git a/src/box/box.cc b/src/box/box.cc
>> index b119c927b..76fe0d534 100644
>> --- a/src/box/box.cc
>> +++ b/src/box/box.cc
>> @@ -1391,13 +1391,14 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
>>   * => JOIN { INSTANCE_UUID: replica_uuid }
>>   * <= OK { VCLOCK: start_vclock }
>>   *    Replica has enough permissions and master is ready for JOIN.
>> - *     - start_vclock - vclock of the latest master's checkpoint.
>> + *     - start_vclock - master's vclock at the time of join.
>>   *
>>   * <= INSERT
>>   *    ...
>>   *    Initial data: a stream of engine-specifc rows, e.g. snapshot
>> - *    rows for memtx or dirty cursor data for Vinyl. Engine can
>> - *    use REPLICA_ID, LSN and other fields for internal purposes.
>> + *    rows for memtx or dirty cursor data for Vinyl fed from a read-view.
>> + *    Engine can use REPLICA_ID, LSN and other fields for internal
>> + *    purposes.
>
>Rule of 66's ship has already sailed here, but lets at least
>keep it in 80.

Fixed.

diff --git a/src/box/box.cc b/src/box/box.cc
index 76fe0d534..22ac8bda1 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1396,9 +1396,9 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
         * <= INSERT
         *    ...
         *    Initial data: a stream of engine-specifc rows, e.g. snapshot
-        *    rows for memtx or dirty cursor data for Vinyl fed from a read-view.
-        *    Engine can use REPLICA_ID, LSN and other fields for internal
-        *    purposes.
+        *    rows for memtx or dirty cursor data for Vinyl fed from a
+        *    read-view. Engine can use REPLICA_ID, LSN and other fields
+        *    for internal purposes.
         *    ...
         * <= INSERT
         * <= OK { VCLOCK: stop_vclock } - end of initial JOIN stage.

>
>
>Also I have a question about what are the internal purposes?
>I really don't know. From what I see in the next commits,
>purposes of REPLICA_ID and LSN didn't change in this patchset.

Well, memtx_join_send_tuple just sets replica_id and lsn of each row
to zero, so I guess we can say this is "using REPLICA_ID and LSN for
internal purposes".  Same for vy_join_send_tuple.
I don't know whether it's worth pointing out that these fields are not used
at all. May be they were used for internal purposes or will be used some
day. 

>
>>   *    ...
>>   * <= INSERT
>>   * <= OK { VCLOCK: stop_vclock } - end of initial JOIN stage.
>> 


-- 
Sergey Petrenko