From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces References: <20190619124823.7021-1-szudin@tarantool.org> <20190620112550.v2mwqa4orlr4y3rj@esperanza> From: Stanislav Zudin Message-ID: <067138d7-a71d-dc15-1ebc-0d2d9f3bab8a@tarantool.org> Date: Thu, 20 Jun 2019 16:33:26 +0300 MIME-Version: 1.0 In-Reply-To: <20190620112550.v2mwqa4orlr4y3rj@esperanza> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit To: Vladimir Davydov Cc: tarantool-patches@freelists.org, georgy@tarantool.org List-ID: Please find the notes and updated patch below. On 20.06.2019 14:25, Vladimir Davydov wrote: > On Wed, Jun 19, 2019 at 03:48:23PM +0300, Stanislav Zudin wrote: >> Do not spread the space:truncate() to replicas if the >> affected space is local and temporary. >> >> Closes #4263 >> --- >> Branch: https://github.com/tarantool/tarantool/tree/stanztt/gh-4263-no-replica-tmplocal-space >> Issue: https://github.com/tarantool/tarantool/issues/4263 >> >> src/box/alter.cc | 10 ++++++ >> test/replication/local_spaces.result | 48 ++++++++++++++++++++++++++ >> test/replication/local_spaces.test.lua | 19 ++++++++++ >> 3 files changed, 77 insertions(+) >> >> diff --git a/src/box/alter.cc b/src/box/alter.cc >> index a37a68ce4..55d588511 100644 >> --- a/src/box/alter.cc >> +++ b/src/box/alter.cc >> @@ -2278,6 +2278,16 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) >> auto scoped_guard = >> make_scoped_guard([=] { alter_space_delete(alter); }); >> >> + /* >> + * Modify the WAL header to prohibit >> + * replication of local & temporary >> + * spaces truncation. >> + */ >> + if (space_is_temporary(old_space) && > > Must be ||. Local or temporary. Fixed. > >> + space_group_id(old_space) == GROUP_LOCAL) { >> + stmt->row->group_id = GROUP_LOCAL; >> + } >> + >> /* >> * Recreate all indexes of the truncated space. >> */ >> diff --git a/test/replication/local_spaces.result b/test/replication/local_spaces.result >> index ed1b76da8..c36e21403 100644 >> --- a/test/replication/local_spaces.result >> +++ b/test/replication/local_spaces.result >> @@ -71,6 +71,22 @@ s3.temporary >> --- >> - true >> ... >> +-- The truncation of the local & temporary space should not >> +-- spread among the replicas > > It would be nice to see the issue no here. Done. > >> +s4 = box.schema.space.create('test4', {is_local = true, temporary = true}) >> +--- >> +... >> +_ = s4:create_index('pk') >> +--- >> +... >> +s4.is_local >> +--- >> +- true >> +... >> +s4.temporary >> +--- >> +- true >> +... >> _ = s1:insert{1} >> --- >> ... >> @@ -80,6 +96,9 @@ _ = s2:insert{1} >> _ = s3:insert{1} >> --- >> ... >> +_ = s4:insert{1} >> +--- >> +... >> box.snapshot() >> --- >> - ok >> @@ -93,6 +112,16 @@ _ = s2:insert{2} >> _ = s3:insert{2} >> --- >> ... >> +_ = s4:insert{2} >> +--- >> +... >> +s4:truncate() >> +--- >> +... >> +box.space._truncate:select{} >> +--- >> +- - [515, 1] >> +... >> box.schema.user.grant('guest', 'replication') >> --- >> ... >> @@ -124,6 +153,14 @@ box.space.test3.temporary >> --- >> - true >> ... >> +box.space.test4.is_local >> +--- >> +- true >> +... >> +box.space.test4.temporary >> +--- >> +- true >> +... >> box.space.test1:select() >> --- >> - - [1] >> @@ -137,6 +174,14 @@ box.space.test3:select() >> --- >> - [] >> ... >> +box.space.test4:select() >> +--- >> +- [] >> +... > > I don't see where you check that 'truncate' command isn't replicated, > i.e. changes done to a space on the replica are not truncated. > Done, see the patch below. From b596d7d9cda4649a3c0ebb27d4887a57422ac458 Mon Sep 17 00:00:00 2001 From: Stanislav Zudin Date: Wed, 19 Jun 2019 12:28:02 +0300 Subject: [PATCH] ddl: No replication for temp and local spaces Do not spread the space:truncate() to replicas if the affected space is local or temporary. Closes #4263 --- Branch: https://github.com/tarantool/tarantool/tree/stanztt/gh-4263-no-replica-tmplocal-space Issue: https://github.com/tarantool/tarantool/issues/4263 src/box/alter.cc | 10 ++++ test/replication/local_spaces.result | 78 ++++++++++++++++++++++++++ test/replication/local_spaces.test.lua | 36 ++++++++++++ 3 files changed, 124 insertions(+) diff --git a/src/box/alter.cc b/src/box/alter.cc index a37a68ce4..e8a49cf98 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -2278,6 +2278,16 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) auto scoped_guard = make_scoped_guard([=] { alter_space_delete(alter); }); + /* + * Modify the WAL header to prohibit + * replication of local & temporary + * spaces truncation. + */ + if (space_is_temporary(old_space) || + space_group_id(old_space) == GROUP_LOCAL) { + stmt->row->group_id = GROUP_LOCAL; + } + /* * Recreate all indexes of the truncated space. */ diff --git a/test/replication/local_spaces.result b/test/replication/local_spaces.result index ed1b76da8..97e9a9814 100644 --- a/test/replication/local_spaces.result +++ b/test/replication/local_spaces.result @@ -71,6 +71,22 @@ s3.temporary --- - true ... +-- gh-4263 The truncation of the local & temporary space +-- should not spread among the replicas +s4 = box.schema.space.create('test4', {is_local = true, temporary = true}) +--- +... +_ = s4:create_index('pk') +--- +... +s4.is_local +--- +- true +... +s4.temporary +--- +- true +... _ = s1:insert{1} --- ... @@ -80,6 +96,9 @@ _ = s2:insert{1} _ = s3:insert{1} --- ... +_ = s4:insert{1} +--- +... box.snapshot() --- - ok @@ -93,6 +112,9 @@ _ = s2:insert{2} _ = s3:insert{2} --- ... +_ = s4:insert{2} +--- +... box.schema.user.grant('guest', 'replication') --- ... @@ -124,6 +146,14 @@ box.space.test3.temporary --- - true ... +box.space.test4.is_local +--- +- true +... +box.space.test4.temporary +--- +- true +... box.space.test1:select() --- - - [1] @@ -137,6 +167,51 @@ box.space.test3:select() --- - [] ... +box.space.test4:select() +--- +- [] +... +-- To check truncation fill replica's copy with 2 entries +_=box.space.test4:insert{4} +--- +... +_=box.space.test4:insert{5} +--- +... +box.space.test4:select() +--- +- - [4] + - [5] +... +-- truncate temp & local space on master +test_run:cmd("switch default") +--- +- true +... +s4:truncate() +--- +... +-- Expect single record +box.space._truncate:count() +--- +- 1 +... +-- check truncation results on replica +test_run:cmd("switch replica") +--- +- true +... +-- Expect no records on replica +box.space._truncate:count() +--- +- 0 +... +-- the affected space must be unchanged +box.space.test4:select() +--- +- - [4] + - [5] +... box.cfg{read_only = true} -- local spaces ignore read_only --- ... @@ -253,3 +328,6 @@ s2:drop() s3:drop() --- ... +s4:drop() +--- +... diff --git a/test/replication/local_spaces.test.lua b/test/replication/local_spaces.test.lua index bb7294538..cfd8306a0 100644 --- a/test/replication/local_spaces.test.lua +++ b/test/replication/local_spaces.test.lua @@ -33,13 +33,23 @@ _ = s3:create_index('pk') s3.is_local s3.temporary +-- gh-4263 The truncation of the local & temporary space +-- should not spread among the replicas +s4 = box.schema.space.create('test4', {is_local = true, temporary = true}) +_ = s4:create_index('pk') +s4.is_local +s4.temporary + _ = s1:insert{1} _ = s2:insert{1} _ = s3:insert{1} +_ = s4:insert{1} box.snapshot() _ = s1:insert{2} _ = s2:insert{2} _ = s3:insert{2} +_ = s4:insert{2} + box.schema.user.grant('guest', 'replication') test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'") @@ -50,9 +60,34 @@ box.space.test1.is_local box.space.test2.is_local box.space.test3.is_local box.space.test3.temporary +box.space.test4.is_local +box.space.test4.temporary box.space.test1:select() box.space.test2:select() box.space.test3:select() +box.space.test4:select() + +-- To check truncation fill replica's copy with 2 entries +_=box.space.test4:insert{4} +_=box.space.test4:insert{5} +box.space.test4:select() + +-- truncate temp & local space on master +test_run:cmd("switch default") +s4:truncate() +-- Expect single record +box.space._truncate:count() + +-- check truncation results on replica +test_run:cmd("switch replica") + +-- Expect no records on replica +box.space._truncate:count() + +-- the affected space must be unchanged +box.space.test4:select() + + box.cfg{read_only = true} -- local spaces ignore read_only for i = 1, 3 do box.space.test2:insert{i, i} end for i = 1, 3 do box.space.test3:insert{i, i, i} end @@ -87,3 +122,4 @@ s3:select() s1:drop() s2:drop() s3:drop() +s4:drop() -- 2.17.1