[Tarantool-patches] [PATCH v3 0/4] replication: fix local space tracking

[Tarantool-patches] [PATCH v3 0/4] replication: fix local space tracking

[Serge Petrenko]

https://github.com/tarantool/tarantool/issues/4114
https://github.com/tarantool/tarantool/tree/sp/gh-4114-local-space-replication

Changes in v3:
  - rewrite the patches re wal gc rework
    to avoid matrix clock.

Changes in v2:
  - used Georgy's patches re introduction of
    matrix clock and wal gc rework to fix the
    problem with ordering gc consumers by
    vclock signature

Serge Petrenko (4):
  vclock: add an ability to set individual clock components
  gc: rely on minimal vclock components instead of signatures
  replication: hide 0-th vclock components in replication responses
  box: start counting local space requests separately

 src/box/applier.cc                            |   5 +-
 src/box/box.cc                                |  32 ++++-
 src/box/gc.c                                  |  29 ++--
 src/box/relay.cc                              |  21 +--
 src/box/vclock.c                              |  15 +++
 src/box/vclock.h                              |  66 +++++++++
 src/box/wal.c                                 |  16 ++-
 test/replication/anon.result                  |   5 +
 test/replication/anon.test.lua                |   2 +
 test/replication/autobootstrap.result         |   6 +
 test/replication/autobootstrap.test.lua       |   2 +
 test/replication/before_replace.result        |   8 +-
 test/replication/before_replace.test.lua      |   4 +-
 .../gh-4114-local-space-replication.result    | 125 ++++++++++++++++++
 .../gh-4114-local-space-replication.test.lua  |  48 +++++++
 test/replication/local_spaces.result          |   4 +
 test/replication/local_spaces.test.lua        |   3 +
 test/replication/misc.result                  |   6 +
 test/replication/misc.test.lua                |   2 +
 test/replication/quorum.result                |   6 +
 test/replication/quorum.test.lua              |   2 +
 test/replication/replica_rejoin.result        |   9 ++
 test/replication/replica_rejoin.test.lua      |   3 +
 test/replication/skip_conflict_row.result     |   3 +
 test/replication/skip_conflict_row.test.lua   |   1 +
 test/replication/suite.cfg                    |   1 +
 test/vinyl/errinj.result                      |   5 +
 test/vinyl/errinj.test.lua                    |   4 +
 28 files changed, 403 insertions(+), 30 deletions(-)
 create mode 100644 test/replication/gh-4114-local-space-replication.result
 create mode 100644 test/replication/gh-4114-local-space-replication.test.lua

-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH v3 1/4] vclock: add an ability to set individual clock components

[Serge Petrenko]

This is needed to 'hide' 0-th vclock component in replication responses.

Follow-up #3186
Prerequisite #4114
---
 src/box/vclock.c | 15 +++++++++++++++
 src/box/vclock.h | 11 +++++++++++
 2 files changed, 26 insertions(+)

diff --git a/src/box/vclock.c b/src/box/vclock.c
index 90ae27591..302c0438d 100644
--- a/src/box/vclock.c
+++ b/src/box/vclock.c
@@ -37,6 +37,21 @@
 #include "diag.h"
 #include "tt_static.h"
 
+void
+vclock_set(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 79e5a1bc0..7dc69015f 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -211,6 +211,17 @@ vclock_calc_sum(const struct vclock *vclock)
 	return sum;
 }
 
+/**
+ * Set vclock component represented by replica id to the desired
+ * value.
+ *
+ * @param vclock Vector clock.
+ * @param replica_id Replica identifier.
+ * @param lsn Lsn to set
+ */
+void
+vclock_set(struct vclock *vclock, uint32_t replica_id, int64_t lsn);
+
 static inline int64_t
 vclock_sum(const struct vclock *vclock)
 {
-- 
2.21.1 (Apple Git-122.3)

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

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [20/03/23 19:23]:
> This is needed to 'hide' 0-th vclock component in replication responses.

I think the name should reflect the magic that happens in this
function.

How about vclock_reset()?

> 
> Follow-up #3186
> Prerequisite #4114
> ---
>  src/box/vclock.c | 15 +++++++++++++++
>  src/box/vclock.h | 11 +++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/src/box/vclock.c b/src/box/vclock.c
> index 90ae27591..302c0438d 100644
> --- a/src/box/vclock.c
> +++ b/src/box/vclock.c
> @@ -37,6 +37,21 @@
>  #include "diag.h"
>  #include "tt_static.h"
>  
> +void
> +vclock_set(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 79e5a1bc0..7dc69015f 100644
> --- a/src/box/vclock.h
> +++ b/src/box/vclock.h
> @@ -211,6 +211,17 @@ vclock_calc_sum(const struct vclock *vclock)
>  	return sum;
>  }
>  
> +/**
> + * Set vclock component represented by replica id to the desired
> + * value.

Why is it necessary and how is it used? Should be in this comment. 
The use case is still not very clear from the commit comment.

> + *
> + * @param vclock Vector clock.
> + * @param replica_id Replica identifier.
> + * @param lsn Lsn to set
> + */
> +void
> +vclock_set(struct vclock *vclock, uint32_t replica_id, int64_t lsn);
> +
>  static inline int64_t
>  vclock_sum(const struct vclock *vclock)

-- 
Konstantin Osipov, Moscow, Russia
https://scylladb.com

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

[Serge Petrenko]

> 23 марта 2020 г., в 19:37, Konstantin Osipov <kostja.osipov@gmail.com> написал(а):
> 
> * Serge Petrenko <sergepetrenko@tarantool.org> [20/03/23 19:23]:
>> This is needed to 'hide' 0-th vclock component in replication responses.
> 
> I think the name should reflect the magic that happens in this
> function.
> 
> How about vclock_reset()?

No problem, fixed.

> 
>> 
>> Follow-up #3186
>> Prerequisite #4114
>> ---
>> src/box/vclock.c | 15 +++++++++++++++
>> src/box/vclock.h | 11 +++++++++++
>> 2 files changed, 26 insertions(+)
>> 
>> diff --git a/src/box/vclock.c b/src/box/vclock.c
>> index 90ae27591..302c0438d 100644
>> --- a/src/box/vclock.c
>> +++ b/src/box/vclock.c
>> @@ -37,6 +37,21 @@
>> #include "diag.h"
>> #include "tt_static.h"
>> 
>> +void
>> +vclock_set(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 79e5a1bc0..7dc69015f 100644
>> --- a/src/box/vclock.h
>> +++ b/src/box/vclock.h
>> @@ -211,6 +211,17 @@ vclock_calc_sum(const struct vclock *vclock)
>> 	return sum;
>> }
>> 
>> +/**
>> + * Set vclock component represented by replica id to the desired
>> + * value.
> 
> Why is it necessary and how is it used? Should be in this comment. 
> The use case is still not very clear from the commit comment.
> 

I amended both the commit message and the comment. Please see v4

>> + *
>> + * @param vclock Vector clock.
>> + * @param replica_id Replica identifier.
>> + * @param lsn Lsn to set
>> + */
>> +void
>> +vclock_set(struct vclock *vclock, uint32_t replica_id, int64_t lsn);
>> +
>> static inline int64_t
>> vclock_sum(const struct vclock *vclock)
> 
> -- 
> Konstantin Osipov, Moscow, Russia
> https://scylladb.com


--
Serge Petrenko
sergepetrenko@tarantool.org

[Tarantool-patches] [PATCH v3 2/4] gc: rely on minimal vclock components instead of signatures

[Serge Petrenko]

The current WAL GC implementation tracks consumers (i.e. remote replicas) by
their vclock signature, which is the sum of all vclock components.
This approach is wrong, and this can be shown by a little example.
The example will be a little synthetic, but it'll illustrate the problem.
Say, you have 2 masters, A and B with ids 1 and 2 respectively, and a replica C
with id 3. Say, С replicates from both A and B, and there is no replication
between A and B (say, the instances were reconfigured to not replicate from each
other). Now, say replica C has followed A and B to vclock {1:5, 2:13}. At the
same time, A has lsn 10 and B has lsn 15. A and B do not know about each other’s
changes, so A’s vclock is {1:10} and B’s vclock is {2:15}. Now imagine A does a
snapshot and creates a new xlog with signature 10. A’s directory will look like:
00…000.xlog 00…010.snap 00….010.xlog
Replica C reports its vclock {1:5, 2:13} to A, A uses the vclock to update the
corresponding GC consumer. Since signatures are used, GC consumer is assigned a
signature = 13 + 5 = 18. This is greater than the signature of the last xlog on
A (which is 10), so the previous xlog (00…00.xlog) can be deleted (at least A
assumes it can be). Actually, replica still needs 00…00.xlog, because it
contains rows corresponding to vclocks {1:6} - {1:10}, which haven’t been
replicated yet.

If instead of using vclock signatures, gc consumers used vclocks, such a problem
wouldn’t arise. Replia would report its vclock {1:5, 2:13}. The vclock is NOT
strictly greater than A’s most recent xlog vclock ({1:10}), so the previous log
is kept until replica reports a vclock {1:10, 2:something} or {1:11, …} and so
on.

Rewrite gc to perform cleanup based on finding minimal vclock
components present in at least one of the consumer vclocks instead of
just comparing vclock signatures.

Prerequisite #4114
---
 src/box/gc.c     | 29 +++++++++++++++----------
 src/box/vclock.h | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/src/box/gc.c b/src/box/gc.c
index f5c387f9d..b63f65af0 100644
--- a/src/box/gc.c
+++ b/src/box/gc.c
@@ -66,16 +66,15 @@ static int
 gc_checkpoint_fiber_f(va_list);
 
 /**
- * Comparator used for ordering gc_consumer objects by signature
- * in a binary tree.
+ * Comparator used for ordering gc_consumer objects
+ * lexicographically by their vclock in a binary tree.
  */
 static inline int
 gc_consumer_cmp(const struct gc_consumer *a, const struct gc_consumer *b)
 {
-	if (vclock_sum(&a->vclock) < vclock_sum(&b->vclock))
-		return -1;
-	if (vclock_sum(&a->vclock) > vclock_sum(&b->vclock))
-		return 1;
+	int rc = vclock_lex_compare(&a->vclock, &b->vclock);
+	if (rc != 0)
+		return rc;
 	if ((intptr_t)a < (intptr_t)b)
 		return -1;
 	if ((intptr_t)a > (intptr_t)b)
@@ -192,11 +191,19 @@ gc_run_cleanup(void)
 	 * Note, we must keep all WALs created after the
 	 * oldest checkpoint, even if no consumer needs them.
 	 */
-	const struct vclock *vclock = (gc_tree_empty(&gc.consumers) ? NULL :
-				       &gc_tree_first(&gc.consumers)->vclock);
-	if (vclock == NULL ||
-	    vclock_sum(vclock) > vclock_sum(&checkpoint->vclock))
-		vclock = &checkpoint->vclock;
+	struct vclock min_vclock;
+	struct vclock *vclock;
+	struct gc_consumer *consumer = gc_tree_first(&gc.consumers);
+	if (consumer != NULL) {
+		vclock_copy(&min_vclock, &consumer->vclock);
+		while ((consumer = gc_tree_next(&gc.consumers, consumer)) != NULL) {
+			vclock_min(&min_vclock, &consumer->vclock);
+		}
+		vclock_min(&min_vclock, &checkpoint->vclock);
+		vclock = &min_vclock;
+	} else {
+		vclock =  &checkpoint->vclock;
+	}
 
 	if (vclock_sum(vclock) > vclock_sum(&gc.vclock)) {
 		vclock_copy(&gc.vclock, vclock);
diff --git a/src/box/vclock.h b/src/box/vclock.h
index 7dc69015f..7ac9af1bb 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -335,6 +335,61 @@ vclock_compare_ignore0(const struct vclock *a, const struct vclock *b)
 	return vclock_compare_generic(a, b, true);
 }
 
+/**
+ * Compare vclocks lexicographically.
+ * The following affirmation holds: if a < b in terms of
+ * vclock_compare, then a < b in terms of vclock_lex_compare.
+ * However, a < b in terms of vclock_lex_compare doesn't mean
+ * than a < b in terms of vclock_compare. Example:
+ * {1:5, 2:10} < {1:7, 2:3} (vclock_lex_compare), but the vclocks
+ * are incomparable in terms of vclock_compare.
+ * All vclocks are comparable lexicographically.
+ *
+ * \param a vclock
+ * \param b vclock
+ * \retval 1 if \a a is lexicographically greater than \a b
+ * \retval -1 if \a a is lexicographically less than \a b
+ * \retval 0 if vclocks are equal
+ */
+static inline int
+vclock_lex_compare(const struct vclock *a, const struct vclock *b)
+{
+	vclock_map_t map = a->map | b->map;
+	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)) {
+		int64_t lsn_a = vclock_get(a, replica_id);
+		int64_t lsn_b = vclock_get(b, replica_id);
+		if (lsn_a < lsn_b) return -1;
+		if (lsn_a > lsn_b) return 1;
+	}
+	return 0;
+}
+
+/**
+ * Return a vclock, which is a componentwise minimum between
+ * vclocks \a a and \a b.
+ *
+ * \param[in,out] a vclock containing the minimum components.
+ * \param b vclock to compare with.
+ */
+static inline void
+vclock_min(struct vclock *a, const struct vclock *b)
+{
+	vclock_map_t map = a->map | b->map;
+	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)) {
+		int64_t lsn_a = vclock_get(a, replica_id);
+		int64_t lsn_b = vclock_get(b, replica_id);
+		if (lsn_a <= lsn_b)
+			continue;
+		vclock_set(a, replica_id, lsn_b);
+	}
+}
+
 /**
  * @brief vclockset - a set of vclocks
  */
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH v3 2/4] gc: rely on minimal vclock components instead of signatures

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [20/03/23 19:23]:
> +	struct vclock min_vclock;
> +	struct vclock *vclock;
> +	struct gc_consumer *consumer = gc_tree_first(&gc.consumers);
> +	if (consumer != NULL) {
> +		vclock_copy(&min_vclock, &consumer->vclock);
> +		while ((consumer = gc_tree_next(&gc.consumers, consumer)) != NULL) {
> +			vclock_min(&min_vclock, &consumer->vclock);
> +		}
> +		vclock_min(&min_vclock, &checkpoint->vclock);
> +		vclock = &min_vclock;
> +	} else {
> +		vclock =  &checkpoint->vclock;
> +	}

