Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces
@ 2019-06-19 12:48 Stanislav Zudin
  2019-06-20  9:00 ` [tarantool-patches] " Konstantin Osipov
  2019-06-20 11:25 ` [tarantool-patches] " Vladimir Davydov
  0 siblings, 2 replies; 10+ messages in thread
From: Stanislav Zudin @ 2019-06-19 12:48 UTC (permalink / raw)
  To: tarantool-patches, georgy; +Cc: Stanislav Zudin

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) &&
+	    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
+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()
+---
+- []
+...
+box.space._truncate:select{}
+---
+- []
+...
 box.cfg{read_only = true} -- local spaces ignore read_only
 ---
 ...
@@ -253,3 +298,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..01fbeeb70 100644
--- a/test/replication/local_spaces.test.lua
+++ b/test/replication/local_spaces.test.lua
@@ -33,13 +33,25 @@ _ = s3:create_index('pk')
 s3.is_local
 s3.temporary
 
+-- 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}
+
+s4:truncate()
+box.space._truncate:select{}
 
 box.schema.user.grant('guest', 'replication')
 test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
@@ -50,9 +62,15 @@ 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()
+
+box.space._truncate: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 +105,4 @@ s3:select()
 s1:drop()
 s2:drop()
 s3:drop()
+s4:drop()
-- 
2.17.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tarantool-patches] Re: [PATCH] ddl: No replication for temp and local spaces
  2019-06-19 12:48 [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces Stanislav Zudin
@ 2019-06-20  9:00 ` Konstantin Osipov
  2019-06-20  9:02   ` Konstantin Osipov
  2019-06-20 10:34   ` Георгий Кириченко
  2019-06-20 11:25 ` [tarantool-patches] " Vladimir Davydov
  1 sibling, 2 replies; 10+ messages in thread
From: Konstantin Osipov @ 2019-06-20  9:00 UTC (permalink / raw)
  To: tarantool-patches; +Cc: georgy, Stanislav Zudin

* Stanislav Zudin <szudin@tarantool.org> [19/06/19 15:53]:
> Do not spread the space:truncate() to replicas if the
> affected space is local and temporary.

While I agree with Georgy's comment that when it comes to
TEMPORARY spaces, we should replicate all DDL but TRUNCATE, for
node-local spaces I think nothing should be replicated. This is
the idea of node-local spaces to begin with.


-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tarantool-patches] Re: [PATCH] ddl: No replication for temp and local spaces
  2019-06-20  9:00 ` [tarantool-patches] " Konstantin Osipov
@ 2019-06-20  9:02   ` Konstantin Osipov
  2019-06-20 10:35     ` Георгий Кириченко
  2019-06-20 10:34   ` Георгий Кириченко
  1 sibling, 1 reply; 10+ messages in thread
From: Konstantin Osipov @ 2019-06-20  9:02 UTC (permalink / raw)
  To: tarantool-patches; +Cc: georgy, Stanislav Zudin

* Konstantin Osipov <kostja@tarantool.org> [19/06/20 12:00]:
> * Stanislav Zudin <szudin@tarantool.org> [19/06/19 15:53]:
> > Do not spread the space:truncate() to replicas if the
> > affected space is local and temporary.
> 
> While I agree with Georgy's comment that when it comes to
> TEMPORARY spaces, we should replicate all DDL but TRUNCATE, for
> node-local spaces I think nothing should be replicated. This is
> the idea of node-local spaces to begin with.

On the other hand I don't know how to prevent replication of this
data since it's already in the snapshot. Plus, if we filter out
group-local spaces from the snapshot, we're going to break the
vinyl deferred delete space.

But we definitely need spaces which are node-local and are not
replicated anywhere, including DDL.

-- 
Konstantin Osipov, Moscow, Russia

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tarantool-patches] Re: [PATCH] ddl: No replication for temp and local spaces
  2019-06-20  9:00 ` [tarantool-patches] " Konstantin Osipov
  2019-06-20  9:02   ` Konstantin Osipov
@ 2019-06-20 10:34   ` Георгий Кириченко
  1 sibling, 0 replies; 10+ messages in thread
From: Георгий Кириченко @ 2019-06-20 10:34 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Konstantin Osipov, Stanislav Zudin

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

On Thursday, June 20, 2019 12:00:38 PM MSK Konstantin Osipov wrote:
> * Stanislav Zudin <szudin@tarantool.org> [19/06/19 15:53]:
> > Do not spread the space:truncate() to replicas if the
> > affected space is local and temporary.
> 
> While I agree with Georgy's comment that when it comes to
> TEMPORARY spaces, we should replicate all DDL but TRUNCATE, for
> node-local spaces I think nothing should be replicated. This is
> the idea of node-local spaces to begin with.

It breaks backward compatibility and inconsistent all. I'm pretty sure we 
definitely should replicate all local space schema changes because it is only 
possible way to keep a fallback possible. In the opposite case we were in duty 
to keep all local spaces definition in mind and apply them before replica 
became master. The other solution is to populate local space changes across 
whole cluster by hands instead of replication and it looks as ugly as previous 
one.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [tarantool-patches] Re: [PATCH] ddl: No replication for temp and local spaces
  2019-06-20  9:02   ` Konstantin Osipov
