[Tarantool-patches] [PATCH] replication: remove unnecessary errors on replicating from an anonymous instance
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sun May 10 22:43:13 MSK 2020
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')
>
More information about the Tarantool-patches
mailing list