From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 983024696C3 for ; Tue, 28 Apr 2020 23:55:22 +0300 (MSK) References: <20200420131115.42047-1-sergepetrenko@tarantool.org> From: Vladislav Shpilevoy Message-ID: <52ac079d-3439-a190-c418-52212a83683c@tarantool.org> Date: Tue, 28 Apr 2020 22:55:20 +0200 MIME-Version: 1.0 In-Reply-To: <20200420131115.42047-1-sergepetrenko@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Petrenko , gorcunov@gmail.com Cc: tarantool-patches@dev.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.