@ 2019-06-20 10:35     ` Георгий Кириченко
  0 siblings, 0 replies; 10+ messages in thread
From: Георгий Кириченко @ 2019-06-20 10:35 UTC (permalink / raw)
  To: Konstantin Osipov; +Cc: tarantool-patches, Stanislav Zudin

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]

On Thursday, June 20, 2019 12:02:45 PM MSK Konstantin Osipov wrote:
> * Konstantin Osipov <kostja@tarantool.org> [19/06/20 12:00]:
> > * Stanislav Zudin <szudin@tarantool.org> [19/06/19 15:53]:
> > > Do not spread the space:truncate() to replicas if the
> > > affected space is local and temporary.
> > 
> > While I agree with Georgy's comment that when it comes to
> > TEMPORARY spaces, we should replicate all DDL but TRUNCATE, for
> > node-local spaces I think nothing should be replicated. This is
> > the idea of node-local spaces to begin with.
> 
> On the other hand I don't know how to prevent replication of this
> data since it's already in the snapshot. Plus, if we filter out
> group-local spaces from the snapshot, we're going to break the
> vinyl deferred delete space.
> 
> But we definitely need spaces which are node-local and are not
> replicated anywhere, including DDL.
So let introduce a new kind of space instead of breaking existing one.


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces
  2019-06-19 12:48 [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces Stanislav Zudin
  2019-06-20  9:00 ` [tarantool-patches] " Konstantin Osipov
@ 2019-06-20 11:25 ` Vladimir Davydov
  2019-06-20 13:33   ` Stanislav Zudin
  1 sibling, 1 reply; 10+ messages in thread
From: Vladimir Davydov @ 2019-06-20 11:25 UTC (permalink / raw)
  To: Stanislav Zudin; +Cc: tarantool-patches, georgy

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.

> +	    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.

> +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.

> +box.space._truncate:select{}
> +---
> +- []
> +...
>  box.cfg{read_only = true} -- local spaces ignore read_only
>  ---
>  ...
> @@ -253,3 +298,6 @@ s2:drop()
>  s3:drop()
>  ---
>  ...
> +s4:drop()
> +---
> +...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces
  2019-06-20 11:25 ` [tarantool-patches] " Vladimir Davydov
@ 2019-06-20 13:33   ` Stanislav Zudin
  2019-06-21 15:05     ` Vladimir Davydov
  0 siblings, 1 reply; 10+ messages in thread
From: Stanislav Zudin @ 2019-06-20 13:33 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: tarantool-patches, georgy

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 <szudin@tarantool.org>
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces
  2019-06-20 13:33   ` Stanislav Zudin
@ 2019-06-21 15:05     ` Vladimir Davydov
  2019-06-25  9:01       ` Stanislav Zudin
  0 siblings, 1 reply; 10+ messages in thread
From: Vladimir Davydov @ 2019-06-21 15:05 UTC (permalink / raw)
  To: Stanislav Zudin; +Cc: tarantool-patches, georgy

On Thu, Jun 20, 2019 at 04:33:26PM +0300, Stanislav Zudin wrote:
> From b596d7d9cda4649a3c0ebb27d4887a57422ac458 Mon Sep 17 00:00:00 2001
> From: Stanislav Zudin <szudin@tarantool.org>
> 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})

