Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH] replication: move cluster id match check to replica
@ 2019-01-31 13:25 Serge Petrenko
  2019-01-31 13:30 ` Serge Petrenko
  2019-02-06 10:48 ` Vladimir Davydov
  0 siblings, 2 replies; 5+ messages in thread
From: Serge Petrenko @ 2019-01-31 13:25 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches, georgy, Serge Petrenko

On replica subscribe master checks that replica's cluster id matches
master's one, and disallows replication in case of mismatch.
This behaviour blocks impplementation of anonymous replicas, which
shouldn't pollute _cluster space and could accumulate changes from
multiple clusters at once.
So let's move the check to replica to let it decide which action to take
in case of mismatch.

Needed for #3186
Closes #3704
---
Issue: https://github.com/tarantool/tarantool/issues/3704
Branch: https://github.com/tarantool/tarantool/tree/sp/gh-3704-cluster-id-check-on-replica

 src/box/applier.cc             | 24 +++++++++++-
 src/box/box.cc                 | 22 +++++------
 src/box/xrow.c                 | 26 +++++++++++++
 src/box/xrow.h                 | 51 ++++++++++++++++++++++++
 test/replication/misc.result   | 71 ++++++++++++++++++++++++++++++++++
 test/replication/misc.test.lua | 26 +++++++++++++
 6 files changed, 206 insertions(+), 14 deletions(-)

diff --git a/src/box/applier.cc b/src/box/applier.cc
index 21d2e6bcb..704ed6c5b 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -391,6 +391,7 @@ applier_subscribe(struct applier *applier)
 	struct ibuf *ibuf = &applier->ibuf;
 	struct xrow_header row;
 	struct vclock remote_vclock_at_subscribe;
+	struct tt_uuid cluster_id = uuid_nil;
 
 	xrow_encode_subscribe_xc(&row, &REPLICASET_UUID, &INSTANCE_UUID,
 				 &replicaset.vclock);
@@ -408,9 +409,30 @@ applier_subscribe(struct applier *applier)
 		/*
 		 * In case of successful subscribe, the server
 		 * responds with its current vclock.
+		 *
+		 * Tarantool 2.1.1 also sends its cluster id to
+		 * the replica, and replica has to check whether
+		 * its and master's cluster ids match.
 		 */
 		vclock_create(&remote_vclock_at_subscribe);
-		xrow_decode_vclock_xc(&row, &remote_vclock_at_subscribe);
+		if (applier->version_id >= version_id(2, 1, 1)) {
+			xrow_decode_subscribe_response_xc(&row,
+							  &cluster_id,
+							  &remote_vclock_at_subscribe);
+			/*
+			 * If master didn't send us its cluster id
+			 * assume that it has done all the checks.
+			 * In this case cluster_id will remain zero.
+			 */
+			if (!tt_uuid_is_equal(&cluster_id, &uuid_nil) &&
+			    !tt_uuid_is_equal(&cluster_id, &REPLICASET_UUID)) {
+				tnt_raise(ClientError, ER_REPLICASET_UUID_MISMATCH,
+					  tt_uuid_str(&cluster_id),
+					  tt_uuid_str(&REPLICASET_UUID));
+			}
+		} else {
+			xrow_decode_vclock_xc(&row, &remote_vclock_at_subscribe);
+		}
 	}
 	/*
 	 * Tarantool < 1.6.7:
diff --git a/src/box/box.cc b/src/box/box.cc
index 8892d0f0e..8a8c08167 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1584,18 +1584,6 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
 	/* Check permissions */
 	access_check_universe_xc(PRIV_R);
 
