From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D75594765E0 for ; Mon, 21 Dec 2020 19:05:58 +0300 (MSK) Received: by mail-lf1-f44.google.com with SMTP id o13so24922369lfr.3 for ; Mon, 21 Dec 2020 08:05:58 -0800 (PST) Date: Mon, 21 Dec 2020 19:05:55 +0300 From: Cyrill Gorcunov Message-ID: <20201221160555.GA81750@grain> References: <20201214113935.1040421-1-gorcunov@gmail.com> <20201214113935.1040421-4-gorcunov@gmail.com> <2b00fba4-63bd-559f-c273-a02cf3fed6f3@tarantool.org> <20201218081416.GG14556@grain> <0df4021b-b2b5-0157-e532-0845b0842e92@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0df4021b-b2b5-0157-e532-0845b0842e92@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 3/3] test: add replication/gh-5446-qsync-eval-quorum.test.lua List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: Mons Anderson , tml 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