Re: [Tarantool-patches] [PATCH v4 3/3] test: add replication/gh-5446-qsync-eval-quorum.test.lua

[Cyrill Gorcunov <gorcunov@gmail.com>]

On Sun, Dec 20, 2020 at 06:01:02PM +0100, Vladislav Shpilevoy wrote:
> Thanks for the fixes!
> 
> > Took both notes from some existing example. Would the following be better?
> 
> Yes, thanks, it does not raise any questions now. Although
> you could keep 's' variable to access the space methods shorter.

Vlad, here is an interdiff for v4. I addressed all your comments I hope.
If you prefer I'll resend the whole series anew.
---
issue https://github.com/tarantool/tarantool/issues/5446
branch gorcunov/gh-5446-eval-quorum-5

Cyrill Gorcunov (4):
  cfg: add cfg_isnumber helper
  cfg: rework box_check_replication_synchro_quorum
  cfg: support symbolic evaluation of replication_synchro_quorum
  test: add replication/gh-5446-qsync-eval-quorum.test.lua

base-commit: 28f3b2f1e845aff49048d92f9062a4dfa365bf57
--
git diff gorcunov/gh-5446-eval-quorum-4
(I removed interdiff for test code)
--
diff --git a/src/box/box.cc b/src/box/box.cc
index ff5c27743..d3ec1faf3 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -585,7 +585,6 @@ box_eval_replication_synchro_quorum(int nr_replicas)
 		"end\n"
 		"return math.floor(res)\n";
 	const char *expr = cfg_gets("replication_synchro_quorum");
-	int quorum = -1;
 
 	/*
 	 * cfg_gets uses static buffer as well so we need a local
@@ -610,6 +609,7 @@ box_eval_replication_synchro_quorum(int nr_replicas)
 		return -1;
 	}
 
+	int quorum = -1;
 	if (lua_isnumber(tarantool_L, -1))
 		quorum = (int)lua_tonumber(tarantool_L, -1);
 	lua_pop(tarantool_L, 1);
@@ -618,9 +618,6 @@ box_eval_replication_synchro_quorum(int nr_replicas)
 	 * At least we should have 1 node to sync, the weird
 	 * formulas such as N-2 do not guarantee quorums thus
 	 * return an error.
-	 *
-	 * Since diag_set doesn't allow to show the valid range
-	 * lets print a warning too.
 	 */
 	if (quorum <= 0 || quorum >= VCLOCK_MAX) {
 		const char *msg =
@@ -640,8 +637,6 @@ box_eval_replication_synchro_quorum(int nr_replicas)
 static int
 box_check_replication_synchro_quorum(void)
 {
-	int quorum = 0;
-
 	if (!cfg_isnumber("replication_synchro_quorum")) {
 		/*
 		 * The formula uses symbolic name 'N' as
@@ -656,27 +651,20 @@ box_check_replication_synchro_quorum(void)
 		 * sure that the cycle won't take too much time.
 		 */
 		for (int i = 1; i < VCLOCK_MAX; i++) {
-			quorum = box_eval_replication_synchro_quorum(i);
-			if (quorum < 0)
+			if (box_eval_replication_synchro_quorum(i) < 0)
 				return -1;
 		}
-		/*
-		 * Just to make clear the number we return here doesn't
-		 * have any special meaning, only errors are matter.
-		 * The real value is dynamic and will be updated on demand.
-		 */
 		return 0;
-	} else {
-		quorum = cfg_geti("replication_synchro_quorum");
 	}
 
+	int 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;
+	return 0;
 }
 
 static double
@@ -877,7 +865,7 @@ box_check_config(void)
 	box_check_replication_connect_timeout();
 	box_check_replication_connect_quorum();
 	box_check_replication_sync_lag();
-	if (box_check_replication_synchro_quorum() < 0)
+	if (box_check_replication_synchro_quorum() != 0)
 		diag_raise();
 	if (box_check_replication_synchro_timeout() < 0)
 		diag_raise();
@@ -1022,10 +1010,6 @@ 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)
 {
@@ -1057,8 +1041,7 @@ box_update_replication_synchro_quorum(void)
 int
 box_set_replication_synchro_quorum(void)
 {
-	int value = box_check_replication_synchro_quorum();
-	if (value < 0)
+	if (box_check_replication_synchro_quorum() != 0)
 		return -1;
 	box_update_replication_synchro_quorum();
 	return 0;
diff --git a/src/box/box.h b/src/box/box.h
index c3e1a1276..8a7cda194 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -251,8 +251,8 @@ void box_set_replication_timeout(void);
 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_quorum(void);
 int box_set_replication_synchro_timeout(void);
 void box_set_replication_sync_timeout(void);
 void box_set_replication_skip_conflict(void);
-- 
2.26.2