From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 0239F6F3C8; Sat, 27 Mar 2021 23:13:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0239F6F3C8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1616876024; bh=jAMnNdSLvP5FrIj7xoP8j79CH3qb9Fl/a7ABcydRVhI=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=MpVHDmNWdw3zIB6qJLrfFW/dljp1AGWUExXaLa8LQeJ+EDLvzSutllOwI4wzto1Bx 7McLTvNydggUK8vwsmvsMhbnZXWC1q49KpTpKSSEzB7XIt/SPYnoJrUs5pwWPXZXWp Y5bzs3c+QerqUk2FgoWEilGssNKM52fWvTzTc07E= Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E7B596F3C8 for ; Sat, 27 Mar 2021 23:13:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E7B596F3C8 Received: by smtp36.i.mail.ru with esmtpa (envelope-from ) id 1lQFJV-0006de-Td; Sat, 27 Mar 2021 23:13:42 +0300 To: Vladislav Shpilevoy , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org References: <9bc5d0c0-39b1-eedd-9651-2f9c0b259f5a@tarantool.org> Message-ID: <618c1086-2bcd-ea5e-1d4c-54da8b6fa36b@tarantool.org> Date: Sat, 27 Mar 2021 23:13:41 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.16; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <9bc5d0c0-39b1-eedd-9651-2f9c0b259f5a@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9ED7173E37F4E32947A0146560F8BA709E798FFD99D1B1662182A05F5380850408EFFF2CB82893939F36002975052A5255D71623EA33AE31F1B09A5DC02934623 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE71E18668C3FD76909EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637ED2BA022FBF94AB68638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95C7428A34725AB662DC0555FC43404AFF33DBDB96A484129EEA471835C12D1D9774AD6D5ED66289B5259CC434672EE6371117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658359CC434672EE6371117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CFA04C57760373E89B76E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8B792346D4E681777A3AA81AA40904B5D9DBF02ECDB25306B2201CA6A4E26CD07C3BBE47FD9DD3FB595F5C1EE8F4F765FCA83251EDC214901ED5E8D9A59859A8B645423645B6F85954089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2368A440D3B0F6089093C9A16E5BC824AC8B6CDF511875BC4E8F7B195E1C97831668C8ADB0F3448D7008EF5BE183B7CB3 X-C1DE0DAB: 0D63561A33F958A50436BEEF15C1DF26DF195A34001C5152D5BFBA3218C5E15FD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D1AE09A115117C961484FAF0791ECD191BA6A9B95332DF778ECAF591809ABE8E68A1B2DDAE7DE8491D7E09C32AA3244C73654B080E4CEB4334B60BE2563F2A87D9ADFF0C0BDB8D1FFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojhfg4BOnpz0r09EwcoUepOw== X-Mailru-Sender: 583F1D7ACE8F49BDD2846D59FC20E9F844DA62AFE32DEF91C9B3A0C56F531283DC3D13817423D1F4424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 7/7] replication: do not ignore replica vclock on register X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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