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

Serge Petrenko sergepetrenko at tarantool.org
Mon Apr 20 16:11:15 MSK 2020


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)



More information about the Tarantool-patches mailing list