Hi all, 
 
QA LGTM
 
 
--
Vitaliia Ioffe
 
 
Понедельник, 9 августа 2021, 16:11 +03:00 от Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
 


09.08.2021 13:09, Yan Shtunder пишет:
> replicaset.applier.vclock is initialized in replication_init(),
> which happens before local recovery. If some changes are come
> frome some instance via applier that applier.vclock will equal 0.
> This means that if some wild master send this node already applied
> data, the node will apply the same data twice.
>
> Closes #6028
> ---

Hi! Thanks for the fixes!
LGTM.

> Issue: https://github.com/tarantool/tarantool/issues/6028
> Patch: https://github.com/tarantool/tarantool/tree/yshtunder/gh-6028-applier-vclock
>
> src/box/applier.cc | 5 ++
> src/box/box.cc | 7 +++
> src/lib/core/errinj.h | 1 +
> test/box/errinj.result | 1 +
> test/replication/gh-6028-replica.lua | 13 ++++
> .../gh-6028-vclock-is-empty.result | 60 +++++++++++++++++++
> .../gh-6028-vclock-is-empty.test.lua | 24 ++++++++
> 7 files changed, 111 insertions(+)
> create mode 100644 test/replication/gh-6028-replica.lua
> create mode 100644 test/replication/gh-6028-vclock-is-empty.result
> create mode 100644 test/replication/gh-6028-vclock-is-empty.test.lua
>
> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index 07fe7f5c7..9855b8d37 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -1230,6 +1230,11 @@ applier_subscribe(struct applier *applier)
> struct vclock vclock;
> vclock_create(&vclock);
> vclock_copy(&vclock, &replicaset.vclock);
> +
> + ERROR_INJECT(ERRINJ_REPLICASET_VCLOCK, {
> + vclock_create(&vclock);
> + });
> +
> /*
> * Stop accepting local rows coming from a remote
> * instance as soon as local WAL starts accepting writes.
> diff --git a/src/box/box.cc b/src/box/box.cc
> index ab7d983c9..f5cd63c9e 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -3451,6 +3451,13 @@ box_cfg_xc(void)
> bootstrap(&instance_uuid, &replicaset_uuid,
> &is_bootstrap_leader);
> }
> +
> + /*
> + * replicaset.applier.vclock is filled with real
> + * value where local restore has already completed
> + */
> + vclock_copy(&replicaset.applier.vclock, &replicaset.vclock);
> +
> fiber_gc();
>
> /*
> diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h
> index 359174b16..fcd856fbb 100644
> --- a/src/lib/core/errinj.h
> +++ b/src/lib/core/errinj.h
> @@ -152,6 +152,7 @@ struct errinj {
> _(ERRINJ_STDIN_ISATTY, ERRINJ_INT, {.iparam = -1}) \
> _(ERRINJ_SNAP_COMMIT_FAIL, ERRINJ_BOOL, {.bparam = false}) \
> _(ERRINJ_IPROTO_SINGLE_THREAD_STAT, ERRINJ_INT, {.iparam = -1}) \
> + _(ERRINJ_REPLICASET_VCLOCK, ERRINJ_BOOL, {.bparam = false}) \
>
> ENUM0(errinj_id, ERRINJ_LIST);
> extern struct errinj errinjs[];
> diff --git a/test/box/errinj.result b/test/box/errinj.result
> index 43daf5f0f..62e37bbdd 100644
> --- a/test/box/errinj.result
> +++ b/test/box/errinj.result
> @@ -70,6 +70,7 @@ evals
> - ERRINJ_RELAY_REPORT_INTERVAL: 0
> - ERRINJ_RELAY_SEND_DELAY: false
> - ERRINJ_RELAY_TIMEOUT: 0
> + - ERRINJ_REPLICASET_VCLOCK: false
> - ERRINJ_REPLICA_JOIN_DELAY: false
> - ERRINJ_SIO_READ_MAX: -1
> - ERRINJ_SNAP_COMMIT_DELAY: false
> diff --git a/test/replication/gh-6028-replica.lua b/test/replication/gh-6028-replica.lua
> new file mode 100644
> index 000000000..5669cc4e9
> --- /dev/null
> +++ b/test/replication/gh-6028-replica.lua
> @@ -0,0 +1,13 @@
> +#!/usr/bin/env tarantool
> +
> +require('console').listen(os.getenv('ADMIN'))
> +
> +box.error.injection.set("ERRINJ_REPLICASET_VCLOCK", true)
> +
> +box.cfg({
> + listen = os.getenv("LISTEN"),
> + replication = {os.getenv("MASTER"), os.getenv("LISTEN")},
> + memtx_memory = 107374182,
> +})
> +
> +box.error.injection.set("ERRINJ_REPLICASET_VCLOCK", false)
> diff --git a/test/replication/gh-6028-vclock-is-empty.result b/test/replication/gh-6028-vclock-is-empty.result
> new file mode 100644
> index 000000000..0b103bb6e
> --- /dev/null
> +++ b/test/replication/gh-6028-vclock-is-empty.result
> @@ -0,0 +1,60 @@
> +-- test-run result file version 2
> +test_run = require('test_run').new()
> + | ---
> + | ...
> +
> +box.schema.user.grant('guest', 'replication')
> + | ---
> + | ...
> +s = box.schema.create_space('test')
> + | ---
> + | ...
> +_ = s:create_index('pk')
> + | ---
> + | ...
> +
> +
> +-- Case 1
> +test_run:cmd('create server replica with rpl_master=default,\
> + script="replication/gh-6028-replica.lua"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica')
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('stop server replica')
> + | ---
> + | - true
> + | ...
> +s:insert{1}
> + | ---
> + | - [1]
> + | ...
> +
> +
> +-- Case 2
> +test_run:cmd('start server replica')
> + | ---
> + | - true
> + | ...
> +s:insert{2}
> + | ---
> + | - [2]
> + | ...
> +
> +
> +test_run:cmd('stop server replica')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('cleanup server replica')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server replica')
> + | ---
> + | - true
> + | ...
> diff --git a/test/replication/gh-6028-vclock-is-empty.test.lua b/test/replication/gh-6028-vclock-is-empty.test.lua
> new file mode 100644
> index 000000000..ba14eca55
> --- /dev/null
> +++ b/test/replication/gh-6028-vclock-is-empty.test.lua
> @@ -0,0 +1,24 @@
> +test_run = require('test_run').new()
> +
> +box.schema.user.grant('guest', 'replication')
> +s = box.schema.create_space('test')
> +_ = s:create_index('pk')
> +
> +
> +-- Case 1
> +test_run:cmd('create server replica with rpl_master=default,\
> + script="replication/gh-6028-replica.lua"')
> +test_run:cmd('start server replica')
> +
> +test_run:cmd('stop server replica')
> +s:insert{1}
> +
> +
> +-- Case 2
> +test_run:cmd('start server replica')
> +s:insert{2}
> +
> +
> +test_run:cmd('stop server replica')
> +test_run:cmd('cleanup server replica')
> +test_run:cmd('delete server replica')
> --
> 2.25.1
>

--
Serge Petrenko