From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 4D8A74696C3 for ; Mon, 20 Apr 2020 16:11:31 +0300 (MSK) From: Serge Petrenko Date: Mon, 20 Apr 2020 16:11:15 +0300 Message-Id: <20200420131115.42047-1-sergepetrenko@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [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: gorcunov@gmail.com, v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org 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)