[Tarantool-patches] [PATCH] vclock: rename vclock_reset to vclock_set

[Tarantool-patches] [PATCH] vclock: rename vclock_reset to vclock_set

[Cyrill Gorcunov]

The `reset` keyword is too overloaded, its traditional
context is to setup some value either to zero or to default.

That is not what we're doing in this helper: we rather set up
a new value to the replica_id in the map, keeping consistency
of the sum.

Thus lets name it as it should -- vclock_set.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/box.cc   | 2 +-
 src/box/vclock.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index af92b55a8..c4f83c575 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1905,7 +1905,7 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
 	 * Speaking of gc, remote instances' local vclock
 	 * components are not used by consumers at all.
 	 */
-	vclock_reset(&replica_clock, 0, vclock_get(&replicaset.vclock, 0));
+	vclock_set(&replica_clock, 0, vclock_get(&replicaset.vclock, 0));
 	/*
 	 * Process SUBSCRIBE request via replication relay
 	 * Send current recovery vector clock as a marker
diff --git a/src/box/vclock.h b/src/box/vclock.h
index 5865f7443..8d62b561f 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -193,7 +193,7 @@ vclock_inc(struct vclock *vclock, uint32_t replica_id)
  * @param lsn Lsn to set
  */
 static inline void
-vclock_reset(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
+vclock_set(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
 {
 	assert(lsn >= 0);
 	assert(replica_id < VCLOCK_MAX);
@@ -411,7 +411,7 @@ vclock_min_ignore0(struct vclock *a, const struct vclock *b)
 		int64_t lsn_b = vclock_get(b, replica_id);
 		if (lsn_a <= lsn_b)
 			continue;
-		vclock_reset(a, replica_id, lsn_b);
+		vclock_set(a, replica_id, lsn_b);
 	}
 }
 
-- 
2.20.1

Re: [Tarantool-patches] [PATCH] vclock: rename vclock_reset to vclock_set

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [20/04/15 17:16]:
> The `reset` keyword is too overloaded, its traditional
> context is to setup some value either to zero or to default.
> 
> That is not what we're doing in this helper: we rather set up
> a new value to the replica_id in the map, keeping consistency
> of the sum.

> Thus lets name it as it should -- vclock_set.

Well, one could argue that it's impossible to set a vclock
component without violating consistency - vclock should never go
back. 

reset() indicates that it can unwind the clock.


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH] vclock: rename vclock_reset to vclock_set

[Cyrill Gorcunov]

On Wed, Apr 15, 2020 at 07:09:02PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [20/04/15 17:16]:
> > The `reset` keyword is too overloaded, its traditional
> > context is to setup some value either to zero or to default.
> > 
> > That is not what we're doing in this helper: we rather set up
> > a new value to the replica_id in the map, keeping consistency
> > of the sum.
> 
> > Thus lets name it as it should -- vclock_set.
> 
> Well, one could argue that it's impossible to set a vclock
> component without violating consistency - vclock should never go
> back. 
> 
> reset() indicates that it can unwind the clock.

Didn't you just named it in correct way -- vlock_unwind() ;-)
Anyway, we've been talking about this name in tchat and
I gave up. Drop the patch please.

Re: [Tarantool-patches] [PATCH] vclock: rename vclock_reset to vclock_set

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [20/04/15 19:22]:
> > > The `reset` keyword is too overloaded, its traditional
> > > context is to setup some value either to zero or to default.
> > > 
> > > That is not what we're doing in this helper: we rather set up
> > > a new value to the replica_id in the map, keeping consistency
> > > of the sum.
> > 
> > > Thus lets name it as it should -- vclock_set.
> > 
> > Well, one could argue that it's impossible to set a vclock
> > component without violating consistency - vclock should never go
> > back. 
> > 
> > reset() indicates that it can unwind the clock.
> 
> Didn't you just named it in correct way -- vlock_unwind() ;-)
> Anyway, we've been talking about this name in tchat and
> I gave up. Drop the patch please.

Let's rename vclock_reset if you like, but not back to vclock-set.
vclock-set is too trivial a name to reflect this is a non-trivial
op. vclock_reset() should be avoided, it's used only in a few
special cases which I would love to weed out eventually. 

Let's call it vclock_hack1 (one - because we're only hacking on a
single component, not entire clock), vclock_mutate, vclock_adjust,
whatever, but not vclock_set please.

-- 
Konstantin Osipov, Moscow, Russia