Tarantool development patches archive
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: Mons Anderson <v.perepelitsa@corp.mail.ru>,
	tml <tarantool-patches@dev.tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v3 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
Date: Tue, 8 Dec 2020 11:02:30 +0300	[thread overview]
Message-ID: <20201208080230.GG2303@grain> (raw)
In-Reply-To: <20201207214825.GF2303@grain>

The whole patch force pushed
---
From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Thu, 3 Dec 2020 12:43:53 +0300
Subject: [PATCH 2/3] cfg: support symbolic evaluation of replication_synchro_quorum

When synchronous replication is used we prefer a user to specify
a quorum number, ie the number of replicas where data must be
replicated before the master node continue accepting new
transactions.

This is not very convenient since a user may not know initially
how many replicas will be used. Moreover the number of replicas
may vary dynamically. For this sake we allow to specify the
number of quorum in a symbolic way.

For example

box.cfg {
	replication_synchro_quorum = "N/2+1",
}

where `N` is a number of registered replicas in a cluster.
Once new replica attached or old one detached the number
is renewed and propagated.

Internally on each replica_set_id() and replica_clear_id(),
ie at moment when replica get registered or unregistered,
we call box_update_replication_synchro_quorum() helper which
finds out if evaluation of replication_synchro_quorum is
needed and if so we calculate new replication_synchro_quorum
value based on number of currently registered replicas. Then
we notify dependent systems such as qsync and raft to update
their guts.

Note: we do *not* change the default settings for this option,
it remains 1 by default for now. Change the default option should
be done as a separate commit once we make sure that everything is
fine.

Closes #5446

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

@TarantoolBot document
Title: Synchronous replication

The plain integer value might be convenient for simple scenarios.
But in case if number of replicas is dynamic a better way should
be to describe a quorum via formula. The formula should use symbol
`N` to represent amount of registered replicas.

For example the canonical definition for a quorum (ie majority
of members in a set) of `N` replicas is `N/2+1`. For such
configuration one can define

```
box.cfg {replication_synchro_quorum = "N/2+1"}
```

Note that for sake of simplicity quorum evaluation never returns
negative values thus for the case of formula say `N-2` the result
will be 1 until number of replicas become 4 and more.
---
 src/box/box.cc           | 121 +++++++++++++++++++++++++++++++++++++--
 src/box/box.h            |   1 +
 src/box/lua/load_cfg.lua |   2 +-
 src/box/replication.cc   |   4 +-
 4 files changed, 121 insertions(+), 7 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index a8bc3471d..2076a8dc3 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -554,10 +554,91 @@ box_check_replication_sync_lag(void)
 	return lag;
 }
 
