[Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Serge Petrenko]

Since the anonymous replica implementation, it was forbidden to
replicate (join/subscribe/register) from anonymous instances.
Actually, only joining and register should be banned. Anonymous replica
isn't able to register its peer in _cluster  anyway.
Allow subscribing to anonymous instances and  remove all the errors
mentioning unsupported replication from anonymous replica. Join and
register are covered by ER_READONLY errors, since anonymous replicas
must be read-only.

Closes #4696
---
https://github.com/tarantool/tarantool/tree/sp/gh-4696-anon-replica-follow
https://github.com/tarantool/tarantool/issues/4696

There's one open question: should we allow 'normal' instances to follow
anonymous instances? The current patch implementation allows this.
I see one possible use for allowing normal instances to follow anonymous ones:
in case such an anonymous instance can be failed over to with its transition to
master. Then it would be better if all the cluster members followed it in
advance, to instantly start receiving changes from it when it becomes 'normal'
and read-write.

@ChangeLog:
  - Allow anonymous replicas to be followed by other ones (gh-4696).

 src/box/applier.cc             |   3 +-
 src/box/box.cc                 |  18 ---
 src/box/replication.cc         |   5 -
 test/replication/anon.result   | 229 +++++++++++++++++++++++----------
 test/replication/anon.test.lua |  88 +++++++++----
 5 files changed, 226 insertions(+), 117 deletions(-)

diff --git a/src/box/applier.cc b/src/box/applier.cc
index 68de3c08c..96068f098 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -636,8 +636,7 @@ applier_read_tx(struct applier *applier, struct stailq *rows)
 			xrow_decode_error_xc(row);
 
 		/* Replication request. */
-		if (row->replica_id == REPLICA_ID_NIL ||
-		    row->replica_id >= VCLOCK_MAX) {
+		if (row->replica_id >= VCLOCK_MAX) {
 			/*
 			 * A safety net, this can only occur
 			 * if we're fed a strangely broken xlog.
diff --git a/src/box/box.cc b/src/box/box.cc
index f4541b577..7a272900c 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1577,12 +1577,6 @@ box_process_register(struct ev_io *io, struct xrow_header *header)
 	if (tt_uuid_is_equal(&instance_uuid, &INSTANCE_UUID))
 		tnt_raise(ClientError, ER_CONNECTION_TO_SELF);
 
-	/* Forbid replication from an anonymous instance. */
-	if (replication_anon) {
-		tnt_raise(ClientError, ER_UNSUPPORTED, "Replication",
-			  "replicating from an anonymous instance.");
-	}
-
 	access_check_universe_xc(PRIV_R);
 	/* We only get register requests from anonymous instances. */
 	struct replica *replica = replica_by_uuid(&instance_uuid);
@@ -1711,12 +1705,6 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
 	if (tt_uuid_is_equal(&instance_uuid, &INSTANCE_UUID))
 		tnt_raise(ClientError, ER_CONNECTION_TO_SELF);
 
-	/* Forbid replication from an anonymous instance. */
-	if (replication_anon) {
-		tnt_raise(ClientError, ER_UNSUPPORTED, "Replication",
-			  "replicating from an anonymous instance.");
-	}
-
 	/* Check permissions */
 	access_check_universe_xc(PRIV_R);
 
@@ -1825,12 +1813,6 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
 	if (tt_uuid_is_equal(&replica_uuid, &INSTANCE_UUID))
 		tnt_raise(ClientError, ER_CONNECTION_TO_SELF);
 
-	/* Forbid replication from an anonymous instance. */
-	if (replication_anon) {
-		tnt_raise(ClientError, ER_UNSUPPORTED, "Replication",
-			  "replicating from an anonymous instance.");
-	}
-
 	/* Check permissions */
 	access_check_universe_xc(PRIV_R);
 
diff --git a/src/box/replication.cc b/src/box/replication.cc
index 7c10fb6f2..0c6bce9f5 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -904,11 +904,6 @@ replica_on_relay_stop(struct replica *replica)
 			replica->gc = NULL;
 		} else {
 			assert(replica->gc == NULL);
-			/*
-			 * We do not replicate from anonymous
-			 * replicas.
-			 */
-			assert(replica->applier == NULL);
 		}
 	}
 	if (replica_is_orphan(replica)) {
diff --git a/test/replication/anon.result b/test/replication/anon.result
index cbbeeef09..997112c84 100644
--- a/test/replication/anon.result
+++ b/test/replication/anon.result
@@ -157,75 +157,10 @@ test_run:cmd('switch default')
  | - 1
  | ...
 
--- Test that replication (even anonymous) from an anonymous
--- instance is forbidden. An anonymous replica will fetch
--- a snapshot though.
-test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon,\
-             script="replication/anon2.lua"]])
- | ---
- | - true
- | ...
-test_run:cmd('start server replica_anon2')
- | ---
- | - true
- | ...
-test_run:wait_log('replica_anon2',\
-                  'Replication does not support replicating from an anonymous instance',\
-                  nil, 10)
- | ---
- | - Replication does not support replicating from an anonymous instance
- | ...
-test_run:cmd('switch replica_anon2')
- | ---
- | - true
- | ...
-a = box.info.vclock[1]
- | ---
- | ...
--- The instance did fetch a snapshot.
-a > 0
- | ---
- | - true
- | ...
--- 0-th vclock component isn't propagated across the cluster.
-box.info.vclock[0]
- | ---
- | - null
- | ...
-test_run:cmd('switch default')
- | ---
- | - true
- | ...
-box.space.test:insert{2}
- | ---
- | - [2]
- | ...
-test_run:cmd("switch replica_anon2")
- | ---
- | - true
- | ...
--- Second replica doesn't follow master through the
--- 1st one. Replication from an anonymous instance
--- is forbidden indeed.
-box.info.vclock[1] == a or box.info.vclock[1]
- | ---
- | - true
- | ...
-
 test_run:cmd('switch replica_anon')
  | ---
  | - true
  | ...
-
-test_run:cmd('stop server replica_anon2')
- | ---
- | - true
- | ...
-test_run:cmd('delete server replica_anon2')
- | ---
- | - true
- | ...
-
 -- Promote anonymous replica.
 box.cfg{replication_anon=false}
  | ---
@@ -388,3 +323,167 @@ test_run:cmd('delete server replica_anon')
  | ---
  | - true
  | ...
