[Tarantool-patches] [PATCH v2 7/7] replication: do not ignore replica vclock on register
Serge Petrenko
sergepetrenko at tarantool.org
Sat Mar 27 23:13:41 MSK 2021
26.03.2021 23:50, Vladislav Shpilevoy пишет:
> Thanks for the patch!
>
> See 2 comments below.
>
> On 24.03.2021 13:24, Serge Petrenko wrote:
>> There was a bug in box_process_register. It decoded replica's vclock but
>> never used it when sending the registration stream. So the replica might
>> lose the data in range (replica_vclock, start_vclock).
> 1. Could you please add a test?
Yes, sure.
>
>> Follow-up #5566
>> ---
>> src/box/box.cc | 14 ++++++--------
>> 1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/box/box.cc b/src/box/box.cc
>> index 292a54213..0bcb505a8 100644
>> --- a/src/box/box.cc
>> +++ b/src/box/box.cc
>> @@ -2154,7 +2154,8 @@ box_process_register(struct ev_io *io, struct xrow_header *header)
>> "wal_mode = 'none'");
>> }
>>
>> - struct gc_consumer *gc = gc_consumer_register(&replicaset.vclock,
>> + vclock_reset(&replica_vclock, 0, vclock_get(&replicaset.vclock, 0));
> 2. xrow_decode_register_xc() already returns the vclock with empty 0
> part. Why do you need this reset?
It sets vclock[0] to master's vclock[0]. But it's not needed here, indeed.
Thanks for noticing!
I thought having vclock[0] = 0 would harm gc somehow, but that's not true.
>
>> + struct gc_consumer *gc = gc_consumer_register(&replica_vclock,
>> "replica %s", tt_uuid_str(&instance_uuid));
>> if (gc == NULL)
>> diag_raise();
=================================
diff --git a/src/box/box.cc b/src/box/box.cc
index 0bcb505a8..ecec8df27 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -2154,7 +2154,6 @@ box_process_register(struct ev_io *io, struct
xrow_header *header)
"wal_mode = 'none'");
}
- vclock_reset(&replica_vclock, 0, vclock_get(&replicaset.vclock, 0));
struct gc_consumer *gc = gc_consumer_register(&replica_vclock,
"replica %s", tt_uuid_str(&instance_uuid));
if (gc == NULL)
diff --git a/test/replication/anon_register_gap.result
b/test/replication/anon_register_gap.result
new file mode 100644
index 000000000..24a3548c8
--- /dev/null
+++ b/test/replication/anon_register_gap.result
@@ -0,0 +1,116 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+--
+-- When master's registering an anonymous replica, it might ignore the
replica's
+-- current vclock, and skip the data in range (replica_clock,
master_clock).
+--
+box.schema.user.grant('guest', 'replication')
+ | ---
+ | ...
+_ = box.schema.space.create('test')
+ | ---
+ | ...
+_ = box.space.test:create_index('pk')
+ | ---
+ | ...
+
+test_run:cmd('create server replica with rpl_master=default,\
+ script="replication/anon1.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica')
+ | ---
+ | - true
+ | ...
+
+test_run:wait_lsn('replica', 'default')
+ | ---
+ | ...
+box.error.injection.set('ERRINJ_RELAY_SEND_DELAY', true)
+ | ---
+ | - ok
+ | ...
+
+box.space.test:insert{1}
+ | ---
+ | - [1]
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+
+test_run:wait_upstream(1, {status='disconnected'})
+ | ---
+ | - true
+ | ...
+box.space.test:select{}
+ | ---
+ | - []
+ | ...
+
+fiber = require('fiber')
+ | ---
+ | ...
+f = fiber.new(function() box.cfg{replication_anon=false} end)
+ | ---
+ | ...
+test_run:wait_upstream(1, {status='register'})
+ | ---
+ | - true
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+box.error.injection.set('ERRINJ_RELAY_SEND_DELAY', false)
+ | ---
+ | - ok
+ | ...
+box.space.test:insert{2}
+ | ---
+ | - [2]
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+test_run:wait_lsn('replica', 'default')
+ | ---
+ | ...
+f:status()
+ | ---
+ | - dead
+ | ...
+box.space.test:select{}
+ | ---
+ | - - [1]
+ | - [2]
+ | ...
+
+-- Cleanup
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+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/anon_register_gap.test.lua
b/test/replication/anon_register_gap.test.lua
new file mode 100644
index 000000000..c71576a23
--- /dev/null
+++ b/test/replication/anon_register_gap.test.lua
@@ -0,0 +1,43 @@
+test_run = require('test_run').new()
+
+--
+-- When master's registering an anonymous replica, it might ignore the
replica's
+-- current vclock, and skip the data in range (replica_clock,
master_clock).
+--
+box.schema.user.grant('guest', 'replication')
+_ = box.schema.space.create('test')
+_ = box.space.test:create_index('pk')
+
+test_run:cmd('create server replica with rpl_master=default,\
+ script="replication/anon1.lua"')
+test_run:cmd('start server replica')
+
+test_run:wait_lsn('replica', 'default')
+box.error.injection.set('ERRINJ_RELAY_SEND_DELAY', true)
+
+box.space.test:insert{1}
+
+test_run:switch('replica')
+
+test_run:wait_upstream(1, {status='disconnected'})
+box.space.test:select{}
+
+fiber = require('fiber')
+f = fiber.new(function() box.cfg{replication_anon=false} end)
+test_run:wait_upstream(1, {status='register'})
+
+test_run:switch('default')
+box.error.injection.set('ERRINJ_RELAY_SEND_DELAY', false)
+box.space.test:insert{2}
+
+test_run:switch('replica')
+test_run:wait_lsn('replica', 'default')
+f:status()
+box.space.test:select{}
+
+-- Cleanup
+test_run:switch('default')
+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/suite.cfg b/test/replication/suite.cfg
index aea8b6157..aff5fda26 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -1,5 +1,6 @@
{
"anon.test.lua": {},
+ "anon_register_gap.test.lua": {},
"gh-2991-misc-asserts-on-update.test.lua": {},
"gh-3111-misc-rebootstrap-from-ro-master.test.lua": {},
"gh-3160-misc-heartbeats-on-master-changes.test.lua": {},
diff --git a/test/replication/suite.ini b/test/replication/suite.ini
index fc161700a..a9e44e8cf 100644
--- a/test/replication/suite.ini
+++ b/test/replication/suite.ini
@@ -3,7 +3,7 @@ core = tarantool
script = master.lua
description = tarantool/box, replication
disabled = consistent.test.lua
-release_disabled = catch.test.lua errinj.test.lua gc.test.lua
gc_no_space.test.lua before_replace.test.lua qsync_advanced.test.lua
qsync_errinj.test.lua quorum.test.lua recover_missing_xlog.test.lua
sync.test.lua long_row_timeout.test.lua gh-4739-vclock-assert.test.lua
gh-4730-applier-rollback.test.lua gh-5140-qsync-casc-rollback.test.lua
gh-5144-qsync-dup-confirm.test.lua gh-5167-qsync-rollback-snap.test.lua
gh-5506-election-on-off.test.lua gh-5536-wal-limit.test.lua
hang_on_synchro_fail.test.lua
+release_disabled = catch.test.lua errinj.test.lua gc.test.lua
gc_no_space.test.lua before_replace.test.lua qsync_advanced.test.lua
qsync_errinj.test.lua quorum.test.lua recover_missing_xlog.test.lua
sync.test.lua long_row_timeout.test.lua gh-4739-vclock-assert.test.lua
gh-4730-applier-rollback.test.lua gh-5140-qsync-casc-rollback.test.lua
gh-5144-qsync-dup-confirm.test.lua gh-5167-qsync-rollback-snap.test.lua
gh-5506-election-on-off.test.lua gh-5536-wal-limit.test.lua
hang_on_synchro_fail.test.lua anon_register_gap.test.lua
config = suite.cfg
lua_libs = lua/fast_replica.lua lua/rlimit.lua
use_unix_sockets = True
--
Serge Petrenko
More information about the Tarantool-patches
mailing list