* [Tarantool-patches] [PATCH v2 1/3] cfg: add cfg_isnumber helper
2020-11-24 15:24 [Tarantool-patches] [PATCH v2 0/3] qsync: evaluate replication_synchro_quorum dynamically Cyrill Gorcunov
@ 2020-11-24 15:24 ` Cyrill Gorcunov
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum Cyrill Gorcunov
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 3/3] test: add replication/gh-5446-sqync-eval-quorum.test.lua Cyrill Gorcunov
2 siblings, 0 replies; 15+ messages in thread
From: Cyrill Gorcunov @ 2020-11-24 15:24 UTC (permalink / raw)
To: tml; +Cc: Vladislav Shpilevoy
We will need it to figure out if parameter
is a numeric value when doing configuration
check.
Part-of #5446
Acked-by: Serge Petrenko <sergepetrenko@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/cfg.c | 9 +++++++++
src/cfg.h | 6 ++++++
2 files changed, 15 insertions(+)
diff --git a/src/cfg.c b/src/cfg.c
index 46cff1999..cf07d5817 100644
--- a/src/cfg.c
+++ b/src/cfg.c
@@ -57,6 +57,15 @@ cfg_geti(const char *param)
return val;
}
+bool
+cfg_isnumber(const char *param)
+{
+ cfg_get(param);
+ bool ret = !!lua_isnumber(tarantool_L, -1);
+ lua_pop(tarantool_L, 1);
+ return ret;
+}
+
int
cfg_getb(const char *param)
{
diff --git a/src/cfg.h b/src/cfg.h
index 140bdddb8..e2955e6b2 100644
--- a/src/cfg.h
+++ b/src/cfg.h
@@ -40,6 +40,12 @@ extern "C" {
int
cfg_geti(const char *param);
+/**
+ * Test if cfg parameter is a number.
+ */
+bool
+cfg_isnumber(const char *param);
+
/**
* Gets boolean parameter of cfg.
* Returns -1 in case of nil
--
2.26.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-24 15:24 [Tarantool-patches] [PATCH v2 0/3] qsync: evaluate replication_synchro_quorum dynamically Cyrill Gorcunov
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 1/3] cfg: add cfg_isnumber helper Cyrill Gorcunov
@ 2020-11-24 15:24 ` Cyrill Gorcunov
2020-11-25 11:36 ` Serge Petrenko
2020-11-25 12:04 ` Serge Petrenko
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 3/3] test: add replication/gh-5446-sqync-eval-quorum.test.lua Cyrill Gorcunov
2 siblings, 2 replies; 15+ messages in thread
From: Cyrill Gorcunov @ 2020-11-24 15:24 UTC (permalink / raw)
To: tml; +Cc: Vladislav Shpilevoy
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_renew_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.
Closes #5446
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
@TarantoolBot document
Title: Synchronous replication
The `replication_synchro_quorum` parameter allows to specify
value not just as a plain integer number but as a formula too.
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 | 115 +++++++++++++++++++++++++++++++++++++--
src/box/box.h | 1 +
src/box/lua/load_cfg.lua | 2 +-
src/box/replication.cc | 4 ++
4 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/src/box/box.cc b/src/box/box.cc
index 1f7dec362..e4cb013c3 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -562,10 +562,81 @@ 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 f, err = loadstring(\"return (%s)\")\n"
+ "if not f then return 'failed to load \"%s\"' end\n"
+ "setfenv(f, { n = %d })\n"
+ "local ok, res = pcall(f)\n"
+ "if not ok then return res end\n"
+ "return math.floor(res)\n";
+ char buf[512];
+ int value = -1;
+
+ const char *expr = cfg_gets("replication_synchro_quorum");
+ size_t ret = snprintf(buf, sizeof(buf), fmt, expr,
+ expr, nr_replicas);
+ if (ret >= sizeof(buf)) {
+ diag_set(ClientError, ER_CFG,
+ "replication_synchro_quorum",
+ "the expression is too big");
+ return -1;
+ }
+
+ luaL_loadstring(tarantool_L, buf);
+ lua_call(tarantool_L, 0, 1);
+
+ if (lua_isnumber(tarantool_L, -1)) {
+ value = (int)lua_tonumber(tarantool_L, -1);
+ } else {
+ diag_set(ClientError, ER_CFG,
+ "replication_synchro_quorum",
+ lua_tostring(tarantool_L, -1));
+ return -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 < 0) {
+ say_warn("replication_synchro_quorum evaluated "
+ "to the negative value %d, ignore", value);
+ }
+ return MAX(1, MIN(value, VCLOCK_MAX-1));
+}
+
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
+ * thus lets pass a sane value here.
+ *
+ * Note though at moment of bootstrap this value
+ * is zero but the evaluator will return a valid
+ * number back, we rather use this variable in
+ * a sake of "sense" pointing out that we're
+ * depending on number of replicas.
+ */
+ int value = replicaset.registered_count;
+ quorum = eval_replication_synchro_quorum(value);
+ } 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 "
@@ -918,18 +989,54 @@ box_set_replication_sync_lag(void)
replication_sync_lag = box_check_replication_sync_lag();
}
+/**
+ * Assign new replication_synchro_quorum value
+ * and notify dependent subsystems.
+ */
+static void
+set_replication_synchro_quorum(int quorum)
+{
+ assert(quorum > 0 && quorum < VCLOCK_MAX);
+
+ replication_synchro_quorum = quorum;
+ txn_limbo_on_parameters_change(&txn_limbo);
+ raft_cfg_election_quorum(box_raft());
+}
+
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);
- raft_cfg_election_quorum(box_raft());
+ set_replication_synchro_quorum(value);
return 0;
}
+/**
+ * Renew replication_synchro_quorum value if defined
+ * as a formula and we need to recalculate it.
+ */
+void
+box_renew_replication_synchro_quorum(void)
+{
+ if (cfg_isnumber("replication_synchro_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)
+ panic("failed to eval replication_synchro_quorum");
+ say_info("renew replication_synchro_quorum = %d", quorum);
+ set_replication_synchro_quorum(quorum);
+}
+
int
box_set_replication_synchro_timeout(void)
{
diff --git a/src/box/box.h b/src/box/box.h
index b47a220b7..dd943f169 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_renew_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 65512cf0f..f89b8dbc3 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -250,6 +250,8 @@ 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_renew_replication_synchro_quorum();
}
void
@@ -298,6 +300,8 @@ replica_clear_id(struct replica *replica)
assert(!replica->anon);
replica_delete(replica);
}
+
+ box_renew_replication_synchro_quorum();
}
void
--
2.26.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum Cyrill Gorcunov
@ 2020-11-25 11:36 ` Serge Petrenko
2020-11-25 11:55 ` Cyrill Gorcunov
2020-11-25 12:04 ` Serge Petrenko
1 sibling, 1 reply; 15+ messages in thread
From: Serge Petrenko @ 2020-11-25 11:36 UTC (permalink / raw)
To: Cyrill Gorcunov, tml; +Cc: Vladislav Shpilevoy
24.11.2020 18:24, Cyrill Gorcunov пишет:
> 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_renew_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.
>
> Closes #5446
>
> Signed-off-by: Cyrill Gorcunov<gorcunov@gmail.com>
>
> @TarantoolBot document
> Title: Synchronous replication
>
> The `replication_synchro_quorum` parameter allows to specify
> value not just as a plain integer number but as a formula too.
> 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.
Hi! Thanks for the patch!
Please find my comments below.
> ---
> src/box/box.cc | 115 +++++++++++++++++++++++++++++++++++++--
> src/box/box.h | 1 +
> src/box/lua/load_cfg.lua | 2 +-
> src/box/replication.cc | 4 ++
> 4 files changed, 117 insertions(+), 5 deletions(-)
>
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 1f7dec362..e4cb013c3 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -562,10 +562,81 @@ 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 f, err = loadstring(\"return (%s)\")\n"
> + "if not f then return 'failed to load \"%s\"' end\n"
> + "setfenv(f, { n = %d })\n"
> + "local ok, res = pcall(f)\n"
> + "if not ok then return res end\n"
> + "return math.floor(res)\n";
> + char buf[512];
> + int value = -1;
> +
> + const char *expr = cfg_gets("replication_synchro_quorum");
> + size_t ret = snprintf(buf, sizeof(buf), fmt, expr,
> + expr, nr_replicas);
> + if (ret >= sizeof(buf)) {
> + diag_set(ClientError, ER_CFG,
> + "replication_synchro_quorum",
> + "the expression is too big");
> + return -1;
> + }
> +
> + luaL_loadstring(tarantool_L, buf);
> + lua_call(tarantool_L, 0, 1);
> +
> + if (lua_isnumber(tarantool_L, -1)) {
> + value = (int)lua_tonumber(tarantool_L, -1);
> + } else {
> + diag_set(ClientError, ER_CFG,
> + "replication_synchro_quorum",
> + lua_tostring(tarantool_L, -1));
> + return -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 < 0) {
> + say_warn("replication_synchro_quorum evaluated "
> + "to the negative value %d, ignore", value);
> + }
> + return MAX(1, MIN(value, VCLOCK_MAX-1));
I'd add a warning for value >= VCLOCK_MAX too.
And since you're checking for value explicitly anyway, IMO it'd be easier to
follow the code this way (up to you):
if (value < 0) {
say_warn(...);
value = 1;
} else if (value >= VCLOCK_MAX) {
say_warn(...);
value = VCLOCK_MAX - 1;
}
return 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
> + * thus lets pass a sane value here.
> + *
> + * Note though at moment of bootstrap this value
> + * is zero but the evaluator will return a valid
> + * number back, we rather use this variable in
> + * a sake of "sense" pointing out that we're
> + * depending on number of replicas.
> + */
This comment is not quite correct. This code will work not only on
bootstrap, but also on reconfiguration (box_set_replication_synchro_quorum
is called each time you issue box.cfg{replication_synchro_quorum=...})
So the value we pass below is not only sane, its the only correct value
to use.
I'd leave only this part of the comment (or any its variation you like):
Note that at the moment of bootstrap this value is zero.
We rely on evaluator returning a correct result (quorum = 1) in this case.
Or maybe simply pass MAX(1, replicaset.registered_count) each time?
With a relevant comment about bootstrap.
> + int value = replicaset.registered_count;
> + quorum = eval_replication_synchro_quorum(value);
> + } 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 "
> @@ -918,18 +989,54 @@ box_set_replication_sync_lag(void)
> replication_sync_lag = box_check_replication_sync_lag();
> }
>
> +/**
> + * Assign new replication_synchro_quorum value
> + * and notify dependent subsystems.
> + */
> +static void
> +set_replication_synchro_quorum(int quorum)
> +{
> + assert(quorum > 0 && quorum < VCLOCK_MAX);
> +
> + replication_synchro_quorum = quorum;
> + txn_limbo_on_parameters_change(&txn_limbo);
> + raft_cfg_election_quorum(box_raft());
> +}
> +
> 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);
> - raft_cfg_election_quorum(box_raft());
> + set_replication_synchro_quorum(value);
> return 0;
> }
>
> +/**
> + * Renew replication_synchro_quorum value if defined
> + * as a formula and we need to recalculate it.
> + */
> +void
> +box_renew_replication_synchro_quorum(void)
What do you think of `box_update_replication_synchro_quorum`?
> +{
> + if (cfg_isnumber("replication_synchro_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)
> + panic("failed to eval replication_synchro_quorum");
I propose to use the same check we had in
box_check_replication_synchro_quorum():
quorum <= 0 || quorum >= VCLOCK_MAX
For now the values are always in correct range (or < 0), but who knows
how quorum eval may change in future?
> + say_info("renew replication_synchro_quorum = %d", quorum);
> + set_replication_synchro_quorum(quorum);
> +}
> +
> int
> box_set_replication_synchro_timeout(void)
> {
> diff --git a/src/box/box.h b/src/box/box.h
> index b47a220b7..dd943f169 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_renew_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 65512cf0f..f89b8dbc3 100644
> --- a/src/box/replication.cc
> +++ b/src/box/replication.cc
> @@ -250,6 +250,8 @@ 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_renew_replication_synchro_quorum();
> }
>
> void
> @@ -298,6 +300,8 @@ replica_clear_id(struct replica *replica)
> assert(!replica->anon);
> replica_delete(replica);
> }
> +
> + box_renew_replication_synchro_quorum();
> }
>
> void
--
Serge Petrenko
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-25 11:36 ` Serge Petrenko
@ 2020-11-25 11:55 ` Cyrill Gorcunov
2020-11-25 12:10 ` Serge Petrenko
0 siblings, 1 reply; 15+ messages in thread
From: Cyrill Gorcunov @ 2020-11-25 11:55 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tml, Vladislav Shpilevoy
On Wed, Nov 25, 2020 at 02:36:18PM +0300, Serge Petrenko wrote:
...
> > + if (value < 0) {
> > + say_warn("replication_synchro_quorum evaluated "
> > + "to the negative value %d, ignore", value);
> > + }
> > + return MAX(1, MIN(value, VCLOCK_MAX-1));
>
>
> I'd add a warning for value >= VCLOCK_MAX too.
>
> And since you're checking for value explicitly anyway, IMO it'd be easier to
> follow the code this way (up to you):
>
> if (value < 0) {
> say_warn(...);
> value = 1;
> } else if (value >= VCLOCK_MAX) {
> say_warn(...);
> value = VCLOCK_MAX - 1;
> }
> return value;
Sure, I don't mind, thanks!
...
>
> I'd leave only this part of the comment (or any its variation you like):
>
> Note that at the moment of bootstrap this value is zero.
> We rely on evaluator returning a correct result (quorum = 1) in this case.
>
> Or maybe simply pass MAX(1, replicaset.registered_count) each time?
> With a relevant comment about bootstrap.
You know I would prefer all manipulations with data range to be inside
the evaluation function (including min/max and etc), This will allow
us to modify operations in one place if something get changed
in future and we eliminate requirements from the caller side. IOW,
the caller may pass whatever it wants but will get valid results
all the time (except syntax errors of course). So I think idea of
shrinking comment is better. But lets return to this place once
I update the code. Won't be hard to change.
> > +/**
> > + * Renew replication_synchro_quorum value if defined
> > + * as a formula and we need to recalculate it.
> > + */
> > +void
> > +box_renew_replication_synchro_quorum(void)
>
> What do you think of `box_update_replication_synchro_quorum`?
I don't mind with any name, so I'llupdate it to match.
You know the most thing which bothers me most is the fact
that we're calling box function from a deep code chain of
replication engine. replica_set/clear helpers are bound to
replication internals and ideally should know nothing about
box configuration that's why I thought of some kind of
notification hooks or triggers.
Say replicaset allocates a trigger on init and allow any code
to be notified with stage changes. In our case the stage is replica
id set or clear. Thus box could setup a trigger into replicaset
and we simply run the trigger. For me this would look a way
more natural but I'm not sure, because this will require to
introduce "stages" and instead of a single call to box_ we
will have a way more bigger patch with unclear picture...
Simply dunno.
> > + int quorum = eval_replication_synchro_quorum(value);
> > + if (quorum < 0)
> > + panic("failed to eval replication_synchro_quorum");
>
>
> I propose to use the same check we had in
> box_check_replication_synchro_quorum():
>
> quorum <= 0 || quorum >= VCLOCK_MAX
>
> For now the values are always in correct range (or < 0), but who knows
> how quorum eval may change in future?
Sure, thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-25 11:55 ` Cyrill Gorcunov
@ 2020-11-25 12:10 ` Serge Petrenko
2020-11-25 12:19 ` Cyrill Gorcunov
0 siblings, 1 reply; 15+ messages in thread
From: Serge Petrenko @ 2020-11-25 12:10 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml, Vladislav Shpilevoy
25.11.2020 14:55, Cyrill Gorcunov пишет:
> On Wed, Nov 25, 2020 at 02:36:18PM +0300, Serge Petrenko wrote:
> ...
>>> + if (value < 0) {
>>> + say_warn("replication_synchro_quorum evaluated "
>>> + "to the negative value %d, ignore", value);
>>> + }
>>> + return MAX(1, MIN(value, VCLOCK_MAX-1));
>>
>> I'd add a warning for value >= VCLOCK_MAX too.
>>
>> And since you're checking for value explicitly anyway, IMO it'd be easier to
>> follow the code this way (up to you):
>>
>> if (value < 0) {
>> say_warn(...);
>> value = 1;
>> } else if (value >= VCLOCK_MAX) {
>> say_warn(...);
>> value = VCLOCK_MAX - 1;
>> }
>> return value;
> Sure, I don't mind, thanks!
> ...
>> I'd leave only this part of the comment (or any its variation you like):
>>
>> Note that at the moment of bootstrap this value is zero.
>> We rely on evaluator returning a correct result (quorum = 1) in this case.
>>
>> Or maybe simply pass MAX(1, replicaset.registered_count) each time?
>> With a relevant comment about bootstrap.
> You know I would prefer all manipulations with data range to be inside
> the evaluation function (including min/max and etc), This will allow
> us to modify operations in one place if something get changed
> in future and we eliminate requirements from the caller side. IOW,
> the caller may pass whatever it wants but will get valid results
> all the time (except syntax errors of course). So I think idea of
> shrinking comment is better. But lets return to this place once
> I update the code. Won't be hard to change.
No problem.
>
>>> +/**
>>> + * Renew replication_synchro_quorum value if defined
>>> + * as a formula and we need to recalculate it.
>>> + */
>>> +void
>>> +box_renew_replication_synchro_quorum(void)
>> What do you think of `box_update_replication_synchro_quorum`?
> I don't mind with any name, so I'llupdate it to match.
>
> You know the most thing which bothers me most is the fact
> that we're calling box function from a deep code chain of
> replication engine. replica_set/clear helpers are bound to
> replication internals and ideally should know nothing about
> box configuration that's why I thought of some kind of
> notification hooks or triggers.
>
> Say replicaset allocates a trigger on init and allow any code
> to be notified with stage changes. In our case the stage is replica
> id set or clear. Thus box could setup a trigger into replicaset
> and we simply run the trigger. For me this would look a way
> more natural but I'm not sure, because this will require to
> introduce "stages" and instead of a single call to box_ we
> will have a way more bigger patch with unclear picture...
> Simply dunno.
I see. I'm not sure we need to implement this.
IMO it looks ok now. Let's wait for Vladislav's opinion.
>
>>> + int quorum = eval_replication_synchro_quorum(value);
>>> + if (quorum < 0)
>>> + panic("failed to eval replication_synchro_quorum");
>>
>> I propose to use the same check we had in
>> box_check_replication_synchro_quorum():
>>
>> quorum <= 0 || quorum >= VCLOCK_MAX
>>
>> For now the values are always in correct range (or < 0), but who knows
>> how quorum eval may change in future?
> Sure, thanks!
--
Serge Petrenko
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-25 12:10 ` Serge Petrenko
@ 2020-11-25 12:19 ` Cyrill Gorcunov
0 siblings, 0 replies; 15+ messages in thread
From: Cyrill Gorcunov @ 2020-11-25 12:19 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tml, Vladislav Shpilevoy
On Wed, Nov 25, 2020 at 03:10:48PM +0300, Serge Petrenko wrote:
>
> > > > +box_renew_replication_synchro_quorum(void)
> > > What do you think of `box_update_replication_synchro_quorum`?
> > I don't mind with any name, so I'llupdate it to match.
> >
> > You know the most thing which bothers me most is the fact
> > that we're calling box function from a deep code chain of
> > replication engine. replica_set/clear helpers are bound to
> > replication internals and ideally should know nothing about
> > box configuration that's why I thought of some kind of
> > notification hooks or triggers.
> >
> > Say replicaset allocates a trigger on init and allow any code
> > to be notified with stage changes. In our case the stage is replica
> > id set or clear. Thus box could setup a trigger into replicaset
> > and we simply run the trigger. For me this would look a way
> > more natural but I'm not sure, because this will require to
> > introduce "stages" and instead of a single call to box_ we
> > will have a way more bigger patch with unclear picture...
> > Simply dunno.
>
>
> I see. I'm not sure we need to implement this.
>
> IMO it looks ok now. Let's wait for Vladislav's opinion.
Sure, I'm fine with leaving it as a direct call for now.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum Cyrill Gorcunov
2020-11-25 11:36 ` Serge Petrenko
@ 2020-11-25 12:04 ` Serge Petrenko
2020-11-25 12:12 ` Cyrill Gorcunov
1 sibling, 1 reply; 15+ messages in thread
From: Serge Petrenko @ 2020-11-25 12:04 UTC (permalink / raw)
To: Cyrill Gorcunov, tml; +Cc: Vladislav Shpilevoy
24.11.2020 18:24, Cyrill Gorcunov пишет:
> diff --git a/src/box/box.cc b/src/box/box.cc
> index 1f7dec362..e4cb013c3 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -562,10 +562,81 @@ 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 f, err = loadstring(\"return (%s)\")\n"
> + "if not f then return 'failed to load \"%s\"' end\n"
> + "setfenv(f, { n = %d })\n"
> + "local ok, res = pcall(f)\n"
> + "if not ok then return res end\n"
> + "return math.floor(res)\n";
> + char buf[512];
> + int value = -1;
> +
> + const char *expr = cfg_gets("replication_synchro_quorum");
> + size_t ret = snprintf(buf, sizeof(buf), fmt, expr,
> + expr, nr_replicas);
> + if (ret >= sizeof(buf)) {
> + diag_set(ClientError, ER_CFG,
> + "replication_synchro_quorum",
> + "the expression is too big");
> + return -1;
> + }
Just noticed. Looks like this diag_set and the one below are ignored:
```
tarantool> a=string.rep('n', 1000)
---
...
tarantool> box.cfg{replication_synchro_quorum=a}
---
- error: 'Incorrect value for option ''replication_synchro_quorum'': the
value must
be greater than zero and less than maximal number of replicas'
...
```
> +
> + luaL_loadstring(tarantool_L, buf);
> + lua_call(tarantool_L, 0, 1);
> +
> + if (lua_isnumber(tarantool_L, -1)) {
> + value = (int)lua_tonumber(tarantool_L, -1);
> + } else {
> + diag_set(ClientError, ER_CFG,
> + "replication_synchro_quorum",
> + lua_tostring(tarantool_L, -1));
> + return -1;
> + }
Same for this diag_set:
```
tarantool> box.cfg{replication_synchro_quorum="garbage"}
---
- error: 'Incorrect value for option ''replication_synchro_quorum'': the
value must
be greater than zero and less than maximal number of replicas'
...
```
--
Serge Petrenko
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-25 12:04 ` Serge Petrenko
@ 2020-11-25 12:12 ` Cyrill Gorcunov
2020-11-25 12:46 ` Serge Petrenko
0 siblings, 1 reply; 15+ messages in thread
From: Cyrill Gorcunov @ 2020-11-25 12:12 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tml, Vladislav Shpilevoy
On Wed, Nov 25, 2020 at 03:04:07PM +0300, Serge Petrenko wrote:
>
> Just noticed. Looks like this diag_set and the one below are ignored:
> ```
> tarantool> a=string.rep('n', 1000)
> ---
> ...
>
> tarantool> box.cfg{replication_synchro_quorum=a}
> ---
> - error: 'Incorrect value for option ''replication_synchro_quorum'': the
> value must
> be greater than zero and less than maximal number of replicas'
> ...
Indeed. Actually if you remember I've been using errno for such errors.
Now we either should call diag_log() right after diag_set or return
errno handling?
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-25 12:12 ` Cyrill Gorcunov
@ 2020-11-25 12:46 ` Serge Petrenko
2020-11-25 12:53 ` Cyrill Gorcunov
0 siblings, 1 reply; 15+ messages in thread
From: Serge Petrenko @ 2020-11-25 12:46 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml, Vladislav Shpilevoy
25.11.2020 15:12, Cyrill Gorcunov пишет:
> On Wed, Nov 25, 2020 at 03:04:07PM +0300, Serge Petrenko wrote:
>> Just noticed. Looks like this diag_set and the one below are ignored:
>> ```
>> tarantool> a=string.rep('n', 1000)
>> ---
>> ...
>>
>> tarantool> box.cfg{replication_synchro_quorum=a}
>> ---
>> - error: 'Incorrect value for option ''replication_synchro_quorum'': the
>> value must
>> be greater than zero and less than maximal number of replicas'
>> ...
> Indeed. Actually if you remember I've been using errno for such errors.
> Now we either should call diag_log() right after diag_set or return
> errno handling?
Why? Every time eval returns a value < 0 this means an error.
Just add a check to check_replication_synchro_quorum.
eval's return value is ignored here, and diag is rewritten with an
unrelated error.
--
Serge Petrenko
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-25 12:46 ` Serge Petrenko
@ 2020-11-25 12:53 ` Cyrill Gorcunov
2020-11-25 13:49 ` Serge Petrenko
0 siblings, 1 reply; 15+ messages in thread
From: Cyrill Gorcunov @ 2020-11-25 12:53 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tml, Vladislav Shpilevoy
On Wed, Nov 25, 2020 at 03:46:04PM +0300, Serge Petrenko wrote:
>
> 25.11.2020 15:12, Cyrill Gorcunov пишет:
> > On Wed, Nov 25, 2020 at 03:04:07PM +0300, Serge Petrenko wrote:
> > > Just noticed. Looks like this diag_set and the one below are ignored:
> > > ```
> > > tarantool> a=string.rep('n', 1000)
> > > ---
> > > ...
> > >
> > > tarantool> box.cfg{replication_synchro_quorum=a}
> > > ---
> > > - error: 'Incorrect value for option ''replication_synchro_quorum'': the
> > > value must
> > > be greater than zero and less than maximal number of replicas'
> > > ...
> > Indeed. Actually if you remember I've been using errno for such errors.
> > Now we either should call diag_log() right after diag_set or return
> > errno handling?
> Why? Every time eval returns a value < 0 this means an error.
> Just add a check to check_replication_synchro_quorum.
> eval's return value is ignored here, and diag is rewritten with an unrelated
> error.
iow, you mean something like this?
---
static int
box_check_replication_synchro_quorum(void)
{
int quorum = 0;
if (!cfg_isnumber("replication_synchro_quorum")) {
int value = replicaset.registered_count;
quorum = box_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 "
"maximal number of replicas");
return -1;
}
return quorum;
}
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum
2020-11-25 12:53 ` Cyrill Gorcunov
@ 2020-11-25 13:49 ` Serge Petrenko
0 siblings, 0 replies; 15+ messages in thread
From: Serge Petrenko @ 2020-11-25 13:49 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml, Vladislav Shpilevoy
25.11.2020 15:53, Cyrill Gorcunov пишет:
> On Wed, Nov 25, 2020 at 03:46:04PM +0300, Serge Petrenko wrote:
>> 25.11.2020 15:12, Cyrill Gorcunov пишет:
>>> On Wed, Nov 25, 2020 at 03:04:07PM +0300, Serge Petrenko wrote:
>>>> Just noticed. Looks like this diag_set and the one below are ignored:
>>>> ```
>>>> tarantool> a=string.rep('n', 1000)
>>>> ---
>>>> ...
>>>>
>>>> tarantool> box.cfg{replication_synchro_quorum=a}
>>>> ---
>>>> - error: 'Incorrect value for option ''replication_synchro_quorum'': the
>>>> value must
>>>> be greater than zero and less than maximal number of replicas'
>>>> ...
>>> Indeed. Actually if you remember I've been using errno for such errors.
>>> Now we either should call diag_log() right after diag_set or return
>>> errno handling?
>> Why? Every time eval returns a value < 0 this means an error.
>> Just add a check to check_replication_synchro_quorum.
>> eval's return value is ignored here, and diag is rewritten with an unrelated
>> error.
> iow, you mean something like this?
Yep.
> ---
> static int
> box_check_replication_synchro_quorum(void)
> {
> int quorum = 0;
>
> if (!cfg_isnumber("replication_synchro_quorum")) {
> int value = replicaset.registered_count;
> quorum = box_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 "
> "maximal number of replicas");
> return -1;
> }
> return quorum;
> }
>
--
Serge Petrenko
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Tarantool-patches] [PATCH v2 3/3] test: add replication/gh-5446-sqync-eval-quorum.test.lua
2020-11-24 15:24 [Tarantool-patches] [PATCH v2 0/3] qsync: evaluate replication_synchro_quorum dynamically Cyrill Gorcunov
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 1/3] cfg: add cfg_isnumber helper Cyrill Gorcunov
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 2/3] cfg: support symbolic evaluation of replication_synchro_quorum Cyrill Gorcunov
@ 2020-11-24 15:24 ` Cyrill Gorcunov
2020-11-25 13:57 ` Serge Petrenko
2 siblings, 1 reply; 15+ messages in thread
From: Cyrill Gorcunov @ 2020-11-24 15:24 UTC (permalink / raw)
To: tml; +Cc: Vladislav Shpilevoy
Part-of #5446
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
.../gh-5446-sqync-eval-quorum.result | 156 ++++++++++++++++++
.../gh-5446-sqync-eval-quorum.test.lua | 62 +++++++
test/replication/replica-quorum-1.lua | 1 +
test/replication/replica-quorum-2.lua | 1 +
test/replication/replica-quorum-3.lua | 1 +
test/replication/replica-quorum-4.lua | 1 +
test/replication/replica-quorum.lua | 13 ++
7 files changed, 235 insertions(+)
create mode 100644 test/replication/gh-5446-sqync-eval-quorum.result
create mode 100644 test/replication/gh-5446-sqync-eval-quorum.test.lua
create mode 120000 test/replication/replica-quorum-1.lua
create mode 120000 test/replication/replica-quorum-2.lua
create mode 120000 test/replication/replica-quorum-3.lua
create mode 120000 test/replication/replica-quorum-4.lua
create mode 100644 test/replication/replica-quorum.lua
diff --git a/test/replication/gh-5446-sqync-eval-quorum.result b/test/replication/gh-5446-sqync-eval-quorum.result
new file mode 100644
index 000000000..3aff913a3
--- /dev/null
+++ b/test/replication/gh-5446-sqync-eval-quorum.result
@@ -0,0 +1,156 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+engine = test_run:get_cfg('engine')
+ | ---
+ | ...
+
+box.schema.user.grant('guest', 'replication')
+ | ---
+ | ...
+
+box.cfg { replication_synchro_quorum = "n/2+1", replication_synchro_timeout = 1000 }
+ | ---
+ | ...
+
+test_run:cmd('create server replica1 with rpl_master=default,\
+ script="replication/replica-quorum-1.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica1 with wait=True, wait_load=True')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('create server replica2 with rpl_master=default,\
+ script="replication/replica-quorum-2.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica2 with wait=True, wait_load=True')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('create server replica3 with rpl_master=default,\
+ script="replication/replica-quorum-3.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica3 with wait=True, wait_load=True')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('create server replica4 with rpl_master=default,\
+ script="replication/replica-quorum-4.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica4 with wait=True, wait_load=True')
+ | ---
+ | - true
+ | ...
+
+_ = box.schema.space.create('sync', {is_sync = true, engine = engine})
+ | ---
+ | ...
+s = box.space.sync
+ | ---
+ | ...
+
+s:format({{name = 'id', type = 'unsigned'}, {name = 'band_name', type = 'string'}})
+ | ---
+ | ...
+
+_ = s:create_index('primary', {parts = {'id'}})
+ | ---
+ | ...
+s:insert({1, '1'})
+ | ---
+ | - [1, '1']
+ | ...
+s:insert({2, '2'})
+ | ---
+ | - [2, '2']
+ | ...
+s:insert({3, '3'})
+ | ---
+ | - [3, '3']
+ | ...
+
+-- Wait the data to be propagated, there is no much
+-- need for this since we're testing the quorum number
+-- update only but just in case.
+test_run:wait_lsn('replica1', 'default')
+ | ---
+ | ...
+test_run:wait_lsn('replica2', 'default')
+ | ---
+ | ...
+test_run:wait_lsn('replica3', 'default')
+ | ---
+ | ...
+test_run:wait_lsn('replica4', 'default')
+ | ---
+ | ...
+
+-- Make sure we were configured in a proper way
+match = 'set \'replication_synchro_quorum\' configuration option to \"n\\/2%+1'
+ | ---
+ | ...
+test_run:grep_log("default", match) ~= nil
+ | ---
+ | - true
+ | ...
+
+-- There are 4 replicas -> replication_synchro_quorum = 4/2 + 1 = 3
+match = 'renew replication_synchro_quorum = 3'
+ | ---
+ | ...
+test_run:grep_log("default", match) ~= nil
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('stop server replica1')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica1')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('stop server replica2')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica2')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('stop server replica3')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica3')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('stop server replica4')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica4')
+ | ---
+ | - true
+ | ...
+
+box.schema.user.revoke('guest', 'replication')
+ | ---
+ | ...
diff --git a/test/replication/gh-5446-sqync-eval-quorum.test.lua b/test/replication/gh-5446-sqync-eval-quorum.test.lua
new file mode 100644
index 000000000..cf8058c85
--- /dev/null
+++ b/test/replication/gh-5446-sqync-eval-quorum.test.lua
@@ -0,0 +1,62 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+
+box.schema.user.grant('guest', 'replication')
+
+box.cfg { replication_synchro_quorum = "n/2+1", replication_synchro_timeout = 1000 }
+
+test_run:cmd('create server replica1 with rpl_master=default,\
+ script="replication/replica-quorum-1.lua"')
+test_run:cmd('start server replica1 with wait=True, wait_load=True')
+
+test_run:cmd('create server replica2 with rpl_master=default,\
+ script="replication/replica-quorum-2.lua"')
+test_run:cmd('start server replica2 with wait=True, wait_load=True')
+
+test_run:cmd('create server replica3 with rpl_master=default,\
+ script="replication/replica-quorum-3.lua"')
+test_run:cmd('start server replica3 with wait=True, wait_load=True')
+
+test_run:cmd('create server replica4 with rpl_master=default,\
+ script="replication/replica-quorum-4.lua"')
+test_run:cmd('start server replica4 with wait=True, wait_load=True')
+
+_ = box.schema.space.create('sync', {is_sync = true, engine = engine})
+s = box.space.sync
+
+s:format({{name = 'id', type = 'unsigned'}, {name = 'band_name', type = 'string'}})
+
+_ = s:create_index('primary', {parts = {'id'}})
+s:insert({1, '1'})
+s:insert({2, '2'})
+s:insert({3, '3'})
+
+-- Wait the data to be propagated, there is no much
+-- need for this since we're testing the quorum number
+-- update only but just in case.
+test_run:wait_lsn('replica1', 'default')
+test_run:wait_lsn('replica2', 'default')
+test_run:wait_lsn('replica3', 'default')
+test_run:wait_lsn('replica4', 'default')
+
+-- Make sure we were configured in a proper way
+match = 'set \'replication_synchro_quorum\' configuration option to \"n\\/2%+1'
+test_run:grep_log("default", match) ~= nil
+
+-- There are 4 replicas -> replication_synchro_quorum = 4/2 + 1 = 3
+match = 'renew replication_synchro_quorum = 3'
+test_run:grep_log("default", match) ~= nil
+
+test_run:cmd('stop server replica1')
+test_run:cmd('delete server replica1')
+
+test_run:cmd('stop server replica2')
+test_run:cmd('delete server replica2')
+
+test_run:cmd('stop server replica3')
+test_run:cmd('delete server replica3')
+
+test_run:cmd('stop server replica4')
+test_run:cmd('delete server replica4')
+
+box.schema.user.revoke('guest', 'replication')
diff --git a/test/replication/replica-quorum-1.lua b/test/replication/replica-quorum-1.lua
new file mode 120000
index 000000000..f0c3e7089
--- /dev/null
+++ b/test/replication/replica-quorum-1.lua
@@ -0,0 +1 @@
+/home/cyrill/projects/tarantool/tarantool.git/test/replication/replica-quorum.lua
\ No newline at end of file
diff --git a/test/replication/replica-quorum-2.lua b/test/replication/replica-quorum-2.lua
new file mode 120000
index 000000000..f0c3e7089
--- /dev/null
+++ b/test/replication/replica-quorum-2.lua
@@ -0,0 +1 @@
+/home/cyrill/projects/tarantool/tarantool.git/test/replication/replica-quorum.lua
\ No newline at end of file
diff --git a/test/replication/replica-quorum-3.lua b/test/replication/replica-quorum-3.lua
new file mode 120000
index 000000000..f0c3e7089
--- /dev/null
+++ b/test/replication/replica-quorum-3.lua
@@ -0,0 +1 @@
+/home/cyrill/projects/tarantool/tarantool.git/test/replication/replica-quorum.lua
\ No newline at end of file
diff --git a/test/replication/replica-quorum-4.lua b/test/replication/replica-quorum-4.lua
new file mode 120000
index 000000000..f0c3e7089
--- /dev/null
+++ b/test/replication/replica-quorum-4.lua
@@ -0,0 +1 @@
+/home/cyrill/projects/tarantool/tarantool.git/test/replication/replica-quorum.lua
\ No newline at end of file
diff --git a/test/replication/replica-quorum.lua b/test/replication/replica-quorum.lua
new file mode 100644
index 000000000..8d91a078f
--- /dev/null
+++ b/test/replication/replica-quorum.lua
@@ -0,0 +1,13 @@
+#!/usr/bin/env tarantool
+repl_list = os.getenv("MASTER")
+
+-- Start the console first to allow test-run to attach even before
+-- box.cfg is finished.
+require('console').listen(os.getenv('ADMIN'))
+
+box.cfg({
+ listen = os.getenv("LISTEN"),
+ replication = repl_list,
+ memtx_memory = 107374182,
+ replication_timeout = 1000,
+})
--
2.26.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Tarantool-patches] [PATCH v2 3/3] test: add replication/gh-5446-sqync-eval-quorum.test.lua
2020-11-24 15:24 ` [Tarantool-patches] [PATCH v2 3/3] test: add replication/gh-5446-sqync-eval-quorum.test.lua Cyrill Gorcunov
@ 2020-11-25 13:57 ` Serge Petrenko
2020-11-25 14:10 ` Cyrill Gorcunov
0 siblings, 1 reply; 15+ messages in thread
From: Serge Petrenko @ 2020-11-25 13:57 UTC (permalink / raw)
To: Cyrill Gorcunov, tml; +Cc: Vladislav Shpilevoy
24.11.2020 18:24, Cyrill Gorcunov пишет:
> Part-of #5446
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> .../gh-5446-sqync-eval-quorum.result | 156 ++++++++++++++++++
> .../gh-5446-sqync-eval-quorum.test.lua | 62 +++++++
> test/replication/replica-quorum-1.lua | 1 +
> test/replication/replica-quorum-2.lua | 1 +
> test/replication/replica-quorum-3.lua | 1 +
> test/replication/replica-quorum-4.lua | 1 +
> test/replication/replica-quorum.lua | 13 ++
> 7 files changed, 235 insertions(+)
> create mode 100644 test/replication/gh-5446-sqync-eval-quorum.result
> create mode 100644 test/replication/gh-5446-sqync-eval-quorum.test.lua
> create mode 120000 test/replication/replica-quorum-1.lua
> create mode 120000 test/replication/replica-quorum-2.lua
> create mode 120000 test/replication/replica-quorum-3.lua
> create mode 120000 test/replication/replica-quorum-4.lua
> create mode 100644 test/replication/replica-quorum.lua
This patch LGTM with one comment.
AFAIU you may use replica.lua for this test.
--
Serge Petrenko
^ permalink raw reply [flat|nested] 15+ messages in thread