* [Tarantool-patches] [PATCH 1/3] vclock: add an ability to set individual clock components
2020-01-31 14:58 [Tarantool-patches] [PATCH 0/3] replication: count local space requests separately Serge Petrenko
@ 2020-01-31 14:58 ` Serge Petrenko
2020-01-31 14:58 ` [Tarantool-patches] [PATCH 2/3] replication: hide 0-th vclock components in replication responses Serge Petrenko
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Serge Petrenko @ 2020-01-31 14:58 UTC (permalink / raw)
To: sergos, v.shpilevoy; +Cc: tarantool-patches
This is needed to 'hide' 0-th vclock component in replication responses.
Follow-up #3186
---
src/box/vclock.c | 12 ++++++++++++
src/box/vclock.h | 11 +++++++++++
2 files changed, 23 insertions(+)
diff --git a/src/box/vclock.c b/src/box/vclock.c
index 90ae27591..90802308e 100644
--- a/src/box/vclock.c
+++ b/src/box/vclock.c
@@ -37,6 +37,17 @@
#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);
+ 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)
{
@@ -51,6 +62,7 @@ vclock_follow(struct vclock *vclock, uint32_t replica_id, int64_t lsn)
return prev_lsn;
}
+
static int
vclock_snprint(char *buf, int size, const struct vclock *vclock)
{
diff --git a/src/box/vclock.h b/src/box/vclock.h
index eb0fb5d8b..e54711bc1 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -209,6 +209,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.0 (Apple Git-122)
^ permalink raw reply [flat|nested] 5+ messages in thread* [Tarantool-patches] [PATCH 2/3] replication: hide 0-th vclock components in replication responses
2020-01-31 14:58 [Tarantool-patches] [PATCH 0/3] replication: count local space requests separately Serge Petrenko
2020-01-31 14:58 ` [Tarantool-patches] [PATCH 1/3] vclock: add an ability to set individual clock components Serge Petrenko
@ 2020-01-31 14:58 ` Serge Petrenko
2020-01-31 14:58 ` [Tarantool-patches] [PATCH 3/3] box: start counting local space requests separately Serge Petrenko
2020-02-03 13:27 ` [Tarantool-patches] [PATCH 0/3] replication: count " Serge Petrenko
3 siblings, 0 replies; 5+ messages in thread
From: Serge Petrenko @ 2020-01-31 14:58 UTC (permalink / raw)
To: sergos, v.shpilevoy; +Cc: tarantool-patches
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.
Follow-up #3186
---
src/box/box.cc | 11 +++++++++--
src/box/relay.cc | 6 +++++-
test/replication/anon.result | 5 +++++
test/replication/anon.test.lua | 2 ++
4 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/box/box.cc b/src/box/box.cc
index 1b2b27d61..57a43c754 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1518,6 +1518,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;
@@ -1599,7 +1600,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);
@@ -1751,7 +1754,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);
diff --git a/src/box/relay.cc b/src/box/relay.cc
index b89632273..e6840541f 100644
--- a/src/box/relay.cc
+++ b/src/box/relay.cc
@@ -315,7 +315,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.0 (Apple Git-122)
^ permalink raw reply [flat|nested] 5+ messages in thread* [Tarantool-patches] [PATCH 3/3] box: start counting local space requests separately
2020-01-31 14:58 [Tarantool-patches] [PATCH 0/3] replication: count local space requests separately Serge Petrenko
2020-01-31 14:58 ` [Tarantool-patches] [PATCH 1/3] vclock: add an ability to set individual clock components Serge Petrenko
2020-01-31 14:58 ` [Tarantool-patches] [PATCH 2/3] replication: hide 0-th vclock components in replication responses Serge Petrenko
@ 2020-01-31 14:58 ` Serge Petrenko
2020-02-03 13:27 ` [Tarantool-patches] [PATCH 0/3] replication: count " Serge Petrenko
3 siblings, 0 replies; 5+ messages in thread
From: Serge Petrenko @ 2020-01-31 14:58 UTC (permalink / raw)
To: sergos, v.shpilevoy; +Cc: tarantool-patches
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 | 19 +++
src/box/relay.cc | 17 +--
src/box/wal.c | 16 ++-
.../gh-4114-local-space-replication.result | 121 ++++++++++++++++++
.../gh-4114-local-space-replication.test.lua | 44 +++++++
test/replication/local_spaces.result | 4 +
test/replication/local_spaces.test.lua | 3 +
test/vinyl/errinj.result | 5 +
test/vinyl/errinj.test.lua | 4 +
test/xlog/panic_on_wal_error.result | 2 +-
10 files changed, 216 insertions(+), 19 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 57a43c754..add6e3743 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1859,6 +1859,25 @@ 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 consumer 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.
+ * Speaking of gc, we have to be more conservative here,
+ * and set 0-th component to the minimal possible value,
+ * zeroing it out. Otherwise we can delete wals still
+ * needed by replica. This is done in relay_subscribe.
+ */
+ 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 e6840541f..7c423e925 100644
--- a/src/box/relay.cc
+++ b/src/box/relay.cc
@@ -691,6 +691,9 @@ relay_subscribe(struct replica *replica, int fd, uint64_t sync,
* join.
*/
if (replica->gc == NULL && !replica->anon) {
+ struct vclock gc_clock;
+ vclock_copy(&gc_clock, replica_clock);
+ vclock_set(&gc_clock, 0, 0);
replica->gc = gc_consumer_register(replica_clock, "replica %s",
tt_uuid_str(&replica->uuid));
if (replica->gc == NULL)
@@ -750,22 +753,10 @@ 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.
- */
+ /* We do not relay replica-local rows to other instances. */
if (packet->group_id == GROUP_LOCAL) {
- /*
- * Replica-local requests generated while replica
- * was anonymous have a zero instance id. Just
- * skip all these rows.
- */
if (packet->replica_id == REPLICA_ID_NIL)
return;
- packet->type = IPROTO_NOP;
- packet->group_id = GROUP_DEFAULT;
- packet->bodycnt = 0;
}
/*
* We're feeding a WAL, thus responding to FINAL JOIN or SUBSCRIBE
diff --git a/src/box/wal.c b/src/box/wal.c
index 0ae66ff32..b453022b4 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -939,13 +939,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/gh-4114-local-space-replication.result b/test/replication/gh-4114-local-space-replication.result
new file mode 100644
index 000000000..ed69c2b6f
--- /dev/null
+++ b/test/replication/gh-4114-local-space-replication.result
@@ -0,0 +1,121 @@
+-- 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}
+ | ---
+ | ...
+box.info.replication[2].upstream.status
+ | ---
+ | - follow
+ | ...
+
+-- 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..415ba1ddc
--- /dev/null
+++ b/test/replication/gh-4114-local-space-replication.test.lua
@@ -0,0 +1,44 @@
+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}
+box.info.replication[2].upstream.status
+
+-- 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/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")
diff --git a/test/xlog/panic_on_wal_error.result b/test/xlog/panic_on_wal_error.result
index 22f14f912..aa2ba417b 100644
--- a/test/xlog/panic_on_wal_error.result
+++ b/test/xlog/panic_on_wal_error.result
@@ -141,7 +141,7 @@ box.info.replication[1].upstream.status
...
box.info.replication[1].upstream.message
---
-- 'Missing .xlog file between LSN 6 {1: 6} and 8 {1: 8}'
+- 'Missing .xlog file between LSN 6 {0: 0, 1: 6} and 8 {1: 8}'
...
box.space.test:select{}
---
--
2.21.0 (Apple Git-122)
^ permalink raw reply [flat|nested] 5+ messages in thread