+
+--
+-- gh-4696. Following an anonymous replica.
+--
+box.schema.user.grant('guest', 'replication')
+ | ---
+ | ...
+
+test_run:cmd([[create server replica_anon1 with rpl_master=default,\
+             script="replication/anon1.lua"]])
+ | ---
+ | - true
+ | ...
+test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon1,\
+             script="replication/anon2.lua"]])
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica_anon1')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica_anon2')
+ | ---
+ | - true
+ | ...
+_ =  box.schema.space.create('test')
+ | ---
+ | ...
+_ = box.space.test:create_index('pk')
+ | ---
+ | ...
+box.space.test:insert{1}
+ | ---
+ | - [1]
+ | ...
+
+-- Check that master's changes are propagated to replica2,
+-- following an anonymous replica1.
+test_run:cmd('switch replica_anon2')
+ | ---
+ | - true
+ | ...
+box.space.test:select{}
+ | ---
+ | - - [1]
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server replica_anon2')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica_anon2')
+ | ---
+ | - true
+ | ...
+
+-- Check that joining to an anonymous replica is prohibited.
+test_run:cmd([[create server replica with rpl_master=replica_anon1,\
+             script="replication/replica.lua"]])
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica with crash_expected=True')
+ | ---
+ | - false
+ | ...
+test_run:cmd('delete server replica')
+ | ---
+ | - true
+ | ...
+
+-- A normal instance (already joined) can follow an anonymous
+-- replica.
+test_run:cmd([[create server replica with rpl_master=default,\
+             script="replication/replica.lua"]])
+ | ---
+ | - true
+ | ...
+test_run:cmd('start  server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd('switch replica')
+ | ---
+ | - true
+ | ...
+test_run:wait_upstream(1, {status='follow'})
+ | ---
+ | - true
+ | ...
+box.info.id
+ | ---
+ | - 2
+ | ...
+test_run:cmd('set variable repl_source to "replica_anon1.listen"')
+ | ---
+ | - true
+ | ...
+box.cfg{replication=repl_source}
+ | ---
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
+box.space.test:insert{2}
+ | ---
+ | - [2]
+ | ...
+
+test_run:cmd('switch replica_anon1')
+ | ---
+ | - true
+ | ...
+-- Check that the new replica follows an anonymous one.
+test_run:wait_downstream(2, {status='follow'})
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('switch replica')
+ | ---
+ | - true
+ | ...
+box.space.test:select{}
+ | ---
+ | - - [1]
+ |   - [2]
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
+
+-- Cleanup.
+test_run:cmd('stop server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server replica_anon1')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica_anon1')
+ | ---
+ | - true
+ | ...
+box.space.test:drop()
+ | ---
+ | ...
+box.schema.user.revoke('guest', 'replication')
+ | ---
+ | ...
diff --git a/test/replication/anon.test.lua b/test/replication/anon.test.lua
index 627dc5c8e..cdbb8c7bb 100644
--- a/test/replication/anon.test.lua
+++ b/test/replication/anon.test.lua
@@ -53,34 +53,7 @@ test_run:cmd('switch default')
 -- Replica isn't visible on master.
 #box.info.replication
 
--- Test that replication (even anonymous) from an anonymous
--- instance is forbidden. An anonymous replica will fetch
--- a snapshot though.
-test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon,\
-             script="replication/anon2.lua"]])
-test_run:cmd('start server replica_anon2')
-test_run:wait_log('replica_anon2',\
-                  'Replication does not support replicating from an anonymous instance',\
-                  nil, 10)
-test_run:cmd('switch replica_anon2')
-a = box.info.vclock[1]
--- The instance did fetch a snapshot.
-a > 0
--- 0-th vclock component isn't propagated across the cluster.
-box.info.vclock[0]
-test_run:cmd('switch default')
-box.space.test:insert{2}
-test_run:cmd("switch replica_anon2")
--- Second replica doesn't follow master through the
--- 1st one. Replication from an anonymous instance
--- is forbidden indeed.
-box.info.vclock[1] == a or box.info.vclock[1]
-
 test_run:cmd('switch replica_anon')
-
-test_run:cmd('stop server replica_anon2')
-test_run:cmd('delete server replica_anon2')
-
 -- Promote anonymous replica.
 box.cfg{replication_anon=false}
 -- Cannot switch back after becoming "normal".
@@ -137,3 +110,64 @@ box.cfg{replication_anon = false}
 test_run:switch('default')
 test_run:cmd('stop server replica_anon')
 test_run:cmd('delete server replica_anon')
+
+--
+-- gh-4696. Following an anonymous replica.
+--
+box.schema.user.grant('guest', 'replication')
+
+test_run:cmd([[create server replica_anon1 with rpl_master=default,\
+             script="replication/anon1.lua"]])
+test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon1,\
+             script="replication/anon2.lua"]])
+test_run:cmd('start server replica_anon1')
+test_run:cmd('start server replica_anon2')
+_ =  box.schema.space.create('test')
+_ = box.space.test:create_index('pk')
+box.space.test:insert{1}
+
+-- Check that master's changes are propagated to replica2,
+-- following an anonymous replica1.
+test_run:cmd('switch replica_anon2')
+box.space.test:select{}
+
+test_run:cmd('switch default')
+test_run:cmd('stop server replica_anon2')
+test_run:cmd('delete server replica_anon2')
+
+-- Check that joining to an anonymous replica is prohibited.
+test_run:cmd([[create server replica with rpl_master=replica_anon1,\
+             script="replication/replica.lua"]])
+test_run:cmd('start server replica with crash_expected=True')
+test_run:cmd('delete server replica')
+
+-- A normal instance (already joined) can follow an anonymous
+-- replica.
+test_run:cmd([[create server replica with rpl_master=default,\
+             script="replication/replica.lua"]])
+test_run:cmd('start  server replica')
+test_run:cmd('switch replica')
+test_run:wait_upstream(1, {status='follow'})
+box.info.id
+test_run:cmd('set variable repl_source to "replica_anon1.listen"')
+box.cfg{replication=repl_source}
+
+test_run:cmd('switch default')
+box.space.test:insert{2}
+
+test_run:cmd('switch replica_anon1')
+-- Check that the new replica follows an anonymous one.
+test_run:wait_downstream(2, {status='follow'})
+
+test_run:cmd('switch replica')
+box.space.test:select{}
+
+test_run:cmd('switch default')
+
+-- Cleanup.
+test_run:cmd('stop server replica')
+test_run:cmd('stop server replica_anon1')
+test_run:cmd('delete server replica')
+test_run:cmd('delete server replica_anon1')
+box.space.test:drop()
+box.schema.user.revoke('guest', 'replication')
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Cyrill Gorcunov]