This loop is a total magic to anyone not present at our zoom
conversation. Please explain what's going on here in a comment.



-- 
Konstantin Osipov, Moscow, Russia
https://scylladb.com

Re: [Tarantool-patches] [PATCH v3 2/4] gc: rely on minimal vclock components instead of signatures

[Serge Petrenko]

> 23 марта 2020 г., в 19:40, Konstantin Osipov <kostja.osipov@gmail.com> написал(а):
> 
> * Serge Petrenko <sergepetrenko@tarantool.org> [20/03/23 19:23]:
>> +	struct vclock min_vclock;
>> +	struct vclock *vclock;
>> +	struct gc_consumer *consumer = gc_tree_first(&gc.consumers);
>> +	if (consumer != NULL) {
>> +		vclock_copy(&min_vclock, &consumer->vclock);
>> +		while ((consumer = gc_tree_next(&gc.consumers, consumer)) != NULL) {
>> +			vclock_min(&min_vclock, &consumer->vclock);
>> +		}
>> +		vclock_min(&min_vclock, &checkpoint->vclock);
>> +		vclock = &min_vclock;
>> +	} else {
>> +		vclock =  &checkpoint->vclock;
>> +	}
> 
> This loop is a total magic to anyone not present at our zoom
> conversation. Please explain what's going on here in a comment.

Added a comment.

> 
> 
> 
> -- 
> Konstantin Osipov, Moscow, Russia
> https://scylladb.com


--
Serge Petrenko
sergepetrenko@tarantool.org

[Tarantool-patches] [PATCH v3 3/4] replication: hide 0-th vclock components in replication responses

[Serge Petrenko]

If an anonymous replica is promoted to a normal one and becomes
replication master later, its vclock contains a non-empty zero
component, tracking local changes on this replica from the time when it
had been anonymous. No need to pollute joining instance's vclock with
our non-empty 0 component.
When an anonymous replica reports its status to a remote instance it
should also hide its 0-th vclock component.

Follow-up #3186
Prerequisite #4114
---
 src/box/applier.cc             |  5 ++++-
 src/box/box.cc                 | 12 ++++++++++--
 src/box/relay.cc               |  6 +++++-
 test/replication/anon.result   |  5 +++++
 test/replication/anon.test.lua |  2 ++
 5 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/box/applier.cc b/src/box/applier.cc
index 8666a3a98..da6bbbac2 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -173,7 +173,10 @@ applier_writer_f(va_list ap)
 			continue;
 		try {
 			struct xrow_header xrow;
-			xrow_encode_vclock(&xrow, &replicaset.vclock);
+			struct vclock vclock;
+			vclock_copy(&vclock, &replicaset.vclock);
+			vclock_set(&vclock, 0, 0);
+			xrow_encode_vclock(&xrow, &vclock);
 			coio_write_xrow(&io, &xrow);
 		} catch (SocketError *e) {
 			/*
diff --git a/src/box/box.cc b/src/box/box.cc
index 765d64678..6de6b677c 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1540,6 +1540,7 @@ box_process_fetch_snapshot(struct ev_io *io, struct xrow_header *header)
 	/* Remember master's vclock after the last request */
 	struct vclock stop_vclock;
 	vclock_copy(&stop_vclock, &replicaset.vclock);
+	vclock_set(&stop_vclock, 0, 0);
 
 	/* Send end of snapshot data marker */
 	struct xrow_header row;
@@ -1621,7 +1622,9 @@ box_process_register(struct ev_io *io, struct xrow_header *header)
 
 	struct xrow_header row;
 	/* Send end of WAL stream marker */
-	xrow_encode_vclock_xc(&row, &replicaset.vclock);
+	vclock_copy(&vclock, &replicaset.vclock);
+	vclock_set(&vclock, 0, 0);
+	xrow_encode_vclock_xc(&row, &vclock);
 	row.sync = header->sync;
 	coio_write_xrow(io, &row);
 
@@ -1773,7 +1776,11 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
 	say_info("final data sent.");
 
 	/* Send end of WAL stream marker */
-	xrow_encode_vclock_xc(&row, &replicaset.vclock);
+	struct vclock vclock;
+	vclock_copy(&vclock, &replicaset.vclock);
+	vclock_set(&vclock, 0, 0);
+	xrow_encode_vclock_xc(&row, &vclock);
+
 	row.sync = header->sync;
 	coio_write_xrow(io, &row);
 
@@ -1905,6 +1912,7 @@ box_process_vote(struct ballot *ballot)
 	ballot->is_loading = is_ro;
 	vclock_copy(&ballot->vclock, &replicaset.vclock);
 	vclock_copy(&ballot->gc_vclock, &gc.vclock);
+	vclock_set(&ballot->gc_vclock, 0, 0);
 }
 
 /** Insert a new cluster into _schema */
diff --git a/src/box/relay.cc b/src/box/relay.cc
index 95245a3cf..21b7c0a6f 100644
--- a/src/box/relay.cc
+++ b/src/box/relay.cc
@@ -322,7 +322,11 @@ relay_initial_join(int fd, uint64_t sync, struct vclock *vclock)
 
 	/* Respond to the JOIN request with the current vclock. */
 	struct xrow_header row;
-	xrow_encode_vclock_xc(&row, vclock);
+	struct vclock subst_vclock;
+	vclock_copy(&subst_vclock, vclock);
+	/* zero - out 0-th vclock component. */
+	vclock_set(&subst_vclock, 0, 0);
+	xrow_encode_vclock_xc(&row, &subst_vclock);
 	row.sync = sync;
 	coio_write_xrow(&relay->io, &row);
 
diff --git a/test/replication/anon.result b/test/replication/anon.result
index 88061569f..cbbeeef09 100644
--- a/test/replication/anon.result
+++ b/test/replication/anon.result
@@ -187,6 +187,11 @@ a > 0
  | ---
  | - true
  | ...
+-- 0-th vclock component isn't propagated across the cluster.
+box.info.vclock[0]
+ | ---
+ | - null
+ | ...
 test_run:cmd('switch default')
  | ---
  | - true
diff --git a/test/replication/anon.test.lua b/test/replication/anon.test.lua
index 8a8d15c18..627dc5c8e 100644
--- a/test/replication/anon.test.lua
+++ b/test/replication/anon.test.lua
@@ -66,6 +66,8 @@ test_run:cmd('switch replica_anon2')
 a = box.info.vclock[1]
 -- The instance did fetch a snapshot.
 a > 0
+-- 0-th vclock component isn't propagated across the cluster.
+box.info.vclock[0]
 test_run:cmd('switch default')
 box.space.test:insert{2}
 test_run:cmd("switch replica_anon2")
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH v3 3/4] replication: hide 0-th vclock components in replication responses

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [20/03/23 19:23]:
> If an anonymous replica is promoted to a normal one and becomes
> replication master later, its vclock contains a non-empty zero
> component, tracking local changes on this replica from the time when it
> had been anonymous. No need to pollute joining instance's vclock with
> our non-empty 0 component.
> When an anonymous replica reports its status to a remote instance it
> should also hide its 0-th vclock component.

The patches for anonymous replicas were pushed without my review.

I think whoever reviewed them should review these patches as well.

-- 
Konstantin Osipov, Moscow, Russia
https://scylladb.com

[Tarantool-patches] [PATCH v3 4/4] box: start counting local space requests separately

[Serge Petrenko]

Sign local space requests with a zero instance id. This allows to split
local changes aside from the changes, which should be visible to the whole
cluster, and stop sending NOPs to replicas to follow local vclock.

Closes #4114
---
 src/box/box.cc                                |  20 +++
 src/box/relay.cc                              |  15 +--
 src/box/wal.c                                 |  16 ++-
 test/replication/autobootstrap.result         |   6 +
 test/replication/autobootstrap.test.lua       |   2 +
 test/replication/before_replace.result        |   8 +-
 test/replication/before_replace.test.lua      |   4 +-
 .../gh-4114-local-space-replication.result    | 125 ++++++++++++++++++
 .../gh-4114-local-space-replication.test.lua  |  48 +++++++
 test/replication/local_spaces.result          |   4 +
 test/replication/local_spaces.test.lua        |   3 +
 test/replication/misc.result                  |   6 +
 test/replication/misc.test.lua                |   2 +
 test/replication/quorum.result                |   6 +
 test/replication/quorum.test.lua              |   2 +
 test/replication/replica_rejoin.result        |   9 ++
 test/replication/replica_rejoin.test.lua      |   3 +
 test/replication/skip_conflict_row.result     |   3 +
 test/replication/skip_conflict_row.test.lua   |   1 +
 test/replication/suite.cfg                    |   1 +
 test/vinyl/errinj.result                      |   5 +
 test/vinyl/errinj.test.lua                    |   4 +
 22 files changed, 278 insertions(+), 15 deletions(-)
 create mode 100644 test/replication/gh-4114-local-space-replication.result
 create mode 100644 test/replication/gh-4114-local-space-replication.test.lua

diff --git a/src/box/box.cc b/src/box/box.cc
index 6de6b677c..b7ea0de2d 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1882,6 +1882,26 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
 	say_info("remote vclock %s local vclock %s",
 		 vclock_to_string(&replica_clock), vclock_to_string(&vclock));
 
