[PATCH] replication: move cluster id match check to replica
Serge Petrenko
sergepetrenko at tarantool.org
Thu Jan 31 16:30:50 MSK 2019
Sorry, the mailing list didn’t forward it for some reason
> 31 янв. 2019 г., в 16:25, Serge Petrenko <sergepetrenko at tarantool.org> написал(а):
>
> 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)
>
More information about the Tarantool-patches
mailing list