-	/**
-	 * Check that the given UUID matches the UUID of the
-	 * replica set this replica belongs to. Used to handshake
-	 * replica connect, and refuse a connection from a replica
-	 * which belongs to a different replica set.
-	 */
-	if (!tt_uuid_is_equal(&replicaset_uuid, &REPLICASET_UUID)) {
-		tnt_raise(ClientError, ER_REPLICASET_UUID_MISMATCH,
-			  tt_uuid_str(&REPLICASET_UUID),
-			  tt_uuid_str(&replicaset_uuid));
-	}
-
 	/* Check replica uuid */
 	struct replica *replica = replica_by_uuid(&replica_uuid);
 	if (replica == NULL || replica->id == REPLICA_ID_NIL) {
@@ -1620,9 +1608,17 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
 	 * Send a response to SUBSCRIBE request, tell
 	 * the replica how many rows we have in stock for it,
 	 * and identify ourselves with our own replica id.
+	 *
+	 * Since Tarantool version 2.1.1 master doesn't check
+	 * that replica and master have the same cluster id.
+	 * It sends its cluster id to replica instead, and replica
+	 * checks that its cluster id matches master's one.
+	 * Older versions will just ignore the additional field.
 	 */
 	struct xrow_header row;
-	xrow_encode_vclock_xc(&row, &replicaset.vclock);
+	xrow_encode_subscribe_response_xc(&row,
+					  &REPLICASET_UUID,
+					  &replicaset.vclock);
 	/*
 	 * Identify the message with the replica id of this
 	 * instance, this is the only way for a replica to find
diff --git a/src/box/xrow.c b/src/box/xrow.c
index c4e3073be..84b3473b1 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -1170,6 +1170,32 @@ xrow_encode_vclock(struct xrow_header *row, const struct vclock *vclock)
 	return 0;
 }
 
+int
+xrow_encode_subscribe_response(struct xrow_header *row,
+			       const struct tt_uuid *replicaset_uuid,
+			       const struct vclock *vclock)
+{
+	memset(row, 0, sizeof(*row));
+	size_t size = XROW_BODY_LEN_MAX + UUID_STR_LEN + mp_sizeof_vclock(vclock);
+	char *buf = (char *) region_alloc(&fiber()->gc, size);
+	if (buf == NULL) {
+		diag_set(OutOfMemory, size, "region_alloc", "buf");
+		return -1;
+	}
+	char *data = buf;
+	data = mp_encode_map(data, 2);
+	data = mp_encode_uint(data, IPROTO_VCLOCK);
+	data = mp_encode_vclock(data, vclock);
+	data = mp_encode_uint(data, IPROTO_CLUSTER_UUID);
+	data = xrow_encode_uuid(data, replicaset_uuid);
+	assert(data <= buf + size);
+	row->body[0].iov_base = buf;
+	row->body[0].iov_len = (data - buf);
+	row->bodycnt = 1;
+	row->type = IPROTO_OK;
+	return 0;
+}
+
 void
 xrow_encode_timestamp(struct xrow_header *row, uint32_t replica_id, double tm)
 {
diff --git a/src/box/xrow.h b/src/box/xrow.h
index 2654e35e6..719add4f0 100644
--- a/src/box/xrow.h
+++ b/src/box/xrow.h
@@ -348,6 +348,37 @@ xrow_decode_vclock(struct xrow_header *row, struct vclock *vclock)
 	return xrow_decode_subscribe(row, NULL, NULL, vclock, NULL);
 }
 
+/**
+ * Encode a response to subscribe request.
+ * @param row[out] Row to encode into.
+ * @param replicaset_uuid.
+ * @param vclock.
+ *
+ * @retval 0 Success.
+ * @retval -1 Memory error.
+ */
+int
+xrow_encode_subscribe_response(struct xrow_header *row,
+			      const struct tt_uuid *replicaset_uuid,
+			      const struct vclock *vclock);
+
+/**
+ * Decode a response to subscribe request.
+ * @param row Row to decode.
+ * @param[out] replicaset_uuid.
+ * @param[out] vclock.
+ *
+ * @retval 0 Success.
+ * @retval -1 Memory or format error.
+ */
+static inline int
+xrow_decode_subscribe_response(struct xrow_header *row,
+			       struct tt_uuid *replicaset_uuid,
+			       struct vclock *vclock)
+{
+	return xrow_decode_subscribe(row, replicaset_uuid, NULL, vclock, NULL);
+}
+
 /**
  * Encode a heartbeat message.
  * @param row[out] Row to encode into.
@@ -759,6 +790,26 @@ xrow_decode_vclock_xc(struct xrow_header *row, struct vclock *vclock)
 		diag_raise();
 }
 
+/** @copydoc xrow_encode_subscribe_response. */
+static inline void
+xrow_encode_subscribe_response_xc(struct xrow_header *row,
+				  const struct tt_uuid *replicaset_uuid,
+				  const struct vclock *vclock)
+{
+	if (xrow_encode_subscribe_response(row, replicaset_uuid, vclock) != 0)
+		diag_raise();
+}
+
+/** @copydoc xrow_decode_subscribe_response. */
+static inline void
+xrow_decode_subscribe_response_xc(struct xrow_header *row,
+				  struct tt_uuid *replicaset_uuid,
+				  struct vclock *vclock)
+{
+	if (xrow_decode_subscribe_response(row, replicaset_uuid, vclock) != 0)
+		diag_raise();
+}
+
 /** @copydoc iproto_reply_ok. */
 static inline void
 iproto_reply_ok_xc(struct obuf *out, uint64_t sync, uint32_t schema_version)
diff --git a/test/replication/misc.result b/test/replication/misc.result
index c32681a7a..1f7e78a2f 100644
--- a/test/replication/misc.result
+++ b/test/replication/misc.result
@@ -604,3 +604,74 @@ test_run:cmd("delete server replica")
 test_run:cleanup_cluster()
 ---
 ...
+--
+-- gh-3704 move cluster id check to replica
+--
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+---
+- true
+...
+box.schema.user.grant("guest", "replication")
+---
+...
+test_run:cmd("start server replica")
+---
+- true
+...
+test_run:grep_log("replica", "REPLICASET_UUID_MISMATCH")
+---
+- null
+...
+box.info.replication[2].downstream ~= nil
+---
+- true
+...
+-- make master generate another cluster uuid and check that
+-- replica doesn't connect
+test_run:cmd("stop server replica")
+---
+- true
+...
+test_run:cmd("restart server default with cleanup=1")
+box.schema.user.grant("guest", "replication")
+---
+...
+test_run:cmd("start server replica")
+---
+- true
+...
+-- master believes replica is in cluster, but their UUIDS differ.
+replica_uuid = test_run:eval("replica", "box.info.uuid")[1]
+---
+...
+_ = box.space._cluster:insert{2, replica_uuid}
+---
+...
+test_run:cmd("restart server replica")
+---
+- true
+...
+test_run:grep_log("replica", "REPLICASET_UUID_MISMATCH")
+---
+- REPLICASET_UUID_MISMATCH
+...
+box.info.replication[2].downstream ~= nil
+---
+- false
+...
+test_run:cmd("restart server default with cleanup=1")
+---
+- true
+...
+test_run:cmd("stop server replica")
+---
+- true
+...
+test_run:cmd("cleanup server replica")
+---
+- true
+...
+test_run:cmd("delete server replica")
+---
+- true
+...
diff --git a/test/replication/misc.test.lua b/test/replication/misc.test.lua
index 6a8af05c3..967c4e912 100644
--- a/test/replication/misc.test.lua
+++ b/test/replication/misc.test.lua
@@ -243,3 +243,29 @@ test_run:cmd("stop server replica")
 test_run:cmd("cleanup server replica")
 test_run:cmd("delete server replica")
 test_run:cleanup_cluster()
+
+--
+-- gh-3704 move cluster id check to replica
+--
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+box.schema.user.grant("guest", "replication")
+test_run:cmd("start server replica")
+test_run:grep_log("replica", "REPLICASET_UUID_MISMATCH")
+box.info.replication[2].downstream ~= nil
+-- make master generate another cluster uuid and check that
+-- replica doesn't connect
+test_run:cmd("stop server replica")
+test_run:cmd("restart server default with cleanup=1")
+box.schema.user.grant("guest", "replication")
+test_run:cmd("start server replica")
+-- master believes replica is in cluster, but their UUIDS differ.
+replica_uuid = test_run:eval("replica", "box.info.uuid")[1]
+_ = box.space._cluster:insert{2, replica_uuid}
+test_run:cmd("restart server replica")
+test_run:grep_log("replica", "REPLICASET_UUID_MISMATCH")
+box.info.replication[2].downstream ~= nil
+
+test_run:cmd("restart server default with cleanup=1")
+test_run:cmd("stop server replica")
+test_run:cmd("cleanup server replica")
+test_run:cmd("delete server replica")
-- 
2.17.2 (Apple Git-113)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-02-07 10:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-31 13:25 [PATCH] replication: move cluster id match check to replica Serge Petrenko
2019-01-31 13:30 ` Serge Petrenko
2019-02-06 10:48 ` Vladimir Davydov
2019-02-06 14:51   ` Serge Petrenko
2019-02-07 10:14     ` Vladimir Davydov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox