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

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

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.