On Mon, Apr 20, 2020 at 04:11:15PM +0300, Serge Petrenko wrote:
> Since the anonymous replica implementation, it was forbidden to
> replicate (join/subscribe/register) from anonymous instances.
> Actually, only joining and register should be banned. Anonymous replica
> isn't able to register its peer in _cluster  anyway.
> Allow subscribing to anonymous instances and  remove all the errors
> mentioning unsupported replication from anonymous replica. Join and
> register are covered by ER_READONLY errors, since anonymous replicas
> must be read-only.
> 
> Closes #4696
> ---

I don't see any obvious errors here. Still I'm far from being repication
expert. So I can put

Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>

only.

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

On 20/04/2020 15:11, Serge Petrenko wrote:
> Since the anonymous replica implementation, it was forbidden to
> replicate (join/subscribe/register) from anonymous instances.
> Actually, only joining and register should be banned. Anonymous replica
> isn't able to register its peer in _cluster  anyway.
> Allow subscribing to anonymous instances and  remove all the errors
> mentioning unsupported replication from anonymous replica. Join and
> register are covered by ER_READONLY errors, since anonymous replicas
> must be read-only.

Seems like some amount of double whitespaces here and in the patch
(I noticed people with new macbooks have them often, huh).

> Closes #4696
> ---
> https://github.com/tarantool/tarantool/tree/sp/gh-4696-anon-replica-follow
> https://github.com/tarantool/tarantool/issues/4696
> 
> There's one open question: should we allow 'normal' instances to follow
> anonymous instances? The current patch implementation allows this.
> I see one possible use for allowing normal instances to follow anonymous ones:
> in case such an anonymous instance can be failed over to with its transition to
> master. Then it would be better if all the cluster members followed it in
> advance, to instantly start receiving changes from it when it becomes 'normal'
> and read-write.
> 
> @ChangeLog:
>   - Allow anonymous replicas to be followed by other ones (gh-4696).
> 
> diff --git a/test/replication/anon.result b/test/replication/anon.result
> index cbbeeef09..997112c84 100644
> --- a/test/replication/anon.result
> +++ b/test/replication/anon.result
> @@ -388,3 +323,167 @@ test_run:cmd('delete server replica_anon')
>   | ---
>   | - true
>   | ...
> +
> +--
> +-- gh-4696. Following an anonymous replica.
> +--
> +box.schema.user.grant('guest', 'replication')
> + | ---
> + | ...
> +
> +test_run:cmd([[create server replica_anon1 with rpl_master=default,\
> +             script="replication/anon1.lua"]])
> + | ---
> + | - true
> + | ...
> +test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon1,\
> +             script="replication/anon2.lua"]])
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica_anon1')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica_anon2')
> + | ---
> + | - true
> + | ...
> +_ =  box.schema.space.create('test')
> + | ---
> + | ...
> +_ = box.space.test:create_index('pk')
> + | ---
> + | ...
> +box.space.test:insert{1}
> + | ---
> + | - [1]
> + | ...
> +
> +-- Check that master's changes are propagated to replica2,
> +-- following an anonymous replica1.
> +test_run:cmd('switch replica_anon2')
> + | ---
> + | - true
> + | ...
> +box.space.test:select{}
> + | ---
> + | - - [1]
> + | ...

Nit: the test is potentially flaky. The change may not manage
to propagate to replica_anon2 while you switch to it. A more
robust option would be to use test_run:wait_cond() to wait
until the space has this tuple.

> +
> +test_run:cmd('switch default')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('stop server replica_anon2')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server replica_anon2')
> + | ---
> + | - true
> + | ...
> +
> +-- Check that joining to an anonymous replica is prohibited.
> +test_run:cmd([[create server replica with rpl_master=replica_anon1,\
> +             script="replication/replica.lua"]])
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica with crash_expected=True')
> + | ---
> + | - false
> + | ...
> +test_run:cmd('delete server replica')
> + | ---
> + | - true
> + | ...
> +
> +-- A normal instance (already joined) can follow an anonymous
> +-- replica.
> +test_run:cmd([[create server replica with rpl_master=default,\
> +             script="replication/replica.lua"]])

Isn't it dangerous? So the master thinks this replica is dead,
even though it is not, but just connected through an anon proxy?
In case it is true, and the master has a replication quorum, that
may turn him into orphan when the normal replicas are connected
via an anon replica.

This is related to your question whether we should allow to normal
replicas replicate from anon. I don't have a strong opinion here.
I just was always thinking that anon replicas are some kind of
satellite instances which never become a master, or even a read-only
replica unless everything else is dead.

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Serge Petrenko]

28.04.2020 23:55, Vladislav Shpilevoy пишет:
> Hi! Thanks for the patch!

Thanks for the review!

>
> On 20/04/2020 15:11, Serge Petrenko wrote:
>> Since the anonymous replica implementation, it was forbidden to
>> replicate (join/subscribe/register) from anonymous instances.
>> Actually, only joining and register should be banned. Anonymous replica
>> isn't able to register its peer in _cluster  anyway.
>> Allow subscribing to anonymous instances and  remove all the errors
>> mentioning unsupported replication from anonymous replica. Join and
>> register are covered by ER_READONLY errors, since anonymous replicas
>> must be read-only.
> Seems like some amount of double whitespaces here and in the patch
> (I noticed people with new macbooks have them often, huh).


Thanks, fixed.  This keybboard is  killing me.


>> Closes #4696
>> ---
>> https://github.com/tarantool/tarantool/tree/sp/gh-4696-anon-replica-follow
>> https://github.com/tarantool/tarantool/issues/4696
>>
>> There's one open question: should we allow 'normal' instances to follow
>> anonymous instances? The current patch implementation allows this.
>> I see one possible use for allowing normal instances to follow anonymous ones:
>> in case such an anonymous instance can be failed over to with its transition to
>> master. Then it would be better if all the cluster members followed it in
>> advance, to instantly start receiving changes from it when it becomes 'normal'
>> and read-write.
>>
>> @ChangeLog:
>>    - Allow anonymous replicas to be followed by other ones (gh-4696).
>>
>> diff --git a/test/replication/anon.result b/test/replication/anon.result
>> index cbbeeef09..997112c84 100644
>> --- a/test/replication/anon.result
>> +++ b/test/replication/anon.result
>> @@ -388,3 +323,167 @@ test_run:cmd('delete server replica_anon')
>>    | ---
>>    | - true
>>    | ...
>> +
>> +--
>> +-- gh-4696. Following an anonymous replica.
>> +--
>> +box.schema.user.grant('guest', 'replication')
>> + | ---
>> + | ...
>> +
>> +test_run:cmd([[create server replica_anon1 with rpl_master=default,\
>> +             script="replication/anon1.lua"]])
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon1,\
>> +             script="replication/anon2.lua"]])
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd('start server replica_anon1')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd('start server replica_anon2')
>> + | ---
>> + | - true
>> + | ...
>> +_ =  box.schema.space.create('test')
>> + | ---
>> + | ...
>> +_ = box.space.test:create_index('pk')
>> + | ---
>> + | ...
>> +box.space.test:insert{1}
>> + | ---
>> + | - [1]
>> + | ...
>> +
>> +-- Check that master's changes are propagated to replica2,
>> +-- following an anonymous replica1.
>> +test_run:cmd('switch replica_anon2')
>> + | ---
>> + | - true
>> + | ...
>> +box.space.test:select{}
>> + | ---
>> + | - - [1]
>> + | ...
> Nit: the test is potentially flaky. The change may not manage
> to propagate to replica_anon2 while you switch to it. A more
> robust option would be to use test_run:wait_cond() to wait
> until the space has this tuple.

Added wait_cond()


>> +
>> +test_run:cmd('switch default')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd('stop server replica_anon2')
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd('delete server replica_anon2')
>> + | ---
>> + | - true
>> + | ...
>> +
>> +-- Check that joining to an anonymous replica is prohibited.
>> +test_run:cmd([[create server replica with rpl_master=replica_anon1,\
>> +             script="replication/replica.lua"]])
>> + | ---
>> + | - true
>> + | ...
>> +test_run:cmd('start server replica with crash_expected=True')
>> + | ---
>> + | - false
>> + | ...
>> +test_run:cmd('delete server replica')
>> + | ---
>> + | - true
>> + | ...
>> +
>> +-- A normal instance (already joined) can follow an anonymous
>> +-- replica.
>> +test_run:cmd([[create server replica with rpl_master=default,\
>> +             script="replication/replica.lua"]])
> Isn't it dangerous? So the master thinks this replica is dead,
> even though it is not, but just connected through an anon proxy?
> In case it is true, and the master has a replication quorum, that
> may turn him into orphan when the normal replicas are connected
> via an anon replica.
>
> This is related to your question whether we should allow to normal
> replicas replicate from anon. I don't have a strong opinion here.
> I just was always thinking that anon replicas are some kind of
> satellite instances which never become a master, or even a read-only
> replica unless everything else is dead.


Ok then, let's prohibit normal replicas to follow anonymous ones.
I've  updated the test case and commit message

The diff is below.


diff --git a/src/box/applier.cc b/src/box/applier.cc
index 96068f098..c6deeff1b 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -640,6 +640,8 @@ applier_read_tx(struct applier *applier, struct 
stailq *rows)
                         /*
                          * A safety net, this can only occur
                          * if we're fed a strangely broken xlog.
+                        * row->replica_id == 0, when reading
+                        * heartbeats from an anonymous instance.
                          */
                         tnt_raise(ClientError, ER_UNKNOWN_REPLICA,
int2str(row->replica_id),
diff --git a/src/box/box.cc b/src/box/box.cc
index 7a272900c..f9b7d5bcf 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1813,6 +1819,15 @@ box_process_subscribe(struct ev_io *io, struct 
xrow_header *header)
         if (tt_uuid_is_equal(&replica_uuid, &INSTANCE_UUID))
                 tnt_raise(ClientError, ER_CONNECTION_TO_SELF);

+       /*
+        * Do not allow non-anonymous followers for anonymous
+        * instances.
+        */
+       if (replication_anon && !anon) {
+               tnt_raise(ClientError, ER_UNSUPPORTED, "Anonymous replica",
+                         "non-anonymous followers.");
+       }
+
         /* Check permissions */
         access_check_universe_xc(PRIV_R);

diff --git a/test/replication/anon.result b/test/replication/anon.result
index 997112c84..8782212ca 100644
--- a/test/replication/anon.result
+++ b/test/replication/anon.result
@@ -330,6 +330,12 @@ test_run:cmd('delete server replica_anon')
  box.schema.user.grant('guest', 'replication')
   | ---
   | ...
+_ = box.schema.space.create('test')
+ | ---
+ | ...
+_ = box.space.test:create_index('pk')
+ | ---
+ | ...

  test_run:cmd([[create server replica_anon1 with rpl_master=default,\
               script="replication/anon1.lua"]])
@@ -349,12 +355,7 @@ test_run:cmd('start server replica_anon2')
   | ---
   | - true
   | ...
-_ =  box.schema.space.create('test')
- | ---
- | ...
-_ = box.space.test:create_index('pk')
- | ---
- | ...
+
  box.space.test:insert{1}
   | ---
   | - [1]
@@ -366,9 +367,9 @@ test_run:cmd('switch replica_anon2')
   | ---
   | - true
   | ...
-box.space.test:select{}
+test_run:wait_cond(function() return box.space.test:get{1} ~= nil  end, 10)
   | ---
- | - - [1]
+ | - true
   | ...

  test_run:cmd('switch default')
@@ -399,14 +400,14 @@ test_run:cmd('delete server replica')
   | - true
   | ...

--- A normal instance (already joined) can follow an anonymous
+-- A normal instance (already joined) can't follow an anonymous
  -- replica.
  test_run:cmd([[create server replica with rpl_master=default,\
               script="replication/replica.lua"]])
   | ---
   | - true
   | ...
-test_run:cmd('start  server replica')
+test_run:cmd('start server replica')
   | ---
   | - true
   | ...
@@ -429,36 +430,10 @@ test_run:cmd('set variable repl_source to 
"replica_anon1.listen"')
  box.cfg{replication=repl_source}
   | ---
   | ...
-
-test_run:cmd('switch default')
- | ---
- | - true
- | ...
-box.space.test:insert{2}
- | ---
- | - [2]
- | ...
-
-test_run:cmd('switch replica_anon1')
- | ---
- | - true
- | ...
--- Check that the new replica follows an anonymous one.
-test_run:wait_downstream(2, {status='follow'})
+test_run:wait_log('replica', 'ER_UNSUPPORTED: Anonymous replica does 
not support non.anonymous followers.', nil, 10)
   | ---
- | - true
+ | - 'ER_UNSUPPORTED: Anonymous replica does not support non-anonymous 
followers.'
   | ...
-
-test_run:cmd('switch replica')
- | ---
- | - true
- | ...
-box.space.test:select{}
- | ---
- | - - [1]
- |   - [2]
- | ...
-
  test_run:cmd('switch default')
   | ---
   | - true
diff --git a/test/replication/anon.test.lua b/test/replication/anon.test.lua
index cdbb8c7bb..39393e24a 100644
--- a/test/replication/anon.test.lua
+++ b/test/replication/anon.test.lua
@@ -115,6 +115,8 @@ test_run:cmd('delete server replica_anon')
  -- gh-4696. Following an anonymous replica.
  --
  box.schema.user.grant('guest', 'replication')
+_ = box.schema.space.create('test')
+_ = box.space.test:create_index('pk')

  test_run:cmd([[create server replica_anon1 with rpl_master=default,\
               script="replication/anon1.lua"]])
@@ -122,14 +124,13 @@ test_run:cmd([[create server replica_anon2 with 
rpl_master=replica_anon1,\
               script="replication/anon2.lua"]])
  test_run:cmd('start server replica_anon1')
  test_run:cmd('start server replica_anon2')
-_ =  box.schema.space.create('test')
-_ = box.space.test:create_index('pk')
+
  box.space.test:insert{1}

  -- Check that master's changes are propagated to replica2,
  -- following an anonymous replica1.
  test_run:cmd('switch replica_anon2')
-box.space.test:select{}
+test_run:wait_cond(function() return box.space.test:get{1} ~= nil  end, 10)

  test_run:cmd('switch default')
  test_run:cmd('stop server replica_anon2')
@@ -141,27 +142,17 @@ test_run:cmd([[create server replica with 
rpl_master=replica_anon1,\
  test_run:cmd('start server replica with crash_expected=True')
  test_run:cmd('delete server replica')

--- A normal instance (already joined) can follow an anonymous
+-- A normal instance (already joined) can't follow an anonymous
  -- replica.
  test_run:cmd([[create server replica with rpl_master=default,\
               script="replication/replica.lua"]])
-test_run:cmd('start  server replica')
+test_run:cmd('start server replica')
  test_run:cmd('switch replica')
  test_run:wait_upstream(1, {status='follow'})
  box.info.id
  test_run:cmd('set variable repl_source to "replica_anon1.listen"')
  box.cfg{replication=repl_source}
-
-test_run:cmd('switch default')
-box.space.test:insert{2}
-
-test_run:cmd('switch replica_anon1')
--- Check that the new replica follows an anonymous one.
-test_run:wait_downstream(2, {status='follow'})
-
-test_run:cmd('switch replica')
-box.space.test:select{}
-
+test_run:wait_log('replica', 'ER_UNSUPPORTED: Anonymous replica does 
not support non.anonymous followers.', nil, 10)
  test_run:cmd('switch default')

  -- Cleanup.


-- 
--
Serge Petrenko

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

See 2 keyboard-related nits below. Everything else LGTM. 

> diff --git a/test/replication/anon.result b/test/replication/anon.result
> index 997112c84..8782212ca 100644
> --- a/test/replication/anon.result
> +++ b/test/replication/anon.result
> @@ -366,9 +367,9 @@ test_run:cmd('switch replica_anon2')
>   | ---
>   | - true
>   | ...
> -box.space.test:select{}
> +test_run:wait_cond(function() return box.space.test:get{1} ~= nil  end, 10)

1. :) Double whitespace after '~= nil'.

>   | ---
> - | - - [1]
> + | - true
>   | ...
> @@ -429,36 +430,10 @@ test_run:cmd('set variable repl_source to "replica_anon1.listen"')
>  box.cfg{replication=repl_source}
>   | ---
>   | ...
> -
> -test_run:cmd('switch default')
> - | ---
> - | - true
> - | ...
> -box.space.test:insert{2}
> - | ---
> - | - [2]
> - | ...
> -
> -test_run:cmd('switch replica_anon1')
> - | ---
> - | - true
> - | ...
> --- Check that the new replica follows an anonymous one.
> -test_run:wait_downstream(2, {status='follow'})
> +test_run:wait_log('replica', 'ER_UNSUPPORTED: Anonymous replica does not support non.anonymous followers.', nil, 10)

2. Perhaps 'non-anonymous' instead of 'non.anonymous'?

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Serge Petrenko]

07.05.2020 00:51, Vladislav Shpilevoy пишет:
> Hi! Thanks for the fixes!


Thanks for the review!

> See 2 keyboard-related nits below. Everything else LGTM.
>
>> diff --git a/test/replication/anon.result b/test/replication/anon.result
>> index 997112c84..8782212ca 100644
>> --- a/test/replication/anon.result
>> +++ b/test/replication/anon.result
>> @@ -366,9 +367,9 @@ test_run:cmd('switch replica_anon2')
>>    | ---
>>    | - true
>>    | ...
>> -box.space.test:select{}
>> +test_run:wait_cond(function() return box.space.test:get{1} ~= nil  end, 10)
> 1. :) Double whitespace after '~= nil'.

Thanks =)

Fixed.

>>    | ---
>> - | - - [1]
>> + | - true
>>    | ...
>> @@ -429,36 +430,10 @@ test_run:cmd('set variable repl_source to "replica_anon1.listen"')
>>   box.cfg{replication=repl_source}
>>    | ---
>>    | ...
>> -
>> -test_run:cmd('switch default')
>> - | ---
>> - | - true
>> - | ...
>> -box.space.test:insert{2}
>> - | ---
>> - | - [2]
>> - | ...
>> -
>> -test_run:cmd('switch replica_anon1')
>> - | ---
>> - | - true
>> - | ...
>> --- Check that the new replica follows an anonymous one.
>> -test_run:wait_downstream(2, {status='follow'})
>> +test_run:wait_log('replica', 'ER_UNSUPPORTED: Anonymous replica does not support non.anonymous followers.', nil, 10)
> 2. Perhaps 'non-anonymous' instead of 'non.anonymous'?

Dot  matches anything, and I couldn't find a way to escape '-', it must 
be a special symbol of sorts.
So let it be 'non.anonymous'.

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

>>> @@ -429,36 +430,10 @@ test_run:cmd('set variable repl_source to "replica_anon1.listen"')
>>>   box.cfg{replication=repl_source}
>>>    | ---
>>>    | ...
>>> -
>>> -test_run:cmd('switch default')
>>> - | ---
>>> - | - true
>>> - | ...
>>> -box.space.test:insert{2}
>>> - | ---
>>> - | - [2]
>>> - | ...
>>> -
>>> -test_run:cmd('switch replica_anon1')
>>> - | ---
>>> - | - true
>>> - | ...
>>> --- Check that the new replica follows an anonymous one.
>>> -test_run:wait_downstream(2, {status='follow'})
>>> +test_run:wait_log('replica', 'ER_UNSUPPORTED: Anonymous replica does not support non.anonymous followers.', nil, 10)
>> 2. Perhaps 'non-anonymous' instead of 'non.anonymous'?
> 
> Dot  matches anything, and I couldn't find a way to escape '-', it must be a special symbol of sorts.
> So let it be 'non.anonymous'.

I again forgot how regular expressions look in Lua. Yeah, sure then, '.' is fine.
Talking of escape symbol - it is %

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Serge Petrenko]

08.05.2020 00:53, Vladislav Shpilevoy пишет:
> Hi! Thanks for the fixes!
>
>>>> @@ -429,36 +430,10 @@ test_run:cmd('set variable repl_source to "replica_anon1.listen"')
>>>>    box.cfg{replication=repl_source}
>>>>     | ---
>>>>     | ...
>>>> -
>>>> -test_run:cmd('switch default')
>>>> - | ---
>>>> - | - true
>>>> - | ...
>>>> -box.space.test:insert{2}
>>>> - | ---
>>>> - | - [2]
>>>> - | ...
>>>> -
>>>> -test_run:cmd('switch replica_anon1')
>>>> - | ---
>>>> - | - true
>>>> - | ...
>>>> --- Check that the new replica follows an anonymous one.
>>>> -test_run:wait_downstream(2, {status='follow'})
>>>> +test_run:wait_log('replica', 'ER_UNSUPPORTED: Anonymous replica does not support non.anonymous followers.', nil, 10)
>>> 2. Perhaps 'non-anonymous' instead of 'non.anonymous'?
>> Dot  matches anything, and I couldn't find a way to escape '-', it must be a special symbol of sorts.
>> So let it be 'non.anonymous'.
> I again forgot how regular expressions look in Lua. Yeah, sure then, '.' is fine.
> Talking of escape symbol - it is %

Didn't know that, thanks!


-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Vladislav Shpilevoy]

LGTM.

On 20/04/2020 15:11, Serge Petrenko wrote:
> Since the anonymous replica implementation, it was forbidden to
> replicate (join/subscribe/register) from anonymous instances.
> Actually, only joining and register should be banned. Anonymous replica
> isn't able to register its peer in _cluster  anyway.
> Allow subscribing to anonymous instances and  remove all the errors
> mentioning unsupported replication from anonymous replica. Join and
> register are covered by ER_READONLY errors, since anonymous replicas
> must be read-only.
> 
> Closes #4696
> ---
> https://github.com/tarantool/tarantool/tree/sp/gh-4696-anon-replica-follow
> https://github.com/tarantool/tarantool/issues/4696
> 
> There's one open question: should we allow 'normal' instances to follow
> anonymous instances? The current patch implementation allows this.
> I see one possible use for allowing normal instances to follow anonymous ones:
> in case such an anonymous instance can be failed over to with its transition to
> master. Then it would be better if all the cluster members followed it in
> advance, to instantly start receiving changes from it when it becomes 'normal'
> and read-write.
> 
> @ChangeLog:
>   - Allow anonymous replicas to be followed by other ones (gh-4696).
> 
>  src/box/applier.cc             |   3 +-
>  src/box/box.cc                 |  18 ---
>  src/box/replication.cc         |   5 -
>  test/replication/anon.result   | 229 +++++++++++++++++++++++----------
>  test/replication/anon.test.lua |  88 +++++++++----
>  5 files changed, 226 insertions(+), 117 deletions(-)
> 
> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index 68de3c08c..96068f098 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -636,8 +636,7 @@ applier_read_tx(struct applier *applier, struct stailq *rows)
>  			xrow_decode_error_xc(row);
>  
>  		/* Replication request. */
> -		if (row->replica_id == REPLICA_ID_NIL ||
> -		    row->replica_id >= VCLOCK_MAX) {
> +		if (row->replica_id >= VCLOCK_MAX) {
>  			/*
>  			 * A safety net, this can only occur
>  			 * if we're fed a strangely broken xlog.
> diff --git a/src/box/box.cc b/src/box/box.cc
> index f4541b577..7a272900c 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -1577,12 +1577,6 @@ box_process_register(struct ev_io *io, struct xrow_header *header)
>  	if (tt_uuid_is_equal(&instance_uuid, &INSTANCE_UUID))
>  		tnt_raise(ClientError, ER_CONNECTION_TO_SELF);
>  
> -	/* Forbid replication from an anonymous instance. */
> -	if (replication_anon) {
> -		tnt_raise(ClientError, ER_UNSUPPORTED, "Replication",
> -			  "replicating from an anonymous instance.");
> -	}
> -
>  	access_check_universe_xc(PRIV_R);
>  	/* We only get register requests from anonymous instances. */
>  	struct replica *replica = replica_by_uuid(&instance_uuid);
> @@ -1711,12 +1705,6 @@ box_process_join(struct ev_io *io, struct xrow_header *header)
>  	if (tt_uuid_is_equal(&instance_uuid, &INSTANCE_UUID))
>  		tnt_raise(ClientError, ER_CONNECTION_TO_SELF);
>  
> -	/* Forbid replication from an anonymous instance. */
> -	if (replication_anon) {
> -		tnt_raise(ClientError, ER_UNSUPPORTED, "Replication",
> -			  "replicating from an anonymous instance.");
> -	}
> -
>  	/* Check permissions */
>  	access_check_universe_xc(PRIV_R);
>  
> @@ -1825,12 +1813,6 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
>  	if (tt_uuid_is_equal(&replica_uuid, &INSTANCE_UUID))
>  		tnt_raise(ClientError, ER_CONNECTION_TO_SELF);
>  
> -	/* Forbid replication from an anonymous instance. */
> -	if (replication_anon) {
> -		tnt_raise(ClientError, ER_UNSUPPORTED, "Replication",
> -			  "replicating from an anonymous instance.");
> -	}
> -
>  	/* Check permissions */
>  	access_check_universe_xc(PRIV_R);
>  
> diff --git a/src/box/replication.cc b/src/box/replication.cc
> index 7c10fb6f2..0c6bce9f5 100644
> --- a/src/box/replication.cc
> +++ b/src/box/replication.cc
> @@ -904,11 +904,6 @@ replica_on_relay_stop(struct replica *replica)
>  			replica->gc = NULL;
>  		} else {
>  			assert(replica->gc == NULL);
> -			/*
> -			 * We do not replicate from anonymous
> -			 * replicas.
> -			 */
> -			assert(replica->applier == NULL);
>  		}
>  	}
>  	if (replica_is_orphan(replica)) {
> diff --git a/test/replication/anon.result b/test/replication/anon.result
> index cbbeeef09..997112c84 100644
> --- a/test/replication/anon.result
> +++ b/test/replication/anon.result
> @@ -157,75 +157,10 @@ test_run:cmd('switch default')
>   | - 1
>   | ...
>  
> --- Test that replication (even anonymous) from an anonymous
> --- instance is forbidden. An anonymous replica will fetch
> --- a snapshot though.
> -test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon,\
> -             script="replication/anon2.lua"]])
> - | ---
> - | - true
> - | ...
> -test_run:cmd('start server replica_anon2')
> - | ---
> - | - true
> - | ...
> -test_run:wait_log('replica_anon2',\
> -                  'Replication does not support replicating from an anonymous instance',\
> -                  nil, 10)
> - | ---
> - | - Replication does not support replicating from an anonymous instance
> - | ...
> -test_run:cmd('switch replica_anon2')
> - | ---
> - | - true
> - | ...
> -a = box.info.vclock[1]
> - | ---
> - | ...
> --- The instance did fetch a snapshot.
> -a > 0
> - | ---
> - | - true
> - | ...
> --- 0-th vclock component isn't propagated across the cluster.
> -box.info.vclock[0]
> - | ---
> - | - null
> - | ...
> -test_run:cmd('switch default')
> - | ---
> - | - true
> - | ...
> -box.space.test:insert{2}
> - | ---
> - | - [2]
> - | ...
> -test_run:cmd("switch replica_anon2")
> - | ---
> - | - true
> - | ...
> --- Second replica doesn't follow master through the
> --- 1st one. Replication from an anonymous instance
> --- is forbidden indeed.
> -box.info.vclock[1] == a or box.info.vclock[1]
> - | ---
> - | - true
> - | ...
> -
>  test_run:cmd('switch replica_anon')
>   | ---
>   | - true
>   | ...
> -
> -test_run:cmd('stop server replica_anon2')
> - | ---
> - | - true
> - | ...
> -test_run:cmd('delete server replica_anon2')
> - | ---
> - | - true
> - | ...
> -
>  -- Promote anonymous replica.
>  box.cfg{replication_anon=false}
>   | ---
> @@ -388,3 +323,167 @@ test_run:cmd('delete server replica_anon')
>   | ---
>   | - true
>   | ...
> +
> +--
> +-- gh-4696. Following an anonymous replica.
> +--
> +box.schema.user.grant('guest', 'replication')
> + | ---
> + | ...
> +
> +test_run:cmd([[create server replica_anon1 with rpl_master=default,\
> +             script="replication/anon1.lua"]])
> + | ---
> + | - true
> + | ...
> +test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon1,\
> +             script="replication/anon2.lua"]])
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica_anon1')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica_anon2')
> + | ---
> + | - true
> + | ...
> +_ =  box.schema.space.create('test')
> + | ---
> + | ...
> +_ = box.space.test:create_index('pk')
> + | ---
> + | ...
> +box.space.test:insert{1}
> + | ---
> + | - [1]
> + | ...
> +
> +-- Check that master's changes are propagated to replica2,
> +-- following an anonymous replica1.
> +test_run:cmd('switch replica_anon2')
> + | ---
> + | - true
> + | ...
> +box.space.test:select{}
> + | ---
> + | - - [1]
> + | ...
> +
> +test_run:cmd('switch default')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('stop server replica_anon2')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server replica_anon2')
> + | ---
> + | - true
> + | ...
> +
> +-- Check that joining to an anonymous replica is prohibited.
> +test_run:cmd([[create server replica with rpl_master=replica_anon1,\
> +             script="replication/replica.lua"]])
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server replica with crash_expected=True')
> + | ---
> + | - false
> + | ...
> +test_run:cmd('delete server replica')
> + | ---
> + | - true
> + | ...
> +
> +-- A normal instance (already joined) can follow an anonymous
> +-- replica.
> +test_run:cmd([[create server replica with rpl_master=default,\
> +             script="replication/replica.lua"]])
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start  server replica')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('switch replica')
> + | ---
> + | - true
> + | ...
> +test_run:wait_upstream(1, {status='follow'})
> + | ---
> + | - true
> + | ...
> +box.info.id
> + | ---
> + | - 2
> + | ...
> +test_run:cmd('set variable repl_source to "replica_anon1.listen"')
> + | ---
> + | - true
> + | ...
> +box.cfg{replication=repl_source}
> + | ---
> + | ...
> +
> +test_run:cmd('switch default')
> + | ---
> + | - true
> + | ...
> +box.space.test:insert{2}
> + | ---
> + | - [2]
> + | ...
> +
> +test_run:cmd('switch replica_anon1')
> + | ---
> + | - true
> + | ...
> +-- Check that the new replica follows an anonymous one.
> +test_run:wait_downstream(2, {status='follow'})
> + | ---
> + | - true
> + | ...
> +
> +test_run:cmd('switch replica')
> + | ---
> + | - true
> + | ...
> +box.space.test:select{}
> + | ---
> + | - - [1]
> + |   - [2]
> + | ...
> +
> +test_run:cmd('switch default')
> + | ---
> + | - true
> + | ...
> +
> +-- Cleanup.
> +test_run:cmd('stop server replica')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('stop server replica_anon1')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server replica')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server replica_anon1')
> + | ---
> + | - true
> + | ...
> +box.space.test:drop()
> + | ---
> + | ...
> +box.schema.user.revoke('guest', 'replication')
> + | ---
> + | ...
> diff --git a/test/replication/anon.test.lua b/test/replication/anon.test.lua
> index 627dc5c8e..cdbb8c7bb 100644
> --- a/test/replication/anon.test.lua
> +++ b/test/replication/anon.test.lua
> @@ -53,34 +53,7 @@ test_run:cmd('switch default')
>  -- Replica isn't visible on master.
>  #box.info.replication
>  
> --- Test that replication (even anonymous) from an anonymous
> --- instance is forbidden. An anonymous replica will fetch
> --- a snapshot though.
> -test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon,\
> -             script="replication/anon2.lua"]])
> -test_run:cmd('start server replica_anon2')
> -test_run:wait_log('replica_anon2',\
> -                  'Replication does not support replicating from an anonymous instance',\
> -                  nil, 10)
> -test_run:cmd('switch replica_anon2')
> -a = box.info.vclock[1]
> --- The instance did fetch a snapshot.
> -a > 0
> --- 0-th vclock component isn't propagated across the cluster.
> -box.info.vclock[0]
> -test_run:cmd('switch default')
> -box.space.test:insert{2}
> -test_run:cmd("switch replica_anon2")
> --- Second replica doesn't follow master through the
> --- 1st one. Replication from an anonymous instance
> --- is forbidden indeed.
> -box.info.vclock[1] == a or box.info.vclock[1]
> -
>  test_run:cmd('switch replica_anon')
> -
> -test_run:cmd('stop server replica_anon2')
> -test_run:cmd('delete server replica_anon2')
> -
>  -- Promote anonymous replica.
>  box.cfg{replication_anon=false}
>  -- Cannot switch back after becoming "normal".
> @@ -137,3 +110,64 @@ box.cfg{replication_anon = false}
>  test_run:switch('default')
>  test_run:cmd('stop server replica_anon')
>  test_run:cmd('delete server replica_anon')
> +
> +--
> +-- gh-4696. Following an anonymous replica.
> +--
> +box.schema.user.grant('guest', 'replication')
> +
> +test_run:cmd([[create server replica_anon1 with rpl_master=default,\
> +             script="replication/anon1.lua"]])
> +test_run:cmd([[create server replica_anon2 with rpl_master=replica_anon1,\
> +             script="replication/anon2.lua"]])
> +test_run:cmd('start server replica_anon1')
> +test_run:cmd('start server replica_anon2')
> +_ =  box.schema.space.create('test')
> +_ = box.space.test:create_index('pk')
> +box.space.test:insert{1}
> +
> +-- Check that master's changes are propagated to replica2,
> +-- following an anonymous replica1.
> +test_run:cmd('switch replica_anon2')
> +box.space.test:select{}
> +
> +test_run:cmd('switch default')
> +test_run:cmd('stop server replica_anon2')
> +test_run:cmd('delete server replica_anon2')
> +
> +-- Check that joining to an anonymous replica is prohibited.
> +test_run:cmd([[create server replica with rpl_master=replica_anon1,\
> +             script="replication/replica.lua"]])
> +test_run:cmd('start server replica with crash_expected=True')
> +test_run:cmd('delete server replica')
> +
> +-- A normal instance (already joined) can follow an anonymous
> +-- replica.
> +test_run:cmd([[create server replica with rpl_master=default,\
> +             script="replication/replica.lua"]])
> +test_run:cmd('start  server replica')
> +test_run:cmd('switch replica')
> +test_run:wait_upstream(1, {status='follow'})
> +box.info.id
> +test_run:cmd('set variable repl_source to "replica_anon1.listen"')
> +box.cfg{replication=repl_source}
> +
> +test_run:cmd('switch default')
> +box.space.test:insert{2}
> +
> +test_run:cmd('switch replica_anon1')
> +-- Check that the new replica follows an anonymous one.
> +test_run:wait_downstream(2, {status='follow'})
> +
> +test_run:cmd('switch replica')
> +box.space.test:select{}
> +
> +test_run:cmd('switch default')
> +
> +-- Cleanup.
> +test_run:cmd('stop server replica')
> +test_run:cmd('stop server replica_anon1')
> +test_run:cmd('delete server replica')
> +test_run:cmd('delete server replica_anon1')
> +box.space.test:drop()
> +box.schema.user.revoke('guest', 'replication')
> 

Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance

[Kirill Yukhin]

Hello,

On 20 апр 16:11, Serge Petrenko wrote:
> Since the anonymous replica implementation, it was forbidden to
> replicate (join/subscribe/register) from anonymous instances.
> Actually, only joining and register should be banned. Anonymous replica
> isn't able to register its peer in _cluster  anyway.
> Allow subscribing to anonymous instances and  remove all the errors
> mentioning unsupported replication from anonymous replica. Join and
> register are covered by ER_READONLY errors, since anonymous replicas
> must be read-only.
> 
> Closes #4696
> ---
> https://github.com/tarantool/tarantool/tree/sp/gh-4696-anon-replica-follow
> https://github.com/tarantool/tarantool/issues/4696
> 
> There's one open question: should we allow 'normal' instances to follow
> anonymous instances? The current patch implementation allows this.
> I see one possible use for allowing normal instances to follow anonymous ones:
> in case such an anonymous instance can be failed over to with its transition to
> master. Then it would be better if all the cluster members followed it in
> advance, to instantly start receiving changes from it when it becomes 'normal'
> and read-write.
> 
> @ChangeLog:
>   - Allow anonymous replicas to be followed by other ones (gh-4696).

I've checked your patch into master.

--
Regards, Kirill Yukhin