+/**
+ * Evaluate replication syncro quorum number from a formula.
+ */
+static int
+eval_replication_synchro_quorum(int nr_replicas)
+{
+	const char fmt[] =
+		"local expr = [[%s]]\n"
+		"local f, err = loadstring('return ('..expr..')')\n"
+		"if not f then "
+			"error(string.format('Failed to load \%\%s:"
+			"\%\%s', expr, err)) "
+		"end\n"
+		"setfenv(f, {N = %d, math = {"
+			"ceil = math.ceil,"
+			"floor = math.floor,"
+			"abs = math.abs,"
+			"random = math.random,"
+			"min = math.min,"
+			"max = math.abs,"
+			"sqrt = math.sqrt,"
+			"fmod = math.fmod,"
+		"}})\n"
+		"local res = f()\n"
+		"if type(res) ~= 'number' then\n"
+			"error('Expression should return a number')\n"
+		"end\n"
+		"return math.floor(res)\n";
+	int value = -1;
+	const char *expr = cfg_gets("replication_synchro_quorum");
+	const char *buf = tt_sprintf(fmt, expr, nr_replicas);
+
+	luaL_loadstring(tarantool_L, buf);
+	if (lua_pcall(tarantool_L, 0, 1, 0) != 0) {
+		diag_set(ClientError, ER_CFG,
+			 "replication_synchro_quorum",
+			 lua_tostring(tarantool_L, -1));
+		return -1;
+	}
+
+	if (lua_isnumber(tarantool_L, -1))
+		value = (int)lua_tonumber(tarantool_L, -1);
+	lua_pop(tarantool_L, 1);
+
+	/*
+	 * At least we should have 1 node to sync, thus
+	 * if the formula has evaluated to some negative
+	 * value (say it was n-2) do not treat it as an
+	 * error but just yield a minimum valid magnitude.
+	 */
+	if (value >= VCLOCK_MAX) {
+		const int value_max = VCLOCK_MAX - 1;
+		say_warn("replication_synchro_quorum evaluated "
+			 "to value %d, set to %d",
+			 value, value_max);
+		value = value_max;
+	}
+
+	/*
+	 * We never return 0, even if we're in bootstrap
+	 * stage were number of replicas equals zero we
+	 * should consider the node itself as a minimum
+	 * quorum number.
+	 */
+	return MAX(1, value);
+}
+
 static int
 box_check_replication_synchro_quorum(void)
 {
-	int quorum = cfg_geti("replication_synchro_quorum");
+	int quorum = 0;
+
+	if (!cfg_isnumber("replication_synchro_quorum")) {
+		/*
+		 * The formula uses symbolic name 'N' as
+		 * a number of currently registered replicas.
+		 */
+		int value = replicaset.registered_count;
+		quorum = eval_replication_synchro_quorum(value);
+		if (quorum < 0)
+			return -1;
+	} else {
+		quorum = cfg_geti("replication_synchro_quorum");
+	}
+
 	if (quorum <= 0 || quorum >= VCLOCK_MAX) {
 		diag_set(ClientError, ER_CFG, "replication_synchro_quorum",
 			 "the value must be greater than zero and less than "
@@ -910,15 +991,47 @@ box_set_replication_sync_lag(void)
 	replication_sync_lag = box_check_replication_sync_lag();
 }
 
+/**
+ * Renew replication_synchro_quorum value if defined
+ * as a formula and we need to recalculate it.
+ */
+void
+box_update_replication_synchro_quorum(void)
+{
+	if (cfg_isnumber("replication_synchro_quorum")) {
+		/*
+		 * Even if replication_synchro_quorum is a constant
+		 * number the RAFT engine should be notified on
+		 * change of replicas amount.
+		 */
+		box_raft_update_election_quorum();
+		return;
+	}
+
+	/*
+	 * The formula has been verified already on the bootstrap
+	 * stage (and on dynamic reconfig as well), still there
+	 * is a Lua call inside, heck knowns what could go wrong
+	 * there thus panic if we're screwed.
+	 */
+	int value = replicaset.registered_count;
+	int quorum = eval_replication_synchro_quorum(value);
+	if (quorum < 0 || quorum >= VCLOCK_MAX)
+		panic("failed to eval replication_synchro_quorum");
+	say_info("update replication_synchro_quorum = %d", quorum);
+
+	replication_synchro_quorum = quorum;
+	txn_limbo_on_parameters_change(&txn_limbo);
+	box_raft_update_election_quorum();
+}
+
 int
 box_set_replication_synchro_quorum(void)
 {
 	int value = box_check_replication_synchro_quorum();
 	if (value < 0)
 		return -1;
-	replication_synchro_quorum = value;
-	txn_limbo_on_parameters_change(&txn_limbo);
-	box_raft_update_election_quorum();
+	box_update_replication_synchro_quorum();
 	return 0;
 }
 
diff --git a/src/box/box.h b/src/box/box.h
index b47a220b7..c3e1a1276 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -252,6 +252,7 @@ void box_set_replication_connect_timeout(void);
 void box_set_replication_connect_quorum(void);
 void box_set_replication_sync_lag(void);
 int box_set_replication_synchro_quorum(void);
+void box_update_replication_synchro_quorum(void);
 int box_set_replication_synchro_timeout(void);
 void box_set_replication_sync_timeout(void);
 void box_set_replication_skip_conflict(void);
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 76e2e92c2..af66c0e46 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -172,7 +172,7 @@ local template_cfg = {
     replication_timeout = 'number',
     replication_sync_lag = 'number',
     replication_sync_timeout = 'number',
-    replication_synchro_quorum = 'number',
+    replication_synchro_quorum = 'string, number',
     replication_synchro_timeout = 'number',
     replication_connect_timeout = 'number',
     replication_connect_quorum = 'number',
diff --git a/src/box/replication.cc b/src/box/replication.cc
index 931c73a37..3126d86ac 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -251,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;
-	box_raft_update_election_quorum();
+	box_update_replication_synchro_quorum();
 }
 
 void
@@ -300,7 +300,7 @@ replica_clear_id(struct replica *replica)
 		assert(!replica->anon);
 		replica_delete(replica);
 	}
-	box_raft_update_election_quorum();
+	box_update_replication_synchro_quorum();
 }
 
 void
-- 
2.26.2

  reply	other threads:[~2020-12-08  8:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-03 14:04 [Tarantool-patches] [PATCH v3 0/3] qsync: evaluate replication_synchro_quorum dynamically Cyrill Gorcunov
2020-12-03 14:04 ` [Tarantool-patches] [PATCH v3 1/3] cfg: add cfg_isnumber helper Cyrill Gorcunov
2020-12-03 14:04 ` [Tarantool-patches] [PATCH v3 2/3] cfg: support symbolic evaluation of replication_synchro_quorum Cyrill Gorcunov
2020-12-04 23:52   ` Vladislav Shpilevoy
2020-12-07 20:17     ` Cyrill Gorcunov
2020-12-07 21:25     ` Vladislav Shpilevoy
2020-12-07 21:48       ` Cyrill Gorcunov
2020-12-08  8:02         ` Cyrill Gorcunov [this message]
2020-12-09 23:22           ` Vladislav Shpilevoy
2020-12-11 12:25             ` Cyrill Gorcunov
2020-12-13 18:12               ` Vladislav Shpilevoy
2020-12-03 14:04 ` [Tarantool-patches] [PATCH v3 3/3] test: add replication/gh-5446-sqync-eval-quorum.test.lua Cyrill Gorcunov
2020-12-04 23:52   ` Vladislav Shpilevoy
2020-12-08  8:48     ` Cyrill Gorcunov
2020-12-09 23:22       ` Vladislav Shpilevoy
2020-12-04 10:15 ` [Tarantool-patches] [PATCH v3 0/3] qsync: evaluate replication_synchro_quorum dynamically Serge Petrenko

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=20201208080230.GG2303@grain \
    --to=gorcunov@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.perepelitsa@corp.mail.ru \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v3 2/3] cfg: support symbolic evaluation of replication_synchro_quorum' \
    /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