You should test 'is_local' and 'tmemporary' separately now, as you
changed the condition in on_replace_dd_truncate. Please fix.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces
  2019-06-21 15:05     ` Vladimir Davydov
@ 2019-06-25  9:01       ` Stanislav Zudin
  2019-06-25 12:56         ` Vladimir Davydov
  0 siblings, 1 reply; 10+ messages in thread
From: Stanislav Zudin @ 2019-06-25  9:01 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: tarantool-patches, georgy

Please find the updated patch below.


On 21.06.2019 18:05, Vladimir Davydov wrote:
>> +-- 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})
> 
> You should test 'is_local' and 'tmemporary' separately now, as you
> changed the condition in on_replace_dd_truncate. Please fix.
> 

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   | 129 +++++++++++++++++++++++++
  test/replication/local_spaces.test.lua |  51 ++++++++++
  3 files changed, 190 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..6b4c1999d 100644
--- a/test/replication/local_spaces.result
+++ b/test/replication/local_spaces.result
@@ -71,6 +71,36 @@ 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})
+---
+...
+_ = s4:create_index('pk')
+---
+...
+s4.is_local
+---
+- true
+...
+s4.temporary
+---
+- false
+...
+s5 = box.schema.space.create('test5', {temporary = true})
+---
+...
+_ = s5:create_index('pk')
+---
+...
+s5.is_local
+---
+- false
+...
+s5.temporary
+---
+- true
+...
  _ = s1:insert{1}
  ---
  ...
@@ -80,6 +110,12 @@ _ = s2:insert{1}
  _ = s3:insert{1}
  ---
  ...
+_ = s4:insert{1}
+---
+...
+_ = s5:insert{1}
+---
+...
  box.snapshot()
  ---
  - ok
@@ -93,6 +129,12 @@ _ = s2:insert{2}
  _ = s3:insert{2}
  ---
  ...
+_ = s4:insert{2}
+---
+...
+_ = s5:insert{2}
+---
+...
  box.schema.user.grant('guest', 'replication')
  ---
  ...
@@ -124,6 +166,22 @@ box.space.test3.temporary
  ---
  - true
  ...
+box.space.test4.is_local
+---
+- true
+...
+box.space.test4.temporary
+---
+- false
+...
+box.space.test5.is_local
+---
+- false
+...
+box.space.test5.temporary
+---
+- true
+...
  box.space.test1:select()
  ---
  - - [1]
@@ -137,6 +195,74 @@ box.space.test3:select()
  ---
  - []
  ...
+box.space.test4:select()
+---
+- []
+...
+box.space.test5:select()
+---
+- []
+...
+-- To check truncation fill replica's copy with 2 entries
+_=box.space.test4:insert{4}
+---
+...
+_=box.space.test4:insert{5}
+---
+...
+_=box.space.test5:insert{4}
+---
+...
+_=box.space.test5:insert{5}
+---
+...
+box.space.test4:select()
+---
+- - [4]
+  - [5]
+...
+box.space.test5:select()
+---
+- - [4]
+  - [5]
+...
+-- truncate temp & local space on master
+test_run:cmd("switch default")
+---
+- true
+...
+s4:truncate()
+---
+...
+s5:truncate()
+---
+...
+-- Expect two records
+box.space._truncate:count()
+---
+- 2
+...
+-- 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.space.test5:select()
+---
+- - [4]
+  - [5]
+...
  box.cfg{read_only = true} -- local spaces ignore read_only
  ---
  ...
@@ -253,3 +379,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..6e2ad2dcc 100644
--- a/test/replication/local_spaces.test.lua
+++ b/test/replication/local_spaces.test.lua
@@ -33,13 +33,30 @@ _ = 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})
+_ = s4:create_index('pk')
+s4.is_local
+s4.temporary
+
+s5 = box.schema.space.create('test5', {temporary = true})
+_ = s5:create_index('pk')
+s5.is_local
+s5.temporary
+
  _ = s1:insert{1}
  _ = s2:insert{1}
  _ = s3:insert{1}
+_ = s4:insert{1}
+_ = s5:insert{1}
  box.snapshot()
  _ = s1:insert{2}
  _ = s2:insert{2}
  _ = s3:insert{2}
+_ = s4:insert{2}
+_ = s5:insert{2}
+

  box.schema.user.grant('guest', 'replication')
  test_run:cmd("create server replica with rpl_master=default, 
script='replication/replica.lua'")
@@ -50,9 +67,42 @@ 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.test5.is_local
+box.space.test5.temporary
  box.space.test1:select()
  box.space.test2:select()
  box.space.test3:select()
+box.space.test4:select()
+box.space.test5:select()
+
+-- To check truncation fill replica's copy with 2 entries
+_=box.space.test4:insert{4}
+_=box.space.test4:insert{5}
+_=box.space.test5:insert{4}
+_=box.space.test5:insert{5}
+box.space.test4:select()
+box.space.test5:select()
+
+-- truncate temp & local space on master
+test_run:cmd("switch default")
+s4:truncate()
+s5:truncate()
+-- Expect two records
+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.space.test5: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 +137,4 @@ s3:select()
  s1:drop()
  s2:drop()
  s3:drop()
+s4:drop()
-- 
2.17.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces
  2019-06-25  9:01       ` Stanislav Zudin
@ 2019-06-25 12:56         ` Vladimir Davydov
  0 siblings, 0 replies; 10+ messages in thread
From: Vladimir Davydov @ 2019-06-25 12:56 UTC (permalink / raw)
  To: Stanislav Zudin; +Cc: tarantool-patches, georgy

Pushed to master and 2.1.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-06-25 12:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-19 12:48 [tarantool-patches] [PATCH] ddl: No replication for temp and local spaces Stanislav Zudin
2019-06-20  9:00 ` [tarantool-patches] " Konstantin Osipov
2019-06-20  9:02   ` Konstantin Osipov
2019-06-20 10:35     ` Георгий Кириченко
2019-06-20 10:34   ` Георгий Кириченко
2019-06-20 11:25 ` [tarantool-patches] " Vladimir Davydov
2019-06-20 13:33   ` Stanislav Zudin
2019-06-21 15:05     ` Vladimir Davydov
2019-06-25  9:01       ` Stanislav Zudin
2019-06-25 12:56         ` Vladimir Davydov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox