Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, sergepetrenko@tarantool.org,
	gorcunov@gmail.com
Subject: [Tarantool-patches] [PATCH v2 05/11] [wip] box: do not register outgoing connections
Date: Thu, 10 Sep 2020 01:16:56 +0200	[thread overview]
Message-ID: <86b2104fd1dd49ed4bb7e432cf47cf9660ef35b3.1599693319.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1599693319.git.v.shpilevoy@tarantool.org>

Replication protocol's first stage for non-anonymous replicas is
that the replica should be registered in _cluster to get a unique
ID number.

That happens, when replica connects to a writable node, which
performs the registration. So it means, registration always
happens on the master node when appears an incoming request for
it. When a relay is created.

That wasn't the case for bootstrap. If box.cfg.replication wasn't
empty on the master node doing the cluster bootstrap, it
registered all the outgoing connections in _cluster. Note, the
target node could be even anonymous, but still was registered.
Also the registration happened for the remote replicas even before
their bootstrap.

That breaks the protocol, and leads to registration of anon
replicas sometimes. The patch drops it.

The main motivation here though is Raft cluster bootstrap
specifics. During Raft bootstrap it is going to be very important
that non-bootstrapped nodes should not be registered in _cluster.
It would break the leader election during bootstrap.

Closes #5287
---
The patch fixes 5287, but now the same test leads to a crash. Because in the
code there is no handling for the case when a not anon replica becomes anon.

That happens, when a master connects to a replica before it is bootstrapped, the
replica allows it, and then after the replica is boostrapped, it sends SUBSCRIBE
right away. Then the master crashes in relay_subscribe() in the first line,
because the replica was connected as not anon (replica->anon == false), but it
does not have an ID (replica->id == REPLICA_ID_NIL).

I am not sure how to fix it now. Decided to think more about it, and see what
reviewers think. In the current state the fix is enough to unblock Raft, so it
is not urgent.

 src/box/box.cc | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index eeb00d5e2..3214ec340 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -2217,15 +2217,6 @@ bootstrap_master(const struct tt_uuid *replicaset_uuid)
 	box_register_replica(replica_id, &INSTANCE_UUID);
 	assert(replica_by_uuid(&INSTANCE_UUID)->id == 1);
 
-	/* Register other cluster members */
-	replicaset_foreach(replica) {
-		if (tt_uuid_is_equal(&replica->uuid, &INSTANCE_UUID))
-			continue;
-		assert(replica->applier != NULL);
-		box_register_replica(++replica_id, &replica->uuid);
-		assert(replica->id == replica_id);
-	}
-
 	/* Set UUID of a new replica set */
 	box_set_replicaset_uuid(replicaset_uuid);
 
-- 
2.21.1 (Apple Git-122.3)

  parent reply	other threads:[~2020-09-09 23:17 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-09 23:16 [Tarantool-patches] [PATCH v2 00/11] dRaft Vladislav Shpilevoy
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 01/11] applier: store instance_id in struct applier Vladislav Shpilevoy
2020-09-14  9:38   ` Serge Petrenko
2020-09-19 15:44   ` Vladislav Shpilevoy
2020-09-21  6:23     ` Serge Petrenko
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 10/11] raft: introduce box.info.raft Vladislav Shpilevoy
2020-09-14  9:42   ` Serge Petrenko
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 11/11] [tosquash] raft: a swarm of minor fixes Vladislav Shpilevoy
2020-09-14 10:13   ` Serge Petrenko
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 02/11] box: introduce summary RO flag Vladislav Shpilevoy
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 03/11] wal: don't touch box.cfg.wal_dir more than once Vladislav Shpilevoy
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 04/11] replication: track registered replica count Vladislav Shpilevoy
2020-09-09 23:16 ` Vladislav Shpilevoy [this message]
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 06/11] raft: introduce persistent raft state Vladislav Shpilevoy
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 07/11] raft: introduce box.cfg.raft_* options Vladislav Shpilevoy
2020-09-09 23:16 ` [Tarantool-patches] [PATCH v2 08/11] raft: relay status updates to followers Vladislav Shpilevoy
2020-09-20 17:17   ` Vladislav Shpilevoy
2020-09-21  7:13     ` Serge Petrenko
2020-09-21 10:50   ` Serge Petrenko
2020-09-21 22:47     ` Vladislav Shpilevoy
2020-09-22  8:48       ` Serge Petrenko
2020-09-21 22:47   ` Vladislav Shpilevoy
2020-09-22  8:47     ` Serge Petrenko
2020-09-09 23:17 ` [Tarantool-patches] [PATCH v2 09/11] raft: introduce state machine Vladislav Shpilevoy
2020-09-19 15:49   ` Vladislav Shpilevoy
2020-09-19 15:50     ` Vladislav Shpilevoy
2020-09-21  8:20       ` Serge Petrenko
2020-09-21  8:22     ` Serge Petrenko
2020-09-21  8:34       ` Serge Petrenko
2020-09-21 22:47     ` Vladislav Shpilevoy
2020-09-22  8:49       ` Serge Petrenko
2020-09-22 22:48   ` Vladislav Shpilevoy
2020-09-23  9:59     ` Serge Petrenko
2020-09-23 20:31       ` Vladislav Shpilevoy
2020-09-24  9:34         ` Serge Petrenko
2020-09-19 15:58 ` [Tarantool-patches] [PATCH v2 12/11] dRaft Vladislav Shpilevoy
2020-09-19 15:59   ` Vladislav Shpilevoy
2020-09-21  7:24     ` Serge Petrenko
2020-09-21 22:48   ` [Tarantool-patches] [PATCH v2 12/11] raft: add tests Vladislav Shpilevoy
2020-09-30 10:56 ` [Tarantool-patches] [PATCH v2 00/11] dRaft Kirill Yukhin

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=86b2104fd1dd49ed4bb7e432cf47cf9660ef35b3.1599693319.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergepetrenko@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 05/11] [wip] box: do not register outgoing connections' \
    /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