Re: [Tarantool-patches] [PATCH v5 1/4] vclock: add an ability to reset individual clock components

[Serge Petrenko <sergepetrenko@tarantool.org>]

Hi! Thanks for the review!


> 6 апр. 2020 г., в 11:39, Konstantin Osipov <kostja.osipov@gmail.com> написал(а):
> 
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [20/04/05 05:06]:
>>> +void
>>> +vclock_reset(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
>> 
>> This is probably already discussed, but why do we have general
>> reset for arbitrary replica_id+lsn, and yet we have specialized
>> copy_ignore0? Why this function was not called vclock_reset0() with
>> replica_id == 0 and lsn == 0 always?


vclock_reset() is used in vclock_min() later. For arbitrary replica_id and
arbitrary lsn.

> 
> It's historical. I also suggest the function should be inlined and
> optimized into callers.

Okay.

>> 
>>> +{
>>> +	assert(lsn >= 0);
>>> +	assert(replica_id < VCLOCK_MAX);
>>> +	vclock->signature -= vclock_get(vclock, replica_id);
>>> +	if (lsn == 0) {
>>> +		vclock->map &= ~(1 << replica_id);
>>> +		return;
>>> +	}
>>> +	vclock->lsn[replica_id] = lsn;
>>> +	vclock->map |= 1 << replica_id;
>>> +	vclock->signature += lsn;
>>> +}
> 
> -- 
> Konstantin Osipov, Moscow, Russia

diff --git a/src/box/vclock.c b/src/box/vclock.c
index ac3e9fccd..90ae27591 100644
--- a/src/box/vclock.c
+++ b/src/box/vclock.c
@@ -37,21 +37,6 @@
 #include "diag.h"
 #include "tt_static.h"
 
-void
-vclock_reset(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
-{
-       assert(lsn >= 0);
-       assert(replica_id < VCLOCK_MAX);
-       vclock->signature -= vclock_get(vclock, replica_id);
-       if (lsn == 0) {
-               vclock->map &= ~(1 << replica_id);
-               return;
-       }
-       vclock->lsn[replica_id] = lsn;
-       vclock->map |= 1 << replica_id;
-       vclock->signature += lsn;
-}
-
 int64_t
 vclock_follow(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
 {
diff --git a/src/box/vclock.h b/src/box/vclock.h
index 2a3a29020..6438e63e4 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -192,8 +192,20 @@ vclock_inc(struct vclock *vclock, uint32_t replica_id)
  * @param replica_id Replica identifier.
  * @param lsn Lsn to set
  */
-void
-vclock_reset(struct vclock *vclock, uint32_t replica_id, int64_t lsn);
+static inline void
+vclock_reset(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
+{
+       assert(lsn >= 0);
+       assert(replica_id < VCLOCK_MAX);
+       vclock->signature -= vclock_get(vclock, replica_id);
+       if (lsn == 0) {
+               vclock->map &= ~(1 << replica_id);
+               return;
+       }
+       vclock->lsn[replica_id] = lsn;
+       vclock->map |= 1 << replica_id;
+       vclock->signature += lsn;
+}
 
 static inline void
 vclock_copy(struct vclock *dst, const struct vclock *src)


--
Serge Petrenko
sergepetrenko@tarantool.org