

<HTML><BODY><div>Hi all, </div><div> </div><div>QA LGTM</div><div> </div><div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Vitaliia Ioffe</div></div></div><div> </div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Понедельник, 9 августа 2021, 16:11 +03:00 от Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16285146790158964331_BODY"><br><br>09.08.2021 13:09, Yan Shtunder пишет:<br>> replicaset.applier.vclock is initialized in replication_init(),<br>> which happens before local recovery. If some changes are come<br>> frome some instance via applier that applier.vclock will equal 0.<br>> This means that if some wild master send this node already applied<br>> data, the node will apply the same data twice.<br>><br>> Closes #6028<br>> ---<br><br>Hi! Thanks for the fixes!<br>LGTM.<br><br>> Issue: <a href="https://github.com/tarantool/tarantool/issues/6028" target="_blank">https://github.com/tarantool/tarantool/issues/6028</a><br>> Patch: <a href="https://github.com/tarantool/tarantool/tree/yshtunder/gh-6028-applier-vclock" target="_blank">https://github.com/tarantool/tarantool/tree/yshtunder/gh-6028-applier-vclock</a><br>><br>> src/box/applier.cc | 5 ++<br>> src/box/box.cc | 7 +++<br>> src/lib/core/errinj.h | 1 +<br>> test/box/errinj.result | 1 +<br>> test/replication/gh-6028-replica.lua | 13 ++++<br>> .../gh-6028-vclock-is-empty.result | 60 +++++++++++++++++++<br>> .../gh-6028-vclock-is-empty.test.lua | 24 ++++++++<br>> 7 files changed, 111 insertions(+)<br>> create mode 100644 test/replication/gh-6028-replica.lua<br>> create mode 100644 test/replication/gh-6028-vclock-is-empty.result<br>> create mode 100644 test/replication/gh-6028-vclock-is-empty.test.lua<br>><br>> diff --git a/src/box/applier.cc b/src/box/applier.cc<br>> index 07fe7f5c7..9855b8d37 100644<br>> --- a/src/box/applier.cc<br>> +++ b/src/box/applier.cc<br>> @@ -1230,6 +1230,11 @@ applier_subscribe(struct applier *applier)<br>> struct vclock vclock;<br>> vclock_create(&vclock);<br>> vclock_copy(&vclock, &replicaset.vclock);<br>> +<br>> + ERROR_INJECT(ERRINJ_REPLICASET_VCLOCK, {<br>> + vclock_create(&vclock);<br>> + });<br>> +<br>> /*<br>> * Stop accepting local rows coming from a remote<br>> * instance as soon as local WAL starts accepting writes.<br>> diff --git a/src/box/box.cc b/src/box/box.cc<br>> index ab7d983c9..f5cd63c9e 100644<br>> --- a/src/box/box.cc<br>> +++ b/src/box/box.cc<br>> @@ -3451,6 +3451,13 @@ box_cfg_xc(void)<br>> bootstrap(&instance_uuid, &replicaset_uuid,<br>> &is_bootstrap_leader);<br>> }<br>> +<br>> + /*<br>> + * replicaset.applier.vclock is filled with real<br>> + * value where local restore has already completed<br>> + */<br>> + vclock_copy(&replicaset.applier.vclock, &replicaset.vclock);<br>> +<br>> fiber_gc();<br>><br>> /*<br>> diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h<br>> index 359174b16..fcd856fbb 100644<br>> --- a/src/lib/core/errinj.h<br>> +++ b/src/lib/core/errinj.h<br>> @@ -152,6 +152,7 @@ struct errinj {<br>> _(ERRINJ_STDIN_ISATTY, ERRINJ_INT, {.iparam = -1}) \<br>> _(ERRINJ_SNAP_COMMIT_FAIL, ERRINJ_BOOL, {.bparam = false}) \<br>> _(ERRINJ_IPROTO_SINGLE_THREAD_STAT, ERRINJ_INT, {.iparam = -1}) \<br>> + _(ERRINJ_REPLICASET_VCLOCK, ERRINJ_BOOL, {.bparam = false}) \<br>><br>> ENUM0(errinj_id, ERRINJ_LIST);<br>> extern struct errinj errinjs[];<br>> diff --git a/test/box/errinj.result b/test/box/errinj.result<br>> index 43daf5f0f..62e37bbdd 100644<br>> --- a/test/box/errinj.result<br>> +++ b/test/box/errinj.result<br>> @@ -70,6 +70,7 @@ evals<br>> - ERRINJ_RELAY_REPORT_INTERVAL: 0<br>> - ERRINJ_RELAY_SEND_DELAY: false<br>> - ERRINJ_RELAY_TIMEOUT: 0<br>> + - ERRINJ_REPLICASET_VCLOCK: false<br>> - ERRINJ_REPLICA_JOIN_DELAY: false<br>> - ERRINJ_SIO_READ_MAX: -1<br>> - ERRINJ_SNAP_COMMIT_DELAY: false<br>> diff --git a/test/replication/gh-6028-replica.lua b/test/replication/gh-6028-replica.lua<br>> new file mode 100644<br>> index 000000000..5669cc4e9<br>> --- /dev/null<br>> +++ b/test/replication/gh-6028-replica.lua<br>> @@ -0,0 +1,13 @@<br>> +#!/usr/bin/env tarantool<br>> +<br>> +require('console').listen(os.getenv('ADMIN'))<br>> +<br>> +box.error.injection.set("ERRINJ_REPLICASET_VCLOCK", true)<br>> +<br>> +box.cfg({<br>> + listen = os.getenv("LISTEN"),<br>> + replication = {os.getenv("MASTER"), os.getenv("LISTEN")},<br>> + memtx_memory = 107374182,<br>> +})<br>> +<br>> +box.error.injection.set("ERRINJ_REPLICASET_VCLOCK", false)<br>> diff --git a/test/replication/gh-6028-vclock-is-empty.result b/test/replication/gh-6028-vclock-is-empty.result<br>> new file mode 100644<br>> index 000000000..0b103bb6e<br>> --- /dev/null<br>> +++ b/test/replication/gh-6028-vclock-is-empty.result<br>> @@ -0,0 +1,60 @@<br>> +-- test-run result file version 2<br>> +test_run = require('test_run').new()<br>> + | ---<br>> + | ...<br>> +<br>> +box.schema.user.grant('guest', 'replication')<br>> + | ---<br>> + | ...<br>> +s = box.schema.create_space('test')<br>> + | ---<br>> + | ...<br>> +_ = s:create_index('pk')<br>> + | ---<br>> + | ...<br>> +<br>> +<br>> +-- Case 1<br>> +test_run:cmd('create server replica with rpl_master=default,\<br>> + script="replication/gh-6028-replica.lua"')<br>> + | ---<br>> + | - true<br>> + | ...<br>> +test_run:cmd('start server replica')<br>> + | ---<br>> + | - true<br>> + | ...<br>> +<br>> +test_run:cmd('stop server replica')<br>> + | ---<br>> + | - true<br>> + | ...<br>> +s:insert{1}<br>> + | ---<br>> + | - [1]<br>> + | ...<br>> +<br>> +<br>> +-- Case 2<br>> +test_run:cmd('start server replica')<br>> + | ---<br>> + | - true<br>> + | ...<br>> +s:insert{2}<br>> + | ---<br>> + | - [2]<br>> + | ...<br>> +<br>> +<br>> +test_run:cmd('stop server replica')<br>> + | ---<br>> + | - true<br>> + | ...<br>> +test_run:cmd('cleanup server replica')<br>> + | ---<br>> + | - true<br>> + | ...<br>> +test_run:cmd('delete server replica')<br>> + | ---<br>> + | - true<br>> + | ...<br>> diff --git a/test/replication/gh-6028-vclock-is-empty.test.lua b/test/replication/gh-6028-vclock-is-empty.test.lua<br>> new file mode 100644<br>> index 000000000..ba14eca55<br>> --- /dev/null<br>> +++ b/test/replication/gh-6028-vclock-is-empty.test.lua<br>> @@ -0,0 +1,24 @@<br>> +test_run = require('test_run').new()<br>> +<br>> +box.schema.user.grant('guest', 'replication')<br>> +s = box.schema.create_space('test')<br>> +_ = s:create_index('pk')<br>> +<br>> +<br>> +-- Case 1<br>> +test_run:cmd('create server replica with rpl_master=default,\<br>> + script="replication/gh-6028-replica.lua"')<br>> +test_run:cmd('start server replica')<br>> +<br>> +test_run:cmd('stop server replica')<br>> +s:insert{1}<br>> +<br>> +<br>> +-- Case 2<br>> +test_run:cmd('start server replica')<br>> +s:insert{2}<br>> +<br>> +<br>> +test_run:cmd('stop server replica')<br>> +test_run:cmd('cleanup server replica')<br>> +test_run:cmd('delete server replica')<br>> --<br>> 2.25.1<br>><br><br>--<br>Serge Petrenko</div></div></div></div></blockquote><div> </div></BODY></HTML>