+	/*
+	 * Replica clock is used in gc state and recovery
+	 * initialization, so we need to replace the remote 0-th
+	 * component with our own one. This makes recovery work
+	 * correctly: we're trying to recover from an xlog whose
+	 * vclock is smaller than remote replica clock in each
+	 * component, exluding the 0-th component. This leads to
+	 * finding the correct WAL, if it exists, since we do not
+	 * need to recover local rows (the ones, that contribute
+	 * to the 0-th vclock component). It would be bad to set
+	 * 0-th vclock component to a smaller value, since it
+	 * would unnecessarily require additional WALs, which may
+	 * have already been deleted.
+	 * Same stands for gc. Remote instances do not need this
+	 * instance's local rows, and after gc was reworked to
+	 * track individual vclock components instead of
+	 * signatures it's safe to set the local component to the
+	 * most recent value.
+	 */
+	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/relay.cc b/src/box/relay.cc
index 21b7c0a6f..a91e4b771 100644
--- a/src/box/relay.cc
+++ b/src/box/relay.cc
@@ -760,16 +760,15 @@ relay_send_row(struct xstream *stream, struct xrow_header *packet)
 {
 	struct relay *relay = container_of(stream, struct relay, stream);
 	assert(iproto_type_is_dml(packet->type));
-	/*
-	 * Transform replica local requests to IPROTO_NOP so as to
-	 * promote vclock on the replica without actually modifying
-	 * any data.
-	 */
 	if (packet->group_id == GROUP_LOCAL) {
 		/*
-		 * Replica-local requests generated while replica
-		 * was anonymous have a zero instance id. Just
-		 * skip all these rows.
+		 * We do not relay replica-local rows to other
+		 * instances, since we started signing them with
+		 * a zero instance id. However, if replica-local
+		 * rows, signed with a non-zero id are present in
+		 * our WAL, we still need to relay them as NOPs in
+		 * order to correctly promote the vclock on the
+		 * replica.
 		 */
 		if (packet->replica_id == REPLICA_ID_NIL)
 			return;
diff --git a/src/box/wal.c b/src/box/wal.c
index 3b094b0e8..a74bdecd9 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -953,13 +953,19 @@ wal_assign_lsn(struct vclock *vclock_diff, struct vclock *base,
 	/** Assign LSN to all local rows. */
 	for ( ; row < end; row++) {
 		if ((*row)->replica_id == 0) {
-			(*row)->lsn = vclock_inc(vclock_diff, instance_id) +
-				      vclock_get(base, instance_id);
 			/*
-			 * Note, an anonymous replica signs local
-			 * rows whith a zero instance id.
+			 * All rows representing local space data
+			 * manipulations are signed wth a zero
+			 * instance id. This is also true for
+			 * anonymous replicas, since they are
+			 * only capable of writing to local and
+			 * temporary spaces.
 			 */
-			(*row)->replica_id = instance_id;
+			if ((*row)->group_id != GROUP_LOCAL)
+				(*row)->replica_id = instance_id;
+
+			(*row)->lsn = vclock_inc(vclock_diff, (*row)->replica_id) +
+						 vclock_get(base, (*row)->replica_id);
 			/* Use lsn of the first local row as transaction id. */
 			tsn = tsn == 0 ? (*row)->lsn : tsn;
 			(*row)->tsn = tsn;
diff --git a/test/replication/autobootstrap.result b/test/replication/autobootstrap.result
index 743982d47..6918e23ea 100644
--- a/test/replication/autobootstrap.result
+++ b/test/replication/autobootstrap.result
@@ -162,6 +162,9 @@ box.schema.user.revoke('test_u', 'create', 'space')
 vclock = test_run:get_vclock('autobootstrap1')
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock("autobootstrap2", vclock)
 ---
 ...
@@ -206,6 +209,9 @@ test_run:wait_fullmesh(SERVERS)
 vclock = test_run:get_vclock("autobootstrap1")
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock("autobootstrap2", vclock)
 ---
 ...
diff --git a/test/replication/autobootstrap.test.lua b/test/replication/autobootstrap.test.lua
index 055ea4277..f8bb1c74a 100644
--- a/test/replication/autobootstrap.test.lua
+++ b/test/replication/autobootstrap.test.lua
@@ -74,6 +74,7 @@ box.schema.user.revoke('test_u', 'create', 'space')
 
 -- Synchronize
 vclock = test_run:get_vclock('autobootstrap1')
+vclock[0] = nil
 _ = test_run:wait_vclock("autobootstrap2", vclock)
 _ = test_run:wait_vclock("autobootstrap3", vclock)
 
@@ -95,6 +96,7 @@ _ = test_run:cmd("switch default")
 test_run:wait_fullmesh(SERVERS)
 
 vclock = test_run:get_vclock("autobootstrap1")
+vclock[0] = nil
 _ = test_run:wait_vclock("autobootstrap2", vclock)
 _ = test_run:wait_vclock("autobootstrap3", vclock)
 
diff --git a/test/replication/before_replace.result b/test/replication/before_replace.result
index ced40547e..61a552e84 100644
--- a/test/replication/before_replace.result
+++ b/test/replication/before_replace.result
@@ -266,7 +266,13 @@ box.space.test:replace{1, 1}
 ---
 - [1, 1]
 ...
-_ = test_run:wait_vclock('replica', test_run:get_vclock('default'))
+vclock = test_run:get_vclock('default')
+---
+...
+vclock[0] = nil
+---
+...
+_ = test_run:wait_vclock('replica', vclock)
 ---
 ...
 -- Check that replace{1, 2} coming from the master was suppressed
diff --git a/test/replication/before_replace.test.lua b/test/replication/before_replace.test.lua
index bcc6dc00d..ecd8ff044 100644
--- a/test/replication/before_replace.test.lua
+++ b/test/replication/before_replace.test.lua
@@ -101,7 +101,9 @@ _ = box.space.test:before_replace(function(old, new) return new:update{{'+', 2,
 test_run:cmd("switch default")
 box.space.test:replace{1, 1}
 
-_ = test_run:wait_vclock('replica', test_run:get_vclock('default'))
+vclock = test_run:get_vclock('default')
+vclock[0] = nil
+_ = test_run:wait_vclock('replica', vclock)
 
 -- Check that replace{1, 2} coming from the master was suppressed
 -- by the before_replace trigger on the replica.
diff --git a/test/replication/gh-4114-local-space-replication.result b/test/replication/gh-4114-local-space-replication.result
new file mode 100644
index 000000000..e524c9a1b
--- /dev/null
+++ b/test/replication/gh-4114-local-space-replication.result
@@ -0,0 +1,125 @@
+-- test-run result file version 2
+env = require('test_run')
+ | ---
+ | ...
+test_run = env.new()
+ | ---
+ | ...
+
+--
+-- gh-4114. Account local space changes in a separate vclock
+-- component. Do not replicate local space changes, even as NOPs.
+--
+
+box.schema.user.grant('guest', 'replication')
+ | ---
+ | ...
+_ = box.schema.space.create('test', {is_local=true})
+ | ---
+ | ...
+_ = box.space.test:create_index("pk")
+ | ---
+ | ...
+
+test_run:cmd('create server replica with rpl_master=default, script "replication/replica.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica with wait=True, wait_load=True')
+ | ---
+ | - true
+ | ...
+
+a = box.info.vclock[0] or 0
+ | ---
+ | ...
+for i = 1,10 do box.space.test:insert{i} end
+ | ---
+ | ...
+box.info.vclock[0] == a + 10 or box.info.vclock[0] - a
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('switch replica')
+ | ---
+ | - true
+ | ...
+box.info.vclock[0]
+ | ---
+ | - null
+ | ...
+box.cfg{checkpoint_count=1}
+ | ---
+ | ...
+box.space.test:select{}
+ | ---
+ | - []
+ | ...
+box.space.test:insert{1}
+ | ---
+ | - [1]
+ | ...
+box.snapshot()
+ | ---
+ | - ok
+ | ...
+box.space.test:insert{2}
+ | ---
+ | - [2]
+ | ...
+box.snapshot()
+ | ---
+ | - ok
+ | ...
+box.space.test:insert{3}
+ | ---
+ | - [3]
+ | ...
+box.snapshot()
+ | ---
+ | - ok
+ | ...
+
+box.info.vclock[0]
+ | ---
+ | - 3
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('set variable repl_source to "replica.listen"')
+ | ---
+ | - true
+ | ...
+
+box.cfg{replication=repl_source}
+ | ---
+ | ...
+test_run:wait_cond(function()\
+                       return box.info.replication[2].upstream and\
+                       box.info.replication[2].upstream.status == 'follow'\
+                   end,\
+                   10)
+ | ---
+ | - true
+ | ...
+
+-- Cleanup.
+test_run:cmd('stop server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica')
+ | ---
+ | - true
+ | ...
+box.space.test:drop()
+ | ---
+ | ...
+box.schema.user.revoke('guest', 'replication')
+ | ---
+ | ...
diff --git a/test/replication/gh-4114-local-space-replication.test.lua b/test/replication/gh-4114-local-space-replication.test.lua
new file mode 100644
index 000000000..26dccee68
--- /dev/null
+++ b/test/replication/gh-4114-local-space-replication.test.lua
@@ -0,0 +1,48 @@
+env = require('test_run')
+test_run = env.new()
+
+--
+-- gh-4114. Account local space changes in a separate vclock
+-- component. Do not replicate local space changes, even as NOPs.
+--
+
+box.schema.user.grant('guest', 'replication')
+_ = box.schema.space.create('test', {is_local=true})
+_ = box.space.test:create_index("pk")
+
+test_run:cmd('create server replica with rpl_master=default, script "replication/replica.lua"')
+test_run:cmd('start server replica with wait=True, wait_load=True')
+
+a = box.info.vclock[0] or 0
+for i = 1,10 do box.space.test:insert{i} end
+box.info.vclock[0] == a + 10 or box.info.vclock[0] - a
+
+test_run:cmd('switch replica')
+box.info.vclock[0]
+box.cfg{checkpoint_count=1}
+box.space.test:select{}
+box.space.test:insert{1}
+box.snapshot()
+box.space.test:insert{2}
+box.snapshot()
+box.space.test:insert{3}
+box.snapshot()
+
+box.info.vclock[0]
+
+test_run:cmd('switch default')
+
+test_run:cmd('set variable repl_source to "replica.listen"')
+
+box.cfg{replication=repl_source}
+test_run:wait_cond(function()\
+                       return box.info.replication[2].upstream and\
+                       box.info.replication[2].upstream.status == 'follow'\
+                   end,\
+                   10)
+
+-- Cleanup.
+test_run:cmd('stop server replica')
+test_run:cmd('delete server replica')
+box.space.test:drop()
+box.schema.user.revoke('guest', 'replication')
diff --git a/test/replication/local_spaces.result b/test/replication/local_spaces.result
index cf2c52010..4855d8a88 100644
--- a/test/replication/local_spaces.result
+++ b/test/replication/local_spaces.result
@@ -288,6 +288,10 @@ _ = s3:insert{3}
 vclock = test_run:get_vclock('default')
 ---
 ...
+-- Ignore 0-th component when waiting. They don't match.
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock('replica', vclock)
 ---
 ...
diff --git a/test/replication/local_spaces.test.lua b/test/replication/local_spaces.test.lua
index 373e2cd20..c5e224030 100644
--- a/test/replication/local_spaces.test.lua
+++ b/test/replication/local_spaces.test.lua
@@ -112,6 +112,9 @@ _ = s1:insert{3}
 _ = s2:insert{3}
 _ = s3:insert{3}
 vclock = test_run:get_vclock('default')
+
+-- Ignore 0-th component when waiting. They don't match.
+vclock[0] = nil
 _ = test_run:wait_vclock('replica', vclock)
 
 test_run:cmd("switch replica")
diff --git a/test/replication/misc.result b/test/replication/misc.result
index b63d72846..e5d1f560e 100644
--- a/test/replication/misc.result
+++ b/test/replication/misc.result
@@ -214,6 +214,9 @@ box.space.space1:select{}
 vclock = test_run:get_vclock("autobootstrap1")
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock("autobootstrap2", vclock)
 ---
 ...
@@ -414,6 +417,9 @@ while box.info.replication[2] == nil do fiber.sleep(0.01) end
 vclock = test_run:get_vclock('default')
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock('replica_auth', vclock)
 ---
 ...
diff --git a/test/replication/misc.test.lua b/test/replication/misc.test.lua
index c454a0992..d285b014a 100644
--- a/test/replication/misc.test.lua
+++ b/test/replication/misc.test.lua
@@ -88,6 +88,7 @@ c.space.space1:insert{box.NULL, "data"} -- fails, but bumps sequence value
 c.space.space1:insert{box.NULL, 1, "data"}
 box.space.space1:select{}
 vclock = test_run:get_vclock("autobootstrap1")
+vclock[0] = nil
 _ = test_run:wait_vclock("autobootstrap2", vclock)
 test_run:cmd("switch autobootstrap2")
 box.space.space1:select{}
@@ -172,6 +173,7 @@ box.schema.user.grant('cluster', 'replication')
 
 while box.info.replication[2] == nil do fiber.sleep(0.01) end
 vclock = test_run:get_vclock('default')
+vclock[0] = nil
 _ = test_run:wait_vclock('replica_auth', vclock)
 
 test_run:cmd("stop server replica_auth")
diff --git a/test/replication/quorum.result b/test/replication/quorum.result
index 07abe7f2a..5ef66bf0a 100644
--- a/test/replication/quorum.result
+++ b/test/replication/quorum.result
@@ -325,6 +325,9 @@ space:insert{2}
 vclock = test_run:get_vclock("default")
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock("replica", vclock)
 ---
 ...
@@ -390,6 +393,9 @@ box.cfg{replication = repl}
 vclock = test_run:get_vclock("master_quorum1")
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock("master_quorum2", vclock)
 ---
 ...
diff --git a/test/replication/quorum.test.lua b/test/replication/quorum.test.lua
index 5f2872675..da24f34a0 100644
--- a/test/replication/quorum.test.lua
+++ b/test/replication/quorum.test.lua
@@ -125,6 +125,7 @@ test_run:cmd("switch default")
 box.cfg{listen = listen}
 space:insert{2}
 vclock = test_run:get_vclock("default")
+vclock[0] = nil
 _ = test_run:wait_vclock("replica", vclock)
 test_run:cmd("switch replica")
 box.info.status -- running
@@ -145,6 +146,7 @@ box.cfg{replication = ""}
 box.space.test:insert{1}
 box.cfg{replication = repl}
 vclock = test_run:get_vclock("master_quorum1")
+vclock[0] = nil
 _ = test_run:wait_vclock("master_quorum2", vclock)
 test_run:cmd("switch master_quorum2")
 box.space.test:select()
diff --git a/test/replication/replica_rejoin.result b/test/replication/replica_rejoin.result
index b8ed79f14..dd04ae297 100644
--- a/test/replication/replica_rejoin.result
+++ b/test/replication/replica_rejoin.result
@@ -144,6 +144,9 @@ for i = 10, 30, 10 do box.space.test:update(i, {{'!', 1, i}}) end
 vclock = test_run:get_vclock('default')
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock('replica', vclock)
 ---
 ...
@@ -295,6 +298,9 @@ for i = 1, 10 do box.space.test:replace{2} end
 vclock = test_run:get_vclock('replica')
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock('default', vclock)
 ---
 ...
@@ -345,6 +351,9 @@ for i = 1, 10 do box.space.test:replace{2} end
 vclock = test_run:get_vclock('replica')
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock('default', vclock)
 ---
 ...
diff --git a/test/replication/replica_rejoin.test.lua b/test/replication/replica_rejoin.test.lua
index 25c0849ec..410ef44d7 100644
--- a/test/replication/replica_rejoin.test.lua
+++ b/test/replication/replica_rejoin.test.lua
@@ -55,6 +55,7 @@ test_run:cmd("switch default")
 -- Make sure the replica follows new changes.
 for i = 10, 30, 10 do box.space.test:update(i, {{'!', 1, i}}) end
 vclock = test_run:get_vclock('default')
+vclock[0] = nil
 _ = test_run:wait_vclock('replica', vclock)
 test_run:cmd("switch replica")
 box.space.test:select()
@@ -109,6 +110,7 @@ box.space.test:replace{1}
 test_run:cmd("switch replica")
 for i = 1, 10 do box.space.test:replace{2} end
 vclock = test_run:get_vclock('replica')
+vclock[0] = nil
 _ = test_run:wait_vclock('default', vclock)
 -- Restart the master and force garbage collection.
 test_run:cmd("switch default")
@@ -126,6 +128,7 @@ test_run:wait_cond(function() return #fio.glob(fio.pathjoin(box.cfg.wal_dir, '*.
 test_run:cmd("switch replica")
 for i = 1, 10 do box.space.test:replace{2} end
 vclock = test_run:get_vclock('replica')
+vclock[0] = nil
 _ = test_run:wait_vclock('default', vclock)
 -- Restart the replica. It should successfully rebootstrap.
 test_run:cmd("restart server replica with args='true'")
diff --git a/test/replication/skip_conflict_row.result b/test/replication/skip_conflict_row.result
index 9b2777872..d70ac8e2a 100644
--- a/test/replication/skip_conflict_row.result
+++ b/test/replication/skip_conflict_row.result
@@ -54,6 +54,9 @@ box.info.status
 vclock = test_run:get_vclock('default')
 ---
 ...
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock("replica", vclock)
 ---
 ...
diff --git a/test/replication/skip_conflict_row.test.lua b/test/replication/skip_conflict_row.test.lua
index 2982c730a..04fd08136 100644
--- a/test/replication/skip_conflict_row.test.lua
+++ b/test/replication/skip_conflict_row.test.lua
@@ -19,6 +19,7 @@ space:insert{2}
 box.info.status
 
 vclock = test_run:get_vclock('default')
+vclock[0] = nil
 _ = test_run:wait_vclock("replica", vclock)
 test_run:cmd("switch replica")
 box.info.replication[1].upstream.message
diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
index 90fd53ca6..0907ac17f 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -12,6 +12,7 @@
     "on_schema_init.test.lua": {},
     "long_row_timeout.test.lua": {},
     "join_without_snap.test.lua": {},
+    "gh-4114-local-space-replication.test.lua": {},
     "gh-4402-info-errno.test.lua": {},
     "gh-4605-empty-password.test.lua": {},
     "gh-4606-admin-creds.test.lua": {},
diff --git a/test/vinyl/errinj.result b/test/vinyl/errinj.result
index 2635da265..2bd701f70 100644
--- a/test/vinyl/errinj.result
+++ b/test/vinyl/errinj.result
@@ -1241,6 +1241,11 @@ test_run:cmd("switch default")
 vclock = test_run:get_vclock("default")
 ---
 ...
+-- Ignore 0-th vclock component. They don't match between
+-- replicas.
+vclock[0] = nil
+---
+...
 _ = test_run:wait_vclock("replica", vclock)
 ---
 ...
diff --git a/test/vinyl/errinj.test.lua b/test/vinyl/errinj.test.lua
index 4230cfae3..750d3bfe8 100644
--- a/test/vinyl/errinj.test.lua
+++ b/test/vinyl/errinj.test.lua
@@ -455,6 +455,10 @@ box.cfg{read_only = true}
 
 test_run:cmd("switch default")
 vclock = test_run:get_vclock("default")
+
+-- Ignore 0-th vclock component. They don't match between
+-- replicas.
+vclock[0] = nil
 _ = test_run:wait_vclock("replica", vclock)
 
 test_run:cmd("stop server replica")
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH v3 4/4] box: start counting local space requests separately

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [20/03/23 19:23]:
> Sign local space requests with a zero instance id. This allows to split
> local changes aside from the changes, which should be visible to the whole
> cluster, and stop sending NOPs to replicas to follow local vclock.

First of all, this is an unrelated fix and should be sent as a
separate series.

Second, I'm not sure how a separate fix is necessary, it seems
lexicographical vclock compare should close 4114.

An explanation and rationale for the fix belongs to the changeset
comment. Similar to how you explained GC bug.

Third, if server id 0 is special, it should be special on vclock api
level, not introduced with a bunch of hacks on replication level.

We should have vclock_signature() and vclock_local_signature()
(the latter includes server id 0), vclock_cmp and
vclock_local_cmp(), vclock_lexicographical_cmp() and
vclock_local_lexicographical_cmp() and so on.

This would make sure that we don't forget to "clear" server id 0
in some place which expects it.

Finally, I find using local vclock id for local changes very neat
and justifying introduction of local vclock id. I really thought
that such an ugly hack is not worth having for anonymous replicas
support. But together with skipping nops for local changes, it
actually begins to make sense. Fine, let's try to have this
feature, but let's implement it properly then.

> ---
>  src/box/box.cc                                |  20 +++
>  src/box/relay.cc                              |  15 +--
>  src/box/wal.c                                 |  16 ++-
>  test/replication/autobootstrap.result         |   6 +
>  test/replication/autobootstrap.test.lua       |   2 +
>  test/replication/before_replace.result        |   8 +-
>  test/replication/before_replace.test.lua      |   4 +-
>  .../gh-4114-local-space-replication.result    | 125 ++++++++++++++++++
>  .../gh-4114-local-space-replication.test.lua  |  48 +++++++
>  test/replication/local_spaces.result          |   4 +
>  test/replication/local_spaces.test.lua        |   3 +
>  test/replication/misc.result                  |   6 +
>  test/replication/misc.test.lua                |   2 +
>  test/replication/quorum.result                |   6 +
>  test/replication/quorum.test.lua              |   2 +
>  test/replication/replica_rejoin.result        |   9 ++
>  test/replication/replica_rejoin.test.lua      |   3 +
>  test/replication/skip_conflict_row.result     |   3 +
>  test/replication/skip_conflict_row.test.lua   |   1 +
>  test/replication/suite.cfg                    |   1 +
>  test/vinyl/errinj.result                      |   5 +
>  test/vinyl/errinj.test.lua                    |   4 +
>  22 files changed, 278 insertions(+), 15 deletions(-)
>  create mode 100644 test/replication/gh-4114-local-space-replication.result
>  create mode 100644 test/replication/gh-4114-local-space-replication.test.lua
> 
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 6de6b677c..b7ea0de2d 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -1882,6 +1882,26 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
>  	say_info("remote vclock %s local vclock %s",
>  		 vclock_to_string(&replica_clock), vclock_to_string(&vclock));
>  
> +	/*
> +	 * Replica clock is used in gc state and recovery
> +	 * initialization, so we need to replace the remote 0-th
> +	 * component with our own one. This makes recovery work
> +	 * correctly: we're trying to recover from an xlog whose
> +	 * vclock is smaller than remote replica clock in each
> +	 * component, exluding the 0-th component. This leads to
> +	 * finding the correct WAL, if it exists, since we do not
> +	 * need to recover local rows (the ones, that contribute
> +	 * to the 0-th vclock component). It would be bad to set
> +	 * 0-th vclock component to a smaller value, since it
> +	 * would unnecessarily require additional WALs, which may
> +	 * have already been deleted.
> +	 * Same stands for gc. Remote instances do not need this
> +	 * instance's local rows, and after gc was reworked to
> +	 * track individual vclock components instead of
> +	 * signatures it's safe to set the local component to the
> +	 * most recent value.
> +	 */
> +	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/relay.cc b/src/box/relay.cc
> index 21b7c0a6f..a91e4b771 100644
> --- a/src/box/relay.cc
> +++ b/src/box/relay.cc
> @@ -760,16 +760,15 @@ relay_send_row(struct xstream *stream, struct xrow_header *packet)
>  {
>  	struct relay *relay = container_of(stream, struct relay, stream);
>  	assert(iproto_type_is_dml(packet->type));
> -	/*
> -	 * Transform replica local requests to IPROTO_NOP so as to
> -	 * promote vclock on the replica without actually modifying
> -	 * any data.
> -	 */
>  	if (packet->group_id == GROUP_LOCAL) {
>  		/*
> -		 * Replica-local requests generated while replica
> -		 * was anonymous have a zero instance id. Just
> -		 * skip all these rows.
> +		 * We do not relay replica-local rows to other
> +		 * instances, since we started signing them with
> +		 * a zero instance id. However, if replica-local
> +		 * rows, signed with a non-zero id are present in
> +		 * our WAL, we still need to relay them as NOPs in
> +		 * order to correctly promote the vclock on the
> +		 * replica.
>  		 */
>  		if (packet->replica_id == REPLICA_ID_NIL)
>  			return;
> diff --git a/src/box/wal.c b/src/box/wal.c
> index 3b094b0e8..a74bdecd9 100644
> --- a/src/box/wal.c
> +++ b/src/box/wal.c
> @@ -953,13 +953,19 @@ wal_assign_lsn(struct vclock *vclock_diff, struct vclock *base,
>  	/** Assign LSN to all local rows. */
>  	for ( ; row < end; row++) {
>  		if ((*row)->replica_id == 0) {
> -			(*row)->lsn = vclock_inc(vclock_diff, instance_id) +
> -				      vclock_get(base, instance_id);
>  			/*
> -			 * Note, an anonymous replica signs local
> -			 * rows whith a zero instance id.
> +			 * All rows representing local space data
> +			 * manipulations are signed wth a zero
> +			 * instance id. This is also true for
> +			 * anonymous replicas, since they are
> +			 * only capable of writing to local and
> +			 * temporary spaces.
>  			 */
> -			(*row)->replica_id = instance_id;
> +			if ((*row)->group_id != GROUP_LOCAL)
> +				(*row)->replica_id = instance_id;
> +
> +			(*row)->lsn = vclock_inc(vclock_diff, (*row)->replica_id) +
> +						 vclock_get(base, (*row)->replica_id);
>  			/* Use lsn of the first local row as transaction id. */
>  			tsn = tsn == 0 ? (*row)->lsn : tsn;
>  			(*row)->tsn = tsn;
> diff --git a/test/replication/autobootstrap.result b/test/replication/autobootstrap.result
> index 743982d47..6918e23ea 100644
> --- a/test/replication/autobootstrap.result
> +++ b/test/replication/autobootstrap.result
> @@ -162,6 +162,9 @@ box.schema.user.revoke('test_u', 'create', 'space')
>  vclock = test_run:get_vclock('autobootstrap1')
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock("autobootstrap2", vclock)
>  ---
>  ...
> @@ -206,6 +209,9 @@ test_run:wait_fullmesh(SERVERS)
>  vclock = test_run:get_vclock("autobootstrap1")
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock("autobootstrap2", vclock)
>  ---
>  ...
> diff --git a/test/replication/autobootstrap.test.lua b/test/replication/autobootstrap.test.lua
> index 055ea4277..f8bb1c74a 100644
> --- a/test/replication/autobootstrap.test.lua
> +++ b/test/replication/autobootstrap.test.lua
> @@ -74,6 +74,7 @@ box.schema.user.revoke('test_u', 'create', 'space')
>  
>  -- Synchronize
>  vclock = test_run:get_vclock('autobootstrap1')
> +vclock[0] = nil
>  _ = test_run:wait_vclock("autobootstrap2", vclock)
>  _ = test_run:wait_vclock("autobootstrap3", vclock)
>  
> @@ -95,6 +96,7 @@ _ = test_run:cmd("switch default")
>  test_run:wait_fullmesh(SERVERS)
>  
>  vclock = test_run:get_vclock("autobootstrap1")
> +vclock[0] = nil
>  _ = test_run:wait_vclock("autobootstrap2", vclock)
>  _ = test_run:wait_vclock("autobootstrap3", vclock)
>  
> diff --git a/test/replication/before_replace.result b/test/replication/before_replace.result
> index ced40547e..61a552e84 100644
> --- a/test/replication/before_replace.result
> +++ b/test/replication/before_replace.result
> @@ -266,7 +266,13 @@ box.space.test:replace{1, 1}
>  ---
>  - [1, 1]
>  ...
> -_ = test_run:wait_vclock('replica', test_run:get_vclock('default'))
> +vclock = test_run:get_vclock('default')
> +---
> +...
> +vclock[0] = nil
> +---
> +...
> +_ = test_run:wait_vclock('replica', vclock)
>  ---
>  ...
>  -- Check that replace{1, 2} coming from the master was suppressed
> diff --git a/test/replication/before_replace.test.lua b/test/replication/before_replace.test.lua
> index bcc6dc00d..ecd8ff044 100644
> --- a/test/replication/before_replace.test.lua
> +++ b/test/replication/before_replace.test.lua
> @@ -101,7 +101,9 @@ _ = box.space.test:before_replace(function(old, new) return new:update{{'+', 2,
>  test_run:cmd("switch default")
>  box.space.test:replace{1, 1}
>  
> -_ = test_run:wait_vclock('replica', test_run:get_vclock('default'))
> +vclock = test_run:get_vclock('default')
> +vclock[0] = nil
> +_ = test_run:wait_vclock('replica', vclock)
>  
>  -- Check that replace{1, 2} coming from the master was suppressed
>  -- by the before_replace trigger on the replica.
> diff --git a/test/replication/gh-4114-local-space-replication.result b/test/replication/gh-4114-local-space-replication.result
> new file mode 100644
> index 000000000..e524c9a1b
> --- /dev/null
> +++ b/test/replication/gh-4114-local-space-replication.result
> @@ -0,0 +1,125 @@
> +-- test-run result file version 2
> +env = require('test_run')
> + | ---
> + | ...
> +test_run = env.new()
> + | ---
> + | ...
> +
> +--
> +-- gh-4114. Account local space changes in a separate vclock
> +-- component. Do not replicate local space changes, even as NOPs.
> +--
> +
> +box.schema.user.grant('guest', 'replication')
> + | ---
> + | ...
> +_ = box.schema.space.create('test', {is_local=true})
> + | ---
> + | ...
> +_ = box.space.test:create_index("pk")
> + | ---
> + | ...
> +
> +test_run:cmd('create server replica with rpl_master=default, script "replication/replica.lua"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica with wait=True, wait_load=True')
> + | ---
> + | - true
> + | ...
> +
> +a = box.info.vclock[0] or 0
> + | ---
> + | ...
> +for i = 1,10 do box.space.test:insert{i} end
> + | ---
> + | ...
> +box.info.vclock[0] == a + 10 or box.info.vclock[0] - a
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('switch replica')
> + | ---
> + | - true
> + | ...
> +box.info.vclock[0]
> + | ---
> + | - null
> + | ...
> +box.cfg{checkpoint_count=1}
> + | ---
> + | ...
> +box.space.test:select{}
> + | ---
> + | - []
> + | ...
> +box.space.test:insert{1}
> + | ---
> + | - [1]
> + | ...
> +box.snapshot()
> + | ---
> + | - ok
> + | ...
> +box.space.test:insert{2}
> + | ---
> + | - [2]
> + | ...
> +box.snapshot()
> + | ---
> + | - ok
> + | ...
> +box.space.test:insert{3}
> + | ---
> + | - [3]
> + | ...
> +box.snapshot()
> + | ---
> + | - ok
> + | ...
> +
> +box.info.vclock[0]
> + | ---
> + | - 3
> + | ...
> +
> +test_run:cmd('switch default')
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('set variable repl_source to "replica.listen"')
> + | ---
> + | - true
> + | ...
> +
> +box.cfg{replication=repl_source}
> + | ---
> + | ...
> +test_run:wait_cond(function()\
> +                       return box.info.replication[2].upstream and\
> +                       box.info.replication[2].upstream.status == 'follow'\
> +                   end,\
> +                   10)
> + | ---
> + | - true
> + | ...
> +
> +-- Cleanup.
> +test_run:cmd('stop server replica')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server replica')
> + | ---
> + | - true
> + | ...
> +box.space.test:drop()
> + | ---
> + | ...
> +box.schema.user.revoke('guest', 'replication')
> + | ---
> + | ...
> diff --git a/test/replication/gh-4114-local-space-replication.test.lua b/test/replication/gh-4114-local-space-replication.test.lua
> new file mode 100644
> index 000000000..26dccee68
> --- /dev/null
> +++ b/test/replication/gh-4114-local-space-replication.test.lua
> @@ -0,0 +1,48 @@
> +env = require('test_run')
> +test_run = env.new()
> +
> +--
> +-- gh-4114. Account local space changes in a separate vclock
> +-- component. Do not replicate local space changes, even as NOPs.
> +--
> +
> +box.schema.user.grant('guest', 'replication')
> +_ = box.schema.space.create('test', {is_local=true})
> +_ = box.space.test:create_index("pk")
> +
> +test_run:cmd('create server replica with rpl_master=default, script "replication/replica.lua"')
> +test_run:cmd('start server replica with wait=True, wait_load=True')
> +
> +a = box.info.vclock[0] or 0
> +for i = 1,10 do box.space.test:insert{i} end
> +box.info.vclock[0] == a + 10 or box.info.vclock[0] - a
> +
> +test_run:cmd('switch replica')
> +box.info.vclock[0]
> +box.cfg{checkpoint_count=1}
> +box.space.test:select{}
> +box.space.test:insert{1}
> +box.snapshot()
> +box.space.test:insert{2}
> +box.snapshot()
> +box.space.test:insert{3}
> +box.snapshot()
> +
> +box.info.vclock[0]
> +
> +test_run:cmd('switch default')
> +
> +test_run:cmd('set variable repl_source to "replica.listen"')
> +
> +box.cfg{replication=repl_source}
> +test_run:wait_cond(function()\
> +                       return box.info.replication[2].upstream and\
> +                       box.info.replication[2].upstream.status == 'follow'\
> +                   end,\
> +                   10)
> +
> +-- Cleanup.
> +test_run:cmd('stop server replica')
> +test_run:cmd('delete server replica')
> +box.space.test:drop()
> +box.schema.user.revoke('guest', 'replication')
> diff --git a/test/replication/local_spaces.result b/test/replication/local_spaces.result
> index cf2c52010..4855d8a88 100644
> --- a/test/replication/local_spaces.result
> +++ b/test/replication/local_spaces.result
> @@ -288,6 +288,10 @@ _ = s3:insert{3}
>  vclock = test_run:get_vclock('default')
>  ---
>  ...
> +-- Ignore 0-th component when waiting. They don't match.
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock('replica', vclock)
>  ---
>  ...
> diff --git a/test/replication/local_spaces.test.lua b/test/replication/local_spaces.test.lua
> index 373e2cd20..c5e224030 100644
> --- a/test/replication/local_spaces.test.lua
> +++ b/test/replication/local_spaces.test.lua
> @@ -112,6 +112,9 @@ _ = s1:insert{3}
>  _ = s2:insert{3}
>  _ = s3:insert{3}
>  vclock = test_run:get_vclock('default')
> +
> +-- Ignore 0-th component when waiting. They don't match.
> +vclock[0] = nil
>  _ = test_run:wait_vclock('replica', vclock)
>  
>  test_run:cmd("switch replica")
> diff --git a/test/replication/misc.result b/test/replication/misc.result
> index b63d72846..e5d1f560e 100644
> --- a/test/replication/misc.result
> +++ b/test/replication/misc.result
> @@ -214,6 +214,9 @@ box.space.space1:select{}
>  vclock = test_run:get_vclock("autobootstrap1")
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock("autobootstrap2", vclock)
>  ---
>  ...
> @@ -414,6 +417,9 @@ while box.info.replication[2] == nil do fiber.sleep(0.01) end
>  vclock = test_run:get_vclock('default')
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock('replica_auth', vclock)
>  ---
>  ...
> diff --git a/test/replication/misc.test.lua b/test/replication/misc.test.lua
> index c454a0992..d285b014a 100644
> --- a/test/replication/misc.test.lua
> +++ b/test/replication/misc.test.lua
> @@ -88,6 +88,7 @@ c.space.space1:insert{box.NULL, "data"} -- fails, but bumps sequence value
>  c.space.space1:insert{box.NULL, 1, "data"}
>  box.space.space1:select{}
>  vclock = test_run:get_vclock("autobootstrap1")
> +vclock[0] = nil
>  _ = test_run:wait_vclock("autobootstrap2", vclock)
>  test_run:cmd("switch autobootstrap2")
>  box.space.space1:select{}
> @@ -172,6 +173,7 @@ box.schema.user.grant('cluster', 'replication')
>  
>  while box.info.replication[2] == nil do fiber.sleep(0.01) end
>  vclock = test_run:get_vclock('default')
> +vclock[0] = nil
>  _ = test_run:wait_vclock('replica_auth', vclock)
>  
>  test_run:cmd("stop server replica_auth")
> diff --git a/test/replication/quorum.result b/test/replication/quorum.result
> index 07abe7f2a..5ef66bf0a 100644
> --- a/test/replication/quorum.result
> +++ b/test/replication/quorum.result
> @@ -325,6 +325,9 @@ space:insert{2}
>  vclock = test_run:get_vclock("default")
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock("replica", vclock)
>  ---
>  ...
> @@ -390,6 +393,9 @@ box.cfg{replication = repl}
>  vclock = test_run:get_vclock("master_quorum1")
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock("master_quorum2", vclock)
>  ---
>  ...
> diff --git a/test/replication/quorum.test.lua b/test/replication/quorum.test.lua
> index 5f2872675..da24f34a0 100644
> --- a/test/replication/quorum.test.lua
> +++ b/test/replication/quorum.test.lua
> @@ -125,6 +125,7 @@ test_run:cmd("switch default")
>  box.cfg{listen = listen}
>  space:insert{2}
>  vclock = test_run:get_vclock("default")
> +vclock[0] = nil
>  _ = test_run:wait_vclock("replica", vclock)
>  test_run:cmd("switch replica")
>  box.info.status -- running
> @@ -145,6 +146,7 @@ box.cfg{replication = ""}
>  box.space.test:insert{1}
>  box.cfg{replication = repl}
>  vclock = test_run:get_vclock("master_quorum1")
> +vclock[0] = nil
>  _ = test_run:wait_vclock("master_quorum2", vclock)
>  test_run:cmd("switch master_quorum2")
>  box.space.test:select()
> diff --git a/test/replication/replica_rejoin.result b/test/replication/replica_rejoin.result
> index b8ed79f14..dd04ae297 100644
> --- a/test/replication/replica_rejoin.result
> +++ b/test/replication/replica_rejoin.result
> @@ -144,6 +144,9 @@ for i = 10, 30, 10 do box.space.test:update(i, {{'!', 1, i}}) end
>  vclock = test_run:get_vclock('default')
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock('replica', vclock)
>  ---
>  ...
> @@ -295,6 +298,9 @@ for i = 1, 10 do box.space.test:replace{2} end
>  vclock = test_run:get_vclock('replica')
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock('default', vclock)
>  ---
>  ...
> @@ -345,6 +351,9 @@ for i = 1, 10 do box.space.test:replace{2} end
>  vclock = test_run:get_vclock('replica')
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock('default', vclock)
>  ---
>  ...
> diff --git a/test/replication/replica_rejoin.test.lua b/test/replication/replica_rejoin.test.lua
> index 25c0849ec..410ef44d7 100644
> --- a/test/replication/replica_rejoin.test.lua
> +++ b/test/replication/replica_rejoin.test.lua
> @@ -55,6 +55,7 @@ test_run:cmd("switch default")
>  -- Make sure the replica follows new changes.
>  for i = 10, 30, 10 do box.space.test:update(i, {{'!', 1, i}}) end
>  vclock = test_run:get_vclock('default')
> +vclock[0] = nil
>  _ = test_run:wait_vclock('replica', vclock)
>  test_run:cmd("switch replica")
>  box.space.test:select()
> @@ -109,6 +110,7 @@ box.space.test:replace{1}
>  test_run:cmd("switch replica")
>  for i = 1, 10 do box.space.test:replace{2} end
>  vclock = test_run:get_vclock('replica')
> +vclock[0] = nil
>  _ = test_run:wait_vclock('default', vclock)
>  -- Restart the master and force garbage collection.
>  test_run:cmd("switch default")
> @@ -126,6 +128,7 @@ test_run:wait_cond(function() return #fio.glob(fio.pathjoin(box.cfg.wal_dir, '*.
>  test_run:cmd("switch replica")
>  for i = 1, 10 do box.space.test:replace{2} end
>  vclock = test_run:get_vclock('replica')
> +vclock[0] = nil
>  _ = test_run:wait_vclock('default', vclock)
>  -- Restart the replica. It should successfully rebootstrap.
>  test_run:cmd("restart server replica with args='true'")
> diff --git a/test/replication/skip_conflict_row.result b/test/replication/skip_conflict_row.result
> index 9b2777872..d70ac8e2a 100644
> --- a/test/replication/skip_conflict_row.result
> +++ b/test/replication/skip_conflict_row.result
> @@ -54,6 +54,9 @@ box.info.status
>  vclock = test_run:get_vclock('default')
>  ---
>  ...
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock("replica", vclock)
>  ---
>  ...
> diff --git a/test/replication/skip_conflict_row.test.lua b/test/replication/skip_conflict_row.test.lua
> index 2982c730a..04fd08136 100644
> --- a/test/replication/skip_conflict_row.test.lua
> +++ b/test/replication/skip_conflict_row.test.lua
> @@ -19,6 +19,7 @@ space:insert{2}
>  box.info.status
>  
>  vclock = test_run:get_vclock('default')
> +vclock[0] = nil
>  _ = test_run:wait_vclock("replica", vclock)
>  test_run:cmd("switch replica")
>  box.info.replication[1].upstream.message
> diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
> index 90fd53ca6..0907ac17f 100644
> --- a/test/replication/suite.cfg
> +++ b/test/replication/suite.cfg
> @@ -12,6 +12,7 @@
>      "on_schema_init.test.lua": {},
>      "long_row_timeout.test.lua": {},
>      "join_without_snap.test.lua": {},
> +    "gh-4114-local-space-replication.test.lua": {},
>      "gh-4402-info-errno.test.lua": {},
>      "gh-4605-empty-password.test.lua": {},
>      "gh-4606-admin-creds.test.lua": {},
> diff --git a/test/vinyl/errinj.result b/test/vinyl/errinj.result
> index 2635da265..2bd701f70 100644
> --- a/test/vinyl/errinj.result
> +++ b/test/vinyl/errinj.result
> @@ -1241,6 +1241,11 @@ test_run:cmd("switch default")
>  vclock = test_run:get_vclock("default")
>  ---
>  ...
> +-- Ignore 0-th vclock component. They don't match between
> +-- replicas.
> +vclock[0] = nil
> +---
> +...
>  _ = test_run:wait_vclock("replica", vclock)
>  ---
>  ...
> diff --git a/test/vinyl/errinj.test.lua b/test/vinyl/errinj.test.lua
> index 4230cfae3..750d3bfe8 100644
> --- a/test/vinyl/errinj.test.lua
> +++ b/test/vinyl/errinj.test.lua
> @@ -455,6 +455,10 @@ box.cfg{read_only = true}
>  
>  test_run:cmd("switch default")
>  vclock = test_run:get_vclock("default")
> +
> +-- Ignore 0-th vclock component. They don't match between
> +-- replicas.
> +vclock[0] = nil
>  _ = test_run:wait_vclock("replica", vclock)
>  
>  test_run:cmd("stop server replica")
> -- 
> 2.21.1 (Apple Git-122.3)
> 

-- 
Konstantin Osipov, Moscow, Russia
https://scylladb.com

Re: [Tarantool-patches] [PATCH v3 4/4] box: start counting local space requests separately

[Serge Petrenko]

Hi! Thank you for the review!

> 23 марта 2020 г., в 19:51, Konstantin Osipov <kostja.osipov@gmail.com> написал(а):
> 
> * Serge Petrenko <sergepetrenko@tarantool.org> [20/03/23 19:23]:
>> Sign local space requests with a zero instance id. This allows to split
>> local changes aside from the changes, which should be visible to the whole
>> cluster, and stop sending NOPs to replicas to follow local vclock.
> 
> First of all, this is an unrelated fix and should be sent as a
> separate series.
> 
> Second, I'm not sure how a separate fix is necessary, it seems
> lexicographical vclock compare should close 4114.

Well, no. I described why this fix is needed in the commit message,
as you requested.

I can send a separate patchset which just reworks gc to work with
clocks instead of signatures, if you want me to.
It’s a nice feature to have on its own, but it was done in scope of
fixing this bug (#4114), and it is a prerequisite for this fix, so I
don’t think it’s worth splitting them apart.

> 
> An explanation and rationale for the fix belongs to the changeset
> comment. Similar to how you explained GC bug.

Done.

> 
> Third, if server id 0 is special, it should be special on vclock api
> level, not introduced with a bunch of hacks on replication level.
> 
> We should have vclock_signature() and vclock_local_signature()
> (the latter includes server id 0), vclock_cmp and
> vclock_local_cmp(), vclock_lexicographical_cmp() and
> vclock_local_lexicographical_cmp() and so on.
> 
> This would make sure that we don't forget to "clear" server id 0
> in some place which expects it.

If I understood correctly, you mean that we have to rewrite recovery
so that it accepts a flag, whether a local recovery is needed, or a
relay one, when rows with id 0 are ignored. I’m not sure it’ll work
correctly or look better than hacking replica vclock in one place.
If you insist on rewriting recovery, I’ll do that, but please see the
patchset first.

Speaking of other places where server id 0 is cleared (commit #3),
it’s still needed for backward compatibility. Older replicas do not
ignore server id 0.

I reworked gc though, so now it doesn’t depend on replica id 0 at all,
and «clearing» server id 0 is now needed only for recovery.
(I added vclock_min_ignore0, to be consistent with vclock_compare_ignore0
Feel free to propose a better name, if you want to)

> 
> Finally, I find using local vclock id for local changes very neat
> and justifying introduction of local vclock id. I really thought
> that such an ugly hack is not worth having for anonymous replicas
> support. But together with skipping nops for local changes, it
> actually begins to make sense. Fine, let's try to have this
> feature, but let's implement it properly then.
> 
>> ---
>> src/box/box.cc                                |  20 +++
>> src/box/relay.cc                              |  15 +--
>> src/box/wal.c                                 |  16 ++-
>> test/replication/autobootstrap.result         |   6 +
>> test/replication/autobootstrap.test.lua       |   2 +
>> test/replication/before_replace.result        |   8 +-
>> test/replication/before_replace.test.lua      |   4 +-
>> .../gh-4114-local-space-replication.result    | 125 ++++++++++++++++++
>> .../gh-4114-local-space-replication.test.lua  |  48 +++++++
>> test/replication/local_spaces.result          |   4 +
>> test/replication/local_spaces.test.lua        |   3 +
>> test/replication/misc.result                  |   6 +
>> test/replication/misc.test.lua                |   2 +
>> test/replication/quorum.result                |   6 +
>> test/replication/quorum.test.lua              |   2 +
>> test/replication/replica_rejoin.result        |   9 ++
>> test/replication/replica_rejoin.test.lua      |   3 +
>> test/replication/skip_conflict_row.result     |   3 +
>> test/replication/skip_conflict_row.test.lua   |   1 +
>> test/replication/suite.cfg                    |   1 +
>> test/vinyl/errinj.result                      |   5 +
>> test/vinyl/errinj.test.lua                    |   4 +
>> 22 files changed, 278 insertions(+), 15 deletions(-)
>> create mode 100644 test/replication/gh-4114-local-space-replication.result
>> create mode 100644 test/replication/gh-4114-local-space-replication.test.lua
>> 
>> diff --git a/src/box/box.cc b/src/box/box.cc
>> index 6de6b677c..b7ea0de2d 100644
>> --- a/src/box/box.cc
>> +++ b/src/box/box.cc
>> @@ -1882,6 +1882,26 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
>> 	say_info("remote vclock %s local vclock %s",
>> 		 vclock_to_string(&replica_clock), vclock_to_string(&vclock));
>> 
>> +	/*
>> +	 * Replica clock is used in gc state and recovery
>> +	 * initialization, so we need to replace the remote 0-th
>> +	 * component with our own one. This makes recovery work
>> +	 * correctly: we're trying to recover from an xlog whose
>> +	 * vclock is smaller than remote replica clock in each
>> +	 * component, exluding the 0-th component. This leads to
>> +	 * finding the correct WAL, if it exists, since we do not
>> +	 * need to recover local rows (the ones, that contribute
>> +	 * to the 0-th vclock component). It would be bad to set
>> +	 * 0-th vclock component to a smaller value, since it
>> +	 * would unnecessarily require additional WALs, which may
>> +	 * have already been deleted.
>> +	 * Same stands for gc. Remote instances do not need this
>> +	 * instance's local rows, and after gc was reworked to
>> +	 * track individual vclock components instead of
>> +	 * signatures it's safe to set the local component to the
>> +	 * most recent value.
>> +	 */
>> +	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/relay.cc b/src/box/relay.cc
>> index 21b7c0a6f..a91e4b771 100644
>> --- a/src/box/relay.cc
>> +++ b/src/box/relay.cc
>> @@ -760,16 +760,15 @@ relay_send_row(struct xstream *stream, struct xrow_header *packet)
>> {
>> 	struct relay *relay = container_of(stream, struct relay, stream);
>> 	assert(iproto_type_is_dml(packet->type));
>> -	/*
>> -	 * Transform replica local requests to IPROTO_NOP so as to
>> -	 * promote vclock on the replica without actually modifying
>> -	 * any data.
>> -	 */
>> 	if (packet->group_id == GROUP_LOCAL) {
>> 		/*
>> -		 * Replica-local requests generated while replica
>> -		 * was anonymous have a zero instance id. Just
>> -		 * skip all these rows.
>> +		 * We do not relay replica-local rows to other
>> +		 * instances, since we started signing them with
>> +		 * a zero instance id. However, if replica-local
>> +		 * rows, signed with a non-zero id are present in
>> +		 * our WAL, we still need to relay them as NOPs in
>> +		 * order to correctly promote the vclock on the
>> +		 * replica.
>> 		 */
>> 		if (packet->replica_id == REPLICA_ID_NIL)
>> 			return;
>> diff --git a/src/box/wal.c b/src/box/wal.c
>> index 3b094b0e8..a74bdecd9 100644
>> --- a/src/box/wal.c
>> +++ b/src/box/wal.c
>> @@ -953,13 +953,19 @@ wal_assign_lsn(struct vclock *vclock_diff, struct vclock *base,
>> 	/** Assign LSN to all local rows. */
>> 	for ( ; row < end; row++) {
>> 		if ((*row)->replica_id == 0) {
>> -			(*row)->lsn = vclock_inc(vclock_diff, instance_id) +
>> -				      vclock_get(base, instance_id);
>> 			/*
>> -			 * Note, an anonymous replica signs local
>> -			 * rows whith a zero instance id.
>> +			 * All rows representing local space data
>> +			 * manipulations are signed wth a zero
>> +			 * instance id. This is also true for
>> +			 * anonymous replicas, since they are
>> +			 * only capable of writing to local and
>> +			 * temporary spaces.
>> 			 */
>> -			(*row)->replica_id = instance_id;
>> +			if ((*row)->group_id != GROUP_LOCAL)
>> +				(*row)->replica_id = instance_id;
>> +
>> +			(*row)->lsn = vclock_inc(vclock_diff, (*row)->replica_id) +
>> +						 vclock_get(base, (*row)->replica_id);
>> 			/* Use lsn of the first local row as transaction id. */
>> 			tsn = tsn == 0 ? (*row)->lsn : tsn;
>> 			(*row)->tsn = tsn;
>> diff --git a/test/replication/autobootstrap.result b/test/replication/autobootstrap.result
>> index 743982d47..6918e23ea 100644
>> --- a/test/replication/autobootstrap.result
>> +++ b/test/replication/autobootstrap.result
>> @@ -162,6 +162,9 @@ box.schema.user.revoke('test_u', 'create', 'space')
>> vclock = test_run:get_vclock('autobootstrap1')
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock("autobootstrap2", vclock)
>> ---
>> ...
>> @@ -206,6 +209,9 @@ test_run:wait_fullmesh(SERVERS)
>> vclock = test_run:get_vclock("autobootstrap1")
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock("autobootstrap2", vclock)
>> ---
>> ...
>> diff --git a/test/replication/autobootstrap.test.lua b/test/replication/autobootstrap.test.lua
>> index 055ea4277..f8bb1c74a 100644
>> --- a/test/replication/autobootstrap.test.lua
>> +++ b/test/replication/autobootstrap.test.lua
>> @@ -74,6 +74,7 @@ box.schema.user.revoke('test_u', 'create', 'space')
>> 
>> -- Synchronize
>> vclock = test_run:get_vclock('autobootstrap1')
>> +vclock[0] = nil
>> _ = test_run:wait_vclock("autobootstrap2", vclock)
>> _ = test_run:wait_vclock("autobootstrap3", vclock)
>> 
>> @@ -95,6 +96,7 @@ _ = test_run:cmd("switch default")
>> test_run:wait_fullmesh(SERVERS)
>> 
>> vclock = test_run:get_vclock("autobootstrap1")
>> +vclock[0] = nil
>> _ = test_run:wait_vclock("autobootstrap2", vclock)
>> _ = test_run:wait_vclock("autobootstrap3", vclock)
>> 
>> diff --git a/test/replication/before_replace.result b/test/replication/before_replace.result
>> index ced40547e..61a552e84 100644
>> --- a/test/replication/before_replace.result
>> +++ b/test/replication/before_replace.result
>> @@ -266,7 +266,13 @@ box.space.test:replace{1, 1}
>> ---
>> - [1, 1]
>> ...
>> -_ = test_run:wait_vclock('replica', test_run:get_vclock('default'))
>> +vclock = test_run:get_vclock('default')
>> +---
>> +...
>> +vclock[0] = nil
>> +---
>> +...
>> +_ = test_run:wait_vclock('replica', vclock)
>> ---
>> ...
>> -- Check that replace{1, 2} coming from the master was suppressed
>> diff --git a/test/replication/before_replace.test.lua b/test/replication/before_replace.test.lua
>> index bcc6dc00d..ecd8ff044 100644
>> --- a/test/replication/before_replace.test.lua
>> +++ b/test/replication/before_replace.test.lua
>> @@ -101,7 +101,9 @@ _ = box.space.test:before_replace(function(old, new) return new:update{{'+', 2,
>> test_run:cmd("switch default")
>> box.space.test:replace{1, 1}
>> 
>> -_ = test_run:wait_vclock('replica', test_run:get_vclock('default'))
>> +vclock = test_run:get_vclock('default')
>> +vclock[0] = nil
>> +_ = test_run:wait_vclock('replica', vclock)
>> 
>> -- Check that replace{1, 2} coming from the master was suppressed
>> -- by the before_replace trigger on the replica.
>> diff --git a/test/replication/gh-4114-local-space-replication.result b/test/replication/gh-4114-local-space-replication.result
>> new file mode 100644
>> index 000000000..e524c9a1b
>> --- /dev/null
>> +++ b/test/replication/gh-4114-local-space-replication.result
>> @@ -0,0 +1,125 @@
>> +-- test-run result file version 2
>> +env = require('test_run')
>> + | ---
>> + | ...
>> +test_run = env.new()
>> + | ---
>> + | ...
>> +
>> +--
>> +-- gh-4114. Account local space changes in a separate vclock
>> +-- component. Do not replicate local space changes, even as NOPs.
>> +--
>> +
>> +box.schema.user.grant('guest', 'replication')
>> + | ---
>> + | ...
>> +_ = box.schema.space.create('test', {is_local=true})
>> + | ---
>> + | ...
>> +_ = box.space.test:create_index("pk")
>> + | ---
>> + | ...
>> +
>> +test_run:cmd('create server replica with rpl_master=default, script "replication/replica.lua"')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd('start server replica with wait=True, wait_load=True')
>> + | ---
>> + | - true
>> + | ...
>> +
>> +a = box.info.vclock[0] or 0
>> + | ---
>> + | ...
>> +for i = 1,10 do box.space.test:insert{i} end
>> + | ---
>> + | ...
>> +box.info.vclock[0] == a + 10 or box.info.vclock[0] - a
>> + | ---
>> + | - true
>> + | ...
>> +
>> +test_run:cmd('switch replica')
>> + | ---
>> + | - true
>> + | ...
>> +box.info.vclock[0]
>> + | ---
>> + | - null
>> + | ...
>> +box.cfg{checkpoint_count=1}
>> + | ---
>> + | ...
>> +box.space.test:select{}
>> + | ---
>> + | - []
>> + | ...
>> +box.space.test:insert{1}
>> + | ---
>> + | - [1]
>> + | ...
>> +box.snapshot()
>> + | ---
>> + | - ok
>> + | ...
>> +box.space.test:insert{2}
>> + | ---
>> + | - [2]
>> + | ...
>> +box.snapshot()
>> + | ---
>> + | - ok
>> + | ...
>> +box.space.test:insert{3}
>> + | ---
>> + | - [3]
>> + | ...
>> +box.snapshot()
>> + | ---
>> + | - ok
>> + | ...
>> +
>> +box.info.vclock[0]
>> + | ---
>> + | - 3
>> + | ...
>> +
>> +test_run:cmd('switch default')
>> + | ---
>> + | - true
>> + | ...
>> +
>> +test_run:cmd('set variable repl_source to "replica.listen"')
>> + | ---
>> + | - true
>> + | ...
>> +
>> +box.cfg{replication=repl_source}
>> + | ---
>> + | ...
>> +test_run:wait_cond(function()\
>> +                       return box.info.replication[2].upstream and\
>> +                       box.info.replication[2].upstream.status == 'follow'\
>> +                   end,\
>> +                   10)
>> + | ---
>> + | - true
>> + | ...
>> +
>> +-- Cleanup.
>> +test_run:cmd('stop server replica')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd('delete server replica')
>> + | ---
>> + | - true
>> + | ...
>> +box.space.test:drop()
>> + | ---
>> + | ...
>> +box.schema.user.revoke('guest', 'replication')
>> + | ---
>> + | ...
>> diff --git a/test/replication/gh-4114-local-space-replication.test.lua b/test/replication/gh-4114-local-space-replication.test.lua
>> new file mode 100644
>> index 000000000..26dccee68
>> --- /dev/null
>> +++ b/test/replication/gh-4114-local-space-replication.test.lua
>> @@ -0,0 +1,48 @@
>> +env = require('test_run')
>> +test_run = env.new()
>> +
>> +--
>> +-- gh-4114. Account local space changes in a separate vclock
>> +-- component. Do not replicate local space changes, even as NOPs.
>> +--
>> +
>> +box.schema.user.grant('guest', 'replication')
>> +_ = box.schema.space.create('test', {is_local=true})
>> +_ = box.space.test:create_index("pk")
>> +
>> +test_run:cmd('create server replica with rpl_master=default, script "replication/replica.lua"')
>> +test_run:cmd('start server replica with wait=True, wait_load=True')
>> +
>> +a = box.info.vclock[0] or 0
>> +for i = 1,10 do box.space.test:insert{i} end
>> +box.info.vclock[0] == a + 10 or box.info.vclock[0] - a
>> +
>> +test_run:cmd('switch replica')
>> +box.info.vclock[0]
>> +box.cfg{checkpoint_count=1}
>> +box.space.test:select{}
>> +box.space.test:insert{1}
>> +box.snapshot()
>> +box.space.test:insert{2}
>> +box.snapshot()
>> +box.space.test:insert{3}
>> +box.snapshot()
>> +
>> +box.info.vclock[0]
>> +
>> +test_run:cmd('switch default')
>> +
>> +test_run:cmd('set variable repl_source to "replica.listen"')
>> +
>> +box.cfg{replication=repl_source}
>> +test_run:wait_cond(function()\
>> +                       return box.info.replication[2].upstream and\
>> +                       box.info.replication[2].upstream.status == 'follow'\
>> +                   end,\
>> +                   10)
>> +
>> +-- Cleanup.
>> +test_run:cmd('stop server replica')
>> +test_run:cmd('delete server replica')
>> +box.space.test:drop()
>> +box.schema.user.revoke('guest', 'replication')
>> diff --git a/test/replication/local_spaces.result b/test/replication/local_spaces.result
>> index cf2c52010..4855d8a88 100644
>> --- a/test/replication/local_spaces.result
>> +++ b/test/replication/local_spaces.result
>> @@ -288,6 +288,10 @@ _ = s3:insert{3}
>> vclock = test_run:get_vclock('default')
>> ---
>> ...
>> +-- Ignore 0-th component when waiting. They don't match.
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock('replica', vclock)
>> ---
>> ...
>> diff --git a/test/replication/local_spaces.test.lua b/test/replication/local_spaces.test.lua
>> index 373e2cd20..c5e224030 100644
>> --- a/test/replication/local_spaces.test.lua
>> +++ b/test/replication/local_spaces.test.lua
>> @@ -112,6 +112,9 @@ _ = s1:insert{3}
>> _ = s2:insert{3}
>> _ = s3:insert{3}
>> vclock = test_run:get_vclock('default')
>> +
>> +-- Ignore 0-th component when waiting. They don't match.
>> +vclock[0] = nil
>> _ = test_run:wait_vclock('replica', vclock)
>> 
>> test_run:cmd("switch replica")
>> diff --git a/test/replication/misc.result b/test/replication/misc.result
>> index b63d72846..e5d1f560e 100644
>> --- a/test/replication/misc.result
>> +++ b/test/replication/misc.result
>> @@ -214,6 +214,9 @@ box.space.space1:select{}
>> vclock = test_run:get_vclock("autobootstrap1")
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock("autobootstrap2", vclock)
>> ---
>> ...
>> @@ -414,6 +417,9 @@ while box.info.replication[2] == nil do fiber.sleep(0.01) end
>> vclock = test_run:get_vclock('default')
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock('replica_auth', vclock)
>> ---
>> ...
>> diff --git a/test/replication/misc.test.lua b/test/replication/misc.test.lua
>> index c454a0992..d285b014a 100644
>> --- a/test/replication/misc.test.lua
>> +++ b/test/replication/misc.test.lua
>> @@ -88,6 +88,7 @@ c.space.space1:insert{box.NULL, "data"} -- fails, but bumps sequence value
>> c.space.space1:insert{box.NULL, 1, "data"}
>> box.space.space1:select{}
>> vclock = test_run:get_vclock("autobootstrap1")
>> +vclock[0] = nil
>> _ = test_run:wait_vclock("autobootstrap2", vclock)
>> test_run:cmd("switch autobootstrap2")
>> box.space.space1:select{}
>> @@ -172,6 +173,7 @@ box.schema.user.grant('cluster', 'replication')
>> 
>> while box.info.replication[2] == nil do fiber.sleep(0.01) end
>> vclock = test_run:get_vclock('default')
>> +vclock[0] = nil
>> _ = test_run:wait_vclock('replica_auth', vclock)
>> 
>> test_run:cmd("stop server replica_auth")
>> diff --git a/test/replication/quorum.result b/test/replication/quorum.result
>> index 07abe7f2a..5ef66bf0a 100644
>> --- a/test/replication/quorum.result
>> +++ b/test/replication/quorum.result
>> @@ -325,6 +325,9 @@ space:insert{2}
>> vclock = test_run:get_vclock("default")
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock("replica", vclock)
>> ---
>> ...
>> @@ -390,6 +393,9 @@ box.cfg{replication = repl}
>> vclock = test_run:get_vclock("master_quorum1")
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock("master_quorum2", vclock)
>> ---
>> ...
>> diff --git a/test/replication/quorum.test.lua b/test/replication/quorum.test.lua
>> index 5f2872675..da24f34a0 100644
>> --- a/test/replication/quorum.test.lua
>> +++ b/test/replication/quorum.test.lua
>> @@ -125,6 +125,7 @@ test_run:cmd("switch default")
>> box.cfg{listen = listen}
>> space:insert{2}
>> vclock = test_run:get_vclock("default")
>> +vclock[0] = nil
>> _ = test_run:wait_vclock("replica", vclock)
>> test_run:cmd("switch replica")
>> box.info.status -- running
>> @@ -145,6 +146,7 @@ box.cfg{replication = ""}
>> box.space.test:insert{1}
>> box.cfg{replication = repl}
>> vclock = test_run:get_vclock("master_quorum1")
>> +vclock[0] = nil
>> _ = test_run:wait_vclock("master_quorum2", vclock)
>> test_run:cmd("switch master_quorum2")
>> box.space.test:select()
>> diff --git a/test/replication/replica_rejoin.result b/test/replication/replica_rejoin.result
>> index b8ed79f14..dd04ae297 100644
>> --- a/test/replication/replica_rejoin.result
>> +++ b/test/replication/replica_rejoin.result
>> @@ -144,6 +144,9 @@ for i = 10, 30, 10 do box.space.test:update(i, {{'!', 1, i}}) end
>> vclock = test_run:get_vclock('default')
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock('replica', vclock)
>> ---
>> ...
>> @@ -295,6 +298,9 @@ for i = 1, 10 do box.space.test:replace{2} end
>> vclock = test_run:get_vclock('replica')
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock('default', vclock)
>> ---
>> ...
>> @@ -345,6 +351,9 @@ for i = 1, 10 do box.space.test:replace{2} end
>> vclock = test_run:get_vclock('replica')
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock('default', vclock)
>> ---
>> ...
>> diff --git a/test/replication/replica_rejoin.test.lua b/test/replication/replica_rejoin.test.lua
>> index 25c0849ec..410ef44d7 100644
>> --- a/test/replication/replica_rejoin.test.lua
>> +++ b/test/replication/replica_rejoin.test.lua
>> @@ -55,6 +55,7 @@ test_run:cmd("switch default")
>> -- Make sure the replica follows new changes.
>> for i = 10, 30, 10 do box.space.test:update(i, {{'!', 1, i}}) end
>> vclock = test_run:get_vclock('default')
>> +vclock[0] = nil
>> _ = test_run:wait_vclock('replica', vclock)
>> test_run:cmd("switch replica")
>> box.space.test:select()
>> @@ -109,6 +110,7 @@ box.space.test:replace{1}
>> test_run:cmd("switch replica")
>> for i = 1, 10 do box.space.test:replace{2} end
>> vclock = test_run:get_vclock('replica')
>> +vclock[0] = nil
>> _ = test_run:wait_vclock('default', vclock)
>> -- Restart the master and force garbage collection.
>> test_run:cmd("switch default")
>> @@ -126,6 +128,7 @@ test_run:wait_cond(function() return #fio.glob(fio.pathjoin(box.cfg.wal_dir, '*.
>> test_run:cmd("switch replica")
>> for i = 1, 10 do box.space.test:replace{2} end
>> vclock = test_run:get_vclock('replica')
>> +vclock[0] = nil
>> _ = test_run:wait_vclock('default', vclock)
>> -- Restart the replica. It should successfully rebootstrap.
>> test_run:cmd("restart server replica with args='true'")
>> diff --git a/test/replication/skip_conflict_row.result b/test/replication/skip_conflict_row.result
>> index 9b2777872..d70ac8e2a 100644
>> --- a/test/replication/skip_conflict_row.result
>> +++ b/test/replication/skip_conflict_row.result
>> @@ -54,6 +54,9 @@ box.info.status
>> vclock = test_run:get_vclock('default')
>> ---
>> ...
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock("replica", vclock)
>> ---
>> ...
>> diff --git a/test/replication/skip_conflict_row.test.lua b/test/replication/skip_conflict_row.test.lua
>> index 2982c730a..04fd08136 100644
>> --- a/test/replication/skip_conflict_row.test.lua
>> +++ b/test/replication/skip_conflict_row.test.lua
>> @@ -19,6 +19,7 @@ space:insert{2}
>> box.info.status
>> 
>> vclock = test_run:get_vclock('default')
>> +vclock[0] = nil
>> _ = test_run:wait_vclock("replica", vclock)
>> test_run:cmd("switch replica")
>> box.info.replication[1].upstream.message
>> diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
>> index 90fd53ca6..0907ac17f 100644
>> --- a/test/replication/suite.cfg
>> +++ b/test/replication/suite.cfg
>> @@ -12,6 +12,7 @@
>>     "on_schema_init.test.lua": {},
>>     "long_row_timeout.test.lua": {},
>>     "join_without_snap.test.lua": {},
>> +    "gh-4114-local-space-replication.test.lua": {},
>>     "gh-4402-info-errno.test.lua": {},
>>     "gh-4605-empty-password.test.lua": {},
>>     "gh-4606-admin-creds.test.lua": {},
>> diff --git a/test/vinyl/errinj.result b/test/vinyl/errinj.result
>> index 2635da265..2bd701f70 100644
>> --- a/test/vinyl/errinj.result
>> +++ b/test/vinyl/errinj.result
>> @@ -1241,6 +1241,11 @@ test_run:cmd("switch default")
>> vclock = test_run:get_vclock("default")
>> ---
>> ...
>> +-- Ignore 0-th vclock component. They don't match between
>> +-- replicas.
>> +vclock[0] = nil
>> +---
>> +...
>> _ = test_run:wait_vclock("replica", vclock)
>> ---
>> ...
>> diff --git a/test/vinyl/errinj.test.lua b/test/vinyl/errinj.test.lua
>> index 4230cfae3..750d3bfe8 100644
>> --- a/test/vinyl/errinj.test.lua
>> +++ b/test/vinyl/errinj.test.lua
>> @@ -455,6 +455,10 @@ box.cfg{read_only = true}
>> 
>> test_run:cmd("switch default")
>> vclock = test_run:get_vclock("default")
>> +
>> +-- Ignore 0-th vclock component. They don't match between
>> +-- replicas.
>> +vclock[0] = nil
>> _ = test_run:wait_vclock("replica", vclock)
>> 
>> test_run:cmd("stop server replica")
>> -- 
>> 2.21.1 (Apple Git-122.3)
>> 
> 
> -- 
> Konstantin Osipov, Moscow, Russia
> https://scylladb.com


--
Serge Petrenko
sergepetrenko@tarantool.org

Re: [Tarantool-patches] [PATCH v3 4/4] box: start counting local space requests separately

[Konstantin Osipov]

* Serge Petrenko <sergepetrenko@tarantool.org> [20/03/27 18:08]:

Hi, 

somehow I got your patches first, then this response.
I will re-check the explanation in the commit message about how
using server id 0 fixes 4114.


-- 
Konstantin Osipov, Moscow, Russia