Re: [Tarantool-patches] [PATCH 4/5] vclock: ignore 0th component in comparisons.

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

>Вторник, 24 декабря 2019, 0:26 +03:00 от Sergey Petrenko <sergepetrenko@tarantool.org>:
>
>Hi! Thanks for the review.
>
>
>>Воскресенье, 22 декабря 2019, 20:59 +03:00 от Vladislav Shpilevoy < v.shpilevoy@tarantool.org >:
>>
>>Thanks for the patch!
>>
>>See 2 comments below.
>>
>>On 15/12/2019 21:58, sergepetrenko wrote:
>>> 0th vclock component will be used to count replica-local rows of an
>>> anonymous replica. These rows won't be replicated and different
>>> instances will have different values in vclock[0]. So ignore 0th
>>> component in comparisons.
>>
>>1. Lets add 'Part of #3186'.
>Done.
>
>>
>>
>>> ---
>>>  src/box/vclock.h    | 7 +++++++
>>>  test/unit/vclock.cc | 8 ++++----
>>>  2 files changed, 11 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/src/box/vclock.h b/src/box/vclock.h
>>> index b5eddcf8b..5dddbd173 100644
>>> --- a/src/box/vclock.h
>>> +++ b/src/box/vclock.h
>>> @@ -281,6 +281,13 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
>>> 
>>>  for (size_t replica_id = bit_iterator_next(&it); replica_id < VCLOCK_MAX;
>>>       replica_id = bit_iterator_next(&it)) {
>>> +/*
>>> + * Ignore 0-th component in comparisons.
>>> + * It is empty for normal replicas and should
>>> + * be ignored for anonymous ones.
>>> + */
>>> +if (replica_id == 0)
>>> +continue;
>>> 
>>>  int64_t lsn_a = vclock_get(a, replica_id);
>>>  int64_t lsn_b = vclock_get(b, replica_id);
>>
>>2. Consider this refactoring in order not to compare replica_id
>>with 0 on each iteration:
>>
>>================================================================================
>>
>>diff --git a/src/box/vclock.h b/src/box/vclock.h
>>index 5dddbd173..fc6aeb724 100644
>>--- a/src/box/vclock.h
>>+++ b/src/box/vclock.h
>>@@ -279,16 +279,15 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
>> struct bit_iterator it;
>> bit_iterator_init(&it, &map, sizeof(map), true);
>>
>>-for (size_t replica_id = bit_iterator_next(&it); replica_id < VCLOCK_MAX;
>>-     replica_id = bit_iterator_next(&it)) {
>>-/*
>>- * Ignore 0-th component in comparisons.
>>- * It is empty for normal replicas and should
>>- * be ignored for anonymous ones.
>>- */
>>-if (replica_id == 0)
>>-continue;
>>-
>>+size_t replica_id = bit_iterator_next(&it);
>>+/*
>>+ * Ignore 0-th component in comparisons. It is empty for
>>+ * normal replicas and should be ignored for anonymous
>>+ * ones.
>>+ */
>>+if (replica_id == 0)
>>+replica_id = bit_iterator_next(&it);
>>+for (; replica_id < VCLOCK_MAX; replica_id = bit_iterator_next(&it)) {
>> int64_t lsn_a = vclock_get(a, replica_id);
>> int64_t lsn_b = vclock_get(b, replica_id);
>> le = le && lsn_a <= lsn_b;
>
>Fixed:
>
>diff --git a/src/box/vclock.h b/src/box/vclock.h
>index 5dddbd173..8df628c62 100644
>--- a/src/box/vclock.h
>+++ b/src/box/vclock.h
>@@ -279,15 +279,16 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
> struct bit_iterator it;
> bit_iterator_init(&it, &map, sizeof(map), true);
> 
>+/*
>+ * Ignore 0-th component in comparisons.
>+ * It is empty for normal replicas and should
>+ * be ignored for anonymous ones.
>+ */
>+if (replica_id == 0)
>+replica_id = bit_iterator_next(&it);
>+
> for (size_t replica_id = bit_iterator_next(&it); replica_id < VCLOCK_MAX;
>      replica_id = bit_iterator_next(&it)) {
>-/*
>- * Ignore 0-th component in comparisons.
>- * It is empty for normal replicas and should
>- * be ignored for anonymous ones.
>- */
>-if (replica_id == 0)
>-continue;
> 
> int64_t lsn_a = vclock_get(a, replica_id);
> int64_t lsn_b = vclock_get(b, replica_id);
>
>>
>>
>>================================================================================
>
>
>-- 
>Sergey Petrenko

Sorry, fixed

diff --git a/src/box/vclock.h b/src/box/vclock.h
index 8df628c62..35ba6284c 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -279,6 +279,7 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
 struct bit_iterator it;
 bit_iterator_init(&it, &map, sizeof(map), true);
 
+size_t replica_id = bit_iterator_next(&it);
 /*
  * Ignore 0-th component in comparisons.
  * It is empty for normal replicas and should
@@ -287,8 +288,7 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
 if (replica_id == 0)
 replica_id = bit_iterator_next(&it);
 
-for (size_t replica_id = bit_iterator_next(&it); replica_id < VCLOCK_MAX;
-     replica_id = bit_iterator_next(&it)) {
+for (; replica_id < VCLOCK_MAX; replica_id = bit_iterator_next(&it)) {
 
 int64_t lsn_a = vclock_get(a, replica_id);
 int64_t lsn_b = vclock_get(b, replica_id);

-- 
Sergey Petrenko