[Tarantool-patches] [PATCH 1/4] replication: replace anon flag with enum

[Vladislav Shpilevoy]

Struct replica had a flag whether it is anonymous. But a flag is
not enough. When a remote instance didn't tell whether it is
anonymous, the flag can't be true and false, it should be
something third until the replica sends its identity flag.

Not knowing whether a replica is actually non-anonymous, or just
was added with a 'false' anon flag locally due to being in
box.cfg.replication, leads to not being able to update the flag
properly, when the remote instance sends it.

For example, when a subscribe request is received with a flag
saying the remote instance is anonymous, and local struct replica
object stores a false anon flag, it is not possible to say,
whether it is false because the replica was added by
box.cfg.replication, or because it really isn't anonymous. And it
is not possible to update the flag.

In turn, not being able to update the anon flag may lead to an
issue, when the replica is considered non-anonymous even though it
is, and it keeps a garbage collection consumer, preventing WAL
files deletion.

This patch replaces the flag with a enum having 3 values, so in a
next patch the issue can be fixed.

Part of #5287
---
 src/box/box.cc         |  2 +-
 src/box/lua/info.c     |  2 +-
 src/box/relay.cc       |  6 +++---
 src/box/replication.cc | 11 ++++++-----
 src/box/replication.h  | 23 +++++++++++++++++++----
 5 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index faffd5769..3f2999fc0 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -955,7 +955,7 @@ box_clear_synchro_queue(void)
 		replicaset_foreach(replica) {
 			if (replica->relay != NULL &&
 			    relay_get_state(replica->relay) != RELAY_OFF &&
-			    !replica->anon) {
+			    replica->id_status != REPLICA_ID_STATUS_ANON) {
 				assert(!tt_uuid_is_equal(&INSTANCE_UUID,
 							 &replica->uuid));
 				vclock = relay_vclock(replica->relay);
diff --git a/src/box/lua/info.c b/src/box/lua/info.c
index 1c131caec..6c28ae721 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -226,7 +226,7 @@ lbox_info_replication_anon_call(struct lua_State *L)
 	lua_setmetatable(L, -2);
 
 	replicaset_foreach(replica) {
-		if (!replica->anon)
+		if (replica->id_status != REPLICA_ID_STATUS_ANON)
 			continue;
 
 		lua_pushstring(L, tt_uuid_str(&replica->uuid));
diff --git a/src/box/relay.cc b/src/box/relay.cc
index a7843a8c2..043ae255f 100644
--- a/src/box/relay.cc
+++ b/src/box/relay.cc
@@ -601,7 +601,7 @@ relay_subscribe_f(va_list ap)
 	 */
 	struct trigger on_close_log;
 	trigger_create(&on_close_log, relay_on_close_log_f, relay, NULL);
-	if (!relay->replica->anon)
+	if (relay->replica->id_status != REPLICA_ID_STATUS_ANON)
 		trigger_add(&r->on_close_log, &on_close_log);
 
 	/* Setup WAL watcher for sending new rows to the replica. */
@@ -709,7 +709,6 @@ relay_subscribe(struct replica *replica, int fd, uint64_t sync,
 		struct vclock *replica_clock, uint32_t replica_version_id,
 		uint32_t replica_id_filter)
 {
-	assert(replica->anon || replica->id != REPLICA_ID_NIL);
 	struct relay *relay = replica->relay;
 	assert(relay->state != RELAY_FOLLOW);
 	/*
@@ -717,7 +716,8 @@ relay_subscribe(struct replica *replica, int fd, uint64_t sync,
 	 * unless it has already been registered by initial
 	 * join.
 	 */
-	if (replica->gc == NULL && !replica->anon) {
+	if (replica->gc == NULL &&
+	    replica->id_status != REPLICA_ID_STATUS_ANON) {
 		replica->gc = gc_consumer_register(replica_clock, "replica %s",
 						   tt_uuid_str(&replica->uuid));
 		if (replica->gc == NULL)
diff --git a/src/box/replication.cc b/src/box/replication.cc
index ef0e2411d..4cd08e6ff 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -173,7 +173,7 @@ replica_new(void)
 		diag_raise();
 	}
 	replica->id = 0;
-	replica->anon = false;
+	replica->id_status = REPLICA_ID_STATUS_UNKNOWN;
 	replica->uuid = uuid_nil;
 	replica->applier = NULL;
 	replica->gc = NULL;
@@ -206,6 +206,7 @@ replicaset_add(uint32_t replica_id, const struct tt_uuid *replica_uuid)
 	assert(replica_by_uuid(replica_uuid) == NULL);
 	struct replica *replica = replica_new();
 	replica->uuid = *replica_uuid;
+	replica->id_status = REPLICA_ID_STATUS_REGISTERED;
 	replica_hash_insert(&replicaset.hash, replica);
 	replica_set_id(replica, replica_id);
 	return replica;
@@ -220,7 +221,7 @@ replicaset_add_anon(const struct tt_uuid *replica_uuid)
 	struct replica *replica = replica_new();
 	replica->uuid = *replica_uuid;
 	replica_hash_insert(&replicaset.hash, replica);
-	replica->anon = true;
+	replica->id_status = REPLICA_ID_STATUS_ANON;
 	replicaset.anon_count++;
 	return replica;
 }
@@ -236,7 +237,7 @@ replica_set_id(struct replica *replica, uint32_t replica_id)
 		/* Assign local replica id */
 		assert(instance_id == REPLICA_ID_NIL);
 		instance_id = replica_id;
-	} else if (replica->anon) {
+	} else if (replica->id_status == REPLICA_ID_STATUS_ANON) {
 		/*
 		 * Set replica gc on its transition from
 		 * anonymous to a normal one.
@@ -250,7 +251,7 @@ replica_set_id(struct replica *replica, uint32_t replica_id)
 
 	say_info("assigned id %d to replica %s",
 		 replica->id, tt_uuid_str(&replica->uuid));
-	replica->anon = false;
+	replica->id_status = REPLICA_ID_STATUS_REGISTERED;
 }
 
 void
@@ -902,7 +903,7 @@ replica_on_relay_stop(struct replica *replica)
 	 * collector then. See also replica_clear_id.
 	 */
 	if (replica->id == REPLICA_ID_NIL) {
-		if (!replica->anon) {
+		if (replica->id_status != REPLICA_ID_STATUS_ANON) {
 			gc_consumer_unregister(replica->gc);
 			replica->gc = NULL;
 		} else {
diff --git a/src/box/replication.h b/src/box/replication.h
index ddc2bddf4..3e34d4544 100644
--- a/src/box/replication.h
+++ b/src/box/replication.h
@@ -269,6 +269,21 @@ struct replicaset {
 };
 extern struct replicaset replicaset;
 
+enum replica_id_status {
+	/**
+	 * Replica is connected, but its ID is unknown, and it didn't report yet
+	 * whether it is anonymous.
+	 */
+	REPLICA_ID_STATUS_UNKNOWN,
+	/**
+	 * Anonymous replica doesn't have an id and isn't present in _cluster
+	 * table.
+	 */
+	REPLICA_ID_STATUS_ANON,
+	/** Normal replica having an ID in _cluster. */
+	REPLICA_ID_STATUS_REGISTERED,
+};
+
 /**
  * Summary information about a replica in the replica set.
  */
@@ -286,11 +301,11 @@ struct replica {
 	 */
 	uint32_t id;
 	/**
-	 * Whether this is an anonymous replica, e.g. a read-only
-	 * replica that doesn't have an id and isn't present in
-	 * _cluster table.
+	 * Current status of the replica identity. It is not a flag, because it
+	 * is the replica's decision whether to tell its identity. And if it
+	 * didn't tell it yet, the status may be unknown.
 	 */
-	bool anon;
+	enum replica_id_status id_status;
 	/** Applier fiber. */
 	struct applier *applier;
 	/** Relay thread. */
-- 
2.21.1 (Apple Git-122.3)