Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Serge Petrenko <sergepetrenko@tarantool.org>,
	tarantool-patches@dev.tarantool.org, gorcunov@gmail.com
Subject: Re: [Tarantool-patches] [PATCH 1/1] replication: check rs uuid on subscribe process
Date: Wed, 2 Jun 2021 22:16:11 +0200	[thread overview]
Message-ID: <5b62582e-20f1-21a3-76d8-993318aadedd@tarantool.org> (raw)
In-Reply-To: <a3fc09d0-fb6f-8d96-1e47-c5e83a85f573@tarantool.org>

Hi! Good job on the review!

>>>> The UUID ignorance on subscribe decode was introduced here:
>>>> https://github.com/tarantool/tarantool/commit/7f8cbde3555084ad6c41f137aec4faba4648c705#diff-fc276b44b551b4eac3431c9433d4bc881790ddd7df76226d7579f80da7798f6e
>>>>
>>>> And I don't understand why. Maybe I miss something? The tests have
>>>> passed. Sergey, do you remember why was it needed really?
>>>>
>>>> Replicaset UUID mismatch definitely means the node can't connect.
>>>> It is not related to whether it is anonymous or not. Because it
>>>> has nothing to do with _cluster.
>>> Hi! Thanks for the patch!
>>>
>>> That change was meant to help anonymous replicas fetch data from
>>> multiple clusters. There was such an idea back then.
>>> It never got implemented though, and I doubt it will.
>> Wow, I am not sure how it is supposed to work at all. Starting from
>> the problem that in different RS you have conflicting replica_id, and
>> you won't be able to keep track of the changes properly. You simply can't
>> pack the same replica_id from 2 different replicasets into one replica_id
>> in your own journal.
>>
>> It does not matter if you are anon or not - the changes in the given
>> replicasets are generated by non-anon nodes and their replica IDs will
>> clash.
> 
> I guess you could assign the same replica_id to all the changes coming from
> one cluster. Just assign lsns in the order of arrival, or something.

How will you resubscribe then to get the changes from one RS starting from
a certain vclock? The same for the idea below.

When you resubscribe, you need to tell the other RS starting from which
vclock it should send data. If you changed the received replica IDs, then
you don't have the needed information persisted anywhere.

Absence of a resubscribe means a rejoin on each restart and disconnect,
which makes it hardly usable.

> Sorry, one more nit. I failed to notice this earlier, but there's an obsolete
> comment in box_process_subscribe().
> 
> It goes like
> "Tarantool > 2.1.1 master doesn't check that replica has
> the same cluster uuid".
> 
> LGTM, once you delete the comment.

Thanks for noticing. I didn't delete it, but applied the diff below,
pushed to master, 2.8, 2.7.

====================
@@ -2786,11 +2786,13 @@ box_process_subscribe(struct ev_io *io, struct xrow_header *header)
 	 * the replica how many rows we have in stock for it,
 	 * and identify ourselves with our own replica id.
 	 *
-	 * Tarantool > 2.1.1 master doesn't check that replica
-	 * has the same cluster id. Instead it sends its cluster
-	 * id to replica, and replica checks that its cluster id
-	 * matches master's one. Older versions will just ignore
-	 * the additional field.
+	 * Master not only checks the replica has the same replicaset UUID, but
+	 * also sends the UUID to the replica so both Tarantools could perform
+	 * any checks they want depending on their version and supported
+	 * features.
+	 *
+	 * Older versions not supporting replicaset UUID in the response will
+	 * just ignore the additional field (these are < 2.1.1).
 	 */
 	struct xrow_header row;
 	xrow_encode_subscribe_response_xc(&row, &REPLICASET_UUID, &vclock);

  reply	other threads:[~2021-06-02 20:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-28 20:35 Vladislav Shpilevoy via Tarantool-patches
2021-05-28 22:06 ` Cyrill Gorcunov via Tarantool-patches
2021-05-28 22:20   ` Vladislav Shpilevoy via Tarantool-patches
2021-05-28 22:30     ` Cyrill Gorcunov via Tarantool-patches
2021-06-01  7:52     ` Cyrill Gorcunov via Tarantool-patches
2021-06-01  8:29 ` Serge Petrenko via Tarantool-patches
2021-06-01 21:34   ` Vladislav Shpilevoy via Tarantool-patches
2021-06-02  6:36     ` Serge Petrenko via Tarantool-patches
2021-06-02 20:16       ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-06-03  7:14         ` Serge Petrenko via Tarantool-patches
2021-06-03  7:18         ` Serge Petrenko via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5b62582e-20f1-21a3-76d8-993318aadedd@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergepetrenko@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 1/1] replication: check rs uuid on subscribe process' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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