Re: [Tarantool-patches] [DRAFT v2] replication: track information about replica

[Cyrill Gorcunov <gorcunov@gmail.com>]

On Thu, Jul 02, 2020 at 11:21:41PM +0300, Sergey Kaplun wrote:
> This is a draft for the patch.
> The patch allows to track information about changing relay state. At
> every change of relay state timestamp, vclock, new state (and error
> message if exists) will be saved at _cluster space.
> 
> The patch adds trigger list at relay, that is invoked when relay changes
> its state.  The trigger that updates _cluster space is setted when a
> replica is registered.
> ---
> 
> This is a draft for the patch. Nevertheless I would like to hear as much
> criticism as possible. Also it's important for me to hear @kostja's
> opinion here.
> 
> Originaly the task splits into the parts [1]:
> 
> 1) Pass extra info from downstreams (box.info.listen) to be saved on
> master. Persist it on master.
> 2) Persist last status change with its timestamp and vclock on master
> for each downstream.
> 
> This patch is the second part.
> 
> As we've discussed with Alexander Turenko, there are several ways how
> the first part can be implemented:
> 
> 1) Use additional bytes inside greeting to transport information to
> master. The bad thing is that greeting is about authorization, not the
> additional information.
> 
> 2) Add a new type of the request (eg IPROTO_INFO). An instance receiving
> such a request should provide information about itself. So far, this is
> only one field yet. Is it reasonable to add new protocol fields for one
> feature?
> 
> Thoughts?
> 
> [1]: https://github.com/tarantool/tarantool/issues/3363#issuecomment-622382549
> 
> Branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-3363-track-replica-status-change
> Issue: https://github.com/tarantool/tarantool/issues/3363

Sergey, here is some update on top of this patch I would make.
Look I'm still thinking about the code architecture in general
so please give me some more time. Still just to share early
(completely untested of course :)
---
 src/box/alter.cc     |   51 +++++++++++++++++++++++++++++++--------------------
 src/box/box.cc       |    6 +++---
 src/box/relay.cc     |   17 +++++++----------
 src/box/schema_def.h |    5 +++++
 4 files changed, 46 insertions(+), 33 deletions(-)

Index: tarantool.git/src/box/alter.cc
===================================================================
--- tarantool.git.orig/src/box/alter.cc
+++ tarantool.git/src/box/alter.cc
@@ -4172,51 +4172,62 @@ relay_on_state_change(struct trigger *tr
 {
 	struct relay *relay = (struct relay *)event;
 	(void)trigger;
+
 	if (relay_get_state(relay) == RELAY_OFF)
 		return 0;
+
 	struct replica *replica = relay_replica(relay);
 	const struct tt_uuid *uuid = &replica->uuid;
+
 	assert(replica_by_uuid(uuid) != NULL);
 	assert(replica->id != REPLICA_ID_NIL);
+
 	struct credentials *orig_credentials = effective_user();
 	fiber_set_user(fiber(), &admin_credentials);
-	int rc;
-	if ((rc = boxk(IPROTO_UPDATE, BOX_CLUSTER_ID, "[%u]["
-		 "[%s%u%lf]" /* last row time */
-		 "[%s%u%s]" /* vclock */
-		 "[%s%u%s]" /* relay state */
-		 "]",
-		 (unsigned) replica->id,
-		 "=", 3, relay_last_row_time(relay),
-		 "=", 4, vclock_to_string(relay_vclock(relay)),
-		 "=", 5, relay_get_state_str(relay)
-		)) != 0) {
+
+	const char *vclock_str = vclock_to_string(relay_vclock(relay));
+	const char *state_str = relay_get_state_str(relay);
+	double timestamp = relay_last_row_time(relay);
+
+	int rc = boxk(IPROTO_UPDATE, BOX_CLUSTER_ID, "[%u]["
+		      "[%s%u%lf]"
+		      "[%s%u%s]"
+		      "[%s%u%s]"
+		      "]",
+		      (unsigned) replica->id,
+		      "=", BOX_CLUSTER_FIELD_LAST_ROW_TIME, timestamp,
+		      "=", BOX_CLUSTER_FIELD_VCLOCK, vclock_str,
+		      "=", BOX_CLUSTER_FIELD_RELAY_STATE, state_str);
+	if (rc != 0)
 		goto restore_cred;
-	}
+
+
 	switch (relay_get_state(relay)) {
 	case RELAY_STOPPED: {
-		struct error *e =
-			diag_last_error(relay_get_diag(relay));
-		if (e != NULL)
+		struct *diag = relay_get_diag(relay);
+		struct error *e = diag_last_error(diag);
+		if (e != NULL) {
 			rc = boxk(IPROTO_UPDATE, BOX_CLUSTER_ID,
 				 "[%u][[%s%u%s]]",
 				 (unsigned) replica->id,
-				 "=", 6, e->errmsg);
+				 "=", BOX_CLUSTER_FIELD_RELAY_ERR,
+				 e->errmsg);
+		}
 		break;
 	}
 	case RELAY_FOLLOW:
 		rc = boxk(IPROTO_UPDATE, BOX_CLUSTER_ID,
 			 "[%u][[%s%uNIL]]",
-			 (unsigned) replica->id, "=", 6);
+			 (unsigned) replica->id,
+			 "=", BOX_CLUSTER_FIELD_RELAY_ERR);
 		break;
 	default:
 		unreachable();
 	}
