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

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

>Четверг, 26 декабря 2019, 8:03 +03:00 от Konstantin Osipov <kostja.osipov@gmail.com>:
>
>* Konstantin Osipov < kostja.osipov@gmail.com > [19/12/26 07:43]:
>> * sergepetrenko < sergepetrenko@tarantool.org > [19/12/16 09:47]:
>> > 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.
>> 
>> I don't know how this is going to work going forward.
>> 
>> vclock id 0 is already reserved for snapshots/the changes
>> of expelled replicas.

I couldn't find any code where id 0 is reserved.
What do you mean by "the changes of expelled replicas"?
However, it's true that vclock comparisons are used in creating snapshots
and finding the latest xlog on recovery.
So an anonymous replica won't create new snapshots if the only new changes
are the one made on the anonymous replica. Some problems with recovery may
also exist. I don't know whether it's severe enough, but looks not so good.
Thanks for pointing this out!

>
>
> A much safer bet would be to use a new special id number, like
> UINT64_MAX, and not change meaning of  an existing id.

This won't help IMO. We still have cases where this vclock component
should be ignored (replication) and cases where it should be taken into
account (checkpoint/xlog clock).
What about this change? I pushed it to the branch.
Also there's no need to fix vclock tests anymore.

=================================================
diff --git a/src/box/applier.cc b/src/box/applier.cc
index f4f9d0670..0df91f721 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -125,8 +125,8 @@ applier_check_sync(struct applier *applier)
  */
 if (applier->state == APPLIER_SYNC &&
     applier->lag <= replication_sync_lag &&
-    vclock_compare(&applier->remote_vclock_at_subscribe,
-   &replicaset.vclock) <= 0) {
+    vclock_compare_ignore0(&applier->remote_vclock_at_subscribe,
+   &replicaset.vclock) <= 0) {
 /* Applier is synced, switch to "follow". */
 applier_set_state(applier, APPLIER_FOLLOW);
 }
diff --git a/src/box/replication.cc b/src/box/replication.cc
index 81f19aa07..c48ec1957 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -934,8 +934,8 @@ replicaset_round(bool skip_ro)
  * with the same vclock, prefer the one with
  * the lowest uuid.
  */
-int cmp = vclock_compare(&applier->ballot.vclock,
-&leader->applier->ballot.vclock);
+int cmp = vclock_compare_ignore0(&applier->ballot.vclock,
+ &leader->applier->ballot.vclock);
 if (cmp < 0)
 continue;
 if (cmp == 0 && tt_uuid_compare(&replica->uuid,
diff --git a/src/box/vclock.h b/src/box/vclock.h
index 5d1a50d3d..e1625410e 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -266,13 +266,16 @@ enum { VCLOCK_ORDER_UNDEFINED = INT_MAX };
  * \brief Compare vclocks
  * \param a vclock
  * \param b vclock
+ * \param ignore_zero Whether to order by 0-th component or not
+ *
  * \retval 1 if \a vclock is ordered after \a other
  * \retval -1 if \a vclock is ordered before than \a other
  * \retval 0 if vclocks are equal
  * \retval VCLOCK_ORDER_UNDEFINED if vclocks are concurrent
  */
 static inline int
-vclock_compare(const struct vclock *a, const struct vclock *b)
+vclock_compare_generic(const struct vclock *a, const struct vclock *b,
+       bool ignore_zero)
 {
 bool le = true, ge = true;
 unsigned int map = a->map | b->map;
@@ -285,7 +288,7 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
  * It is empty for normal replicas and should
  * be ignored for anonymous ones.
  */
-if (replica_id == 0)
+if (replica_id == 0 && ignore_zero)
 replica_id = bit_iterator_next(&it);
 
 for (; replica_id < VCLOCK_MAX; replica_id = bit_iterator_next(&it)) {
@@ -303,6 +306,24 @@ vclock_compare(const struct vclock *a, const struct vclock *b)
 return 0;
 }
 
+/**
+ * \sa vclock_compare_generic
+ */
+static inline int
+vclock_compare(const struct vclock *a, const struct vclock *b)
+{
+return vclock_compare_generic(a, b, false);
+}
+
+/**
+ * \sa vclock_compare_generic
+ */
+static inline int
+vclock_compare_ignore0(const struct vclock *a, const struct vclock *b)
+{
+return vclock_compare_generic(a, b, true);
+}
+
 /**
  * @brief vclockset - a set of vclocks
  */
 
=================================================

>
>
>-- 
>Konstantin Osipov, Moscow, Russia


-- 
Sergey Petrenko