+
 restore_cred:
 	fiber_set_user(fiber(), orig_credentials);
-	if (rc != 0)
-		return -1;
-	return 0;
+	return rc;
 }
 
 static inline void
Index: tarantool.git/src/box/box.cc
===================================================================
--- tarantool.git.orig/src/box/box.cc
+++ tarantool.git/src/box/box.cc
@@ -1476,8 +1476,8 @@ box_session_push(const char *data, const
 	return rc;
 }
 
-static inline void
-box_register_replica(uint32_t id, const struct tt_uuid *uuid)
+static void
+box_register_replica_xc(uint32_t id, const struct tt_uuid *uuid)
 {
 	if (boxk(IPROTO_INSERT, BOX_CLUSTER_ID, "["
 		 "%u" /* replica id */
@@ -1524,7 +1524,7 @@ box_on_join(const tt_uuid *instance_uuid
 			break;
 		replica_id++;
 	}
-	box_register_replica(replica_id, instance_uuid);
+	box_register_replica_xc(replica_id, instance_uuid);
 }
 
 void
Index: tarantool.git/src/box/relay.cc
===================================================================
--- tarantool.git.orig/src/box/relay.cc
+++ tarantool.git/src/box/relay.cc
@@ -153,16 +153,13 @@ struct relay {
 const char *
 relay_get_state_str(const struct relay *relay)
 {
-	switch(relay->state) {
-	case RELAY_OFF:
-		return "off";
-	case RELAY_FOLLOW:
-		return "follow";
-	case RELAY_STOPPED:
-		return "stopped";
-	default:
-		return "<unknown relay state>";
-	}
+	static const char *st[] = {
+		[RELAY_OFF]	= "off",
+		[RELAY_FOLLOW]	= "follow",
+		[RELAY_STOPPED]	= "stopped",
+	};
+	return relay->state < lengthof(st) ?
+		st[relay->state] : "unknown";
 }
 
 static inline void
Index: tarantool.git/src/box/schema_def.h
===================================================================
--- tarantool.git.orig/src/box/schema_def.h
+++ tarantool.git/src/box/schema_def.h
@@ -206,6 +206,11 @@ enum {
 enum {
 	BOX_CLUSTER_FIELD_ID = 0,
 	BOX_CLUSTER_FIELD_UUID = 1,
+	BOX_CLUSTER_FIELD_HOST_PORT = 2,
+	BOX_CLUSTER_FIELD_RELAY_TIMESTAMP = 3,
+	BOX_CLUSTER_FIELD_RELAY_VCLOCK = 4,
+	BOX_CLUSTER_FIELD_RELAY_STATE = 5,
+	BOX_CLUSTER_FIELD_RELAY_ERR = 6,
 };
 
 /** _truncate fields. */