[Tarantool-patches] [PATCH v6 0/5] qsync: evaluate replication_synchro_quorum dynamically
Cyrill Gorcunov
gorcunov at gmail.com
Tue Dec 22 14:14:03 MSK 2020
Take a look please once time permit.
v2 (by Serge):
- keep replication_synchro_quorum been skipped at bootstrap in load_cfg.lua
- eliminate redundant say_info calls
- call quorum update routine from replica_set_id/replica_clear_id
- use replicaset.registered_count directly when evaluating the formula
- make quorum evaluation procedure always return value in allowed range,
the only error which may happen here is some syntax error or Lua evaluation
errors
- a test has been added
v3 (by Serge, Mons, Vlad):
- use replica.lua in tests
- use N symbol in formula
- use lua_pcall when evaluating a formula
- make formula more safe itself, provide various math helpers
- use box_update_replication_synchro_quorum name as a general
updater from replication code
- do not forget to update raft election quorum inside
box_update_replication_synchro_quorum
- print warns inside functions evaluator if value get out of bounds
v4 (by Vlad):
- when testing a formula we walk over all amount of replicas,
thus we are sure that later when real evaluation takes place
we won't get quorum out of bounds
- improve test to make sure that when no quorum aquired the
transaction doesn't pass
v5 (by Vlad):
- make box_check_replication_synchro_quorum to return 0|1
- in test use `s` and a shorthand for the space
v6 (by Vlad):
- use wait_log in test 'cause log flushing doesn't go immediately
- fix potential numbers overflow when setting up quorum
Vlad, I resend the whole series for one more general review.
Take a look once time permit. The interdiff for v5 is below.
issue https://github.com/tarantool/tarantool/issues/5446
branch gorcunov/gh-5446-eval-quorum-6
Cyrill Gorcunov (5):
cfg: add cfg_isnumber helper
cfg: rework box_check_replication_synchro_quorum
cfg: support symbolic evaluation of replication_synchro_quorum
cfg: more precise check for replication_synchro_quorum value
test: add replication/gh-5446-qsync-eval-quorum.test.lua
src/box/box.cc | 148 +++++++++-
src/box/box.h | 1 +
src/box/lua/load_cfg.lua | 2 +-
src/box/replication.cc | 4 +-
src/cfg.c | 9 +
src/cfg.h | 6 +
.../gh-5446-qsync-eval-quorum.result | 277 ++++++++++++++++++
.../gh-5446-qsync-eval-quorum.test.lua | 110 +++++++
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-5.lua | 1 +
test/replication/replica-quorum-6.lua | 1 +
14 files changed, 552 insertions(+), 11 deletions(-)
create mode 100644 test/replication/gh-5446-qsync-eval-quorum.result
create mode 100644 test/replication/gh-5446-qsync-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 120000 test/replication/replica-quorum-5.lua
create mode 120000 test/replication/replica-quorum-6.lua
base-commit: 28f3b2f1e845aff49048d92f9062a4dfa365bf57
--
diff --git a/src/box/box.cc b/src/box/box.cc
index d3ec1faf3..b3cc45358 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -609,9 +609,9 @@ box_eval_replication_synchro_quorum(int nr_replicas)
return -1;
}
- int quorum = -1;
+ int64_t quorum = -1;
if (lua_isnumber(tarantool_L, -1))
- quorum = (int)lua_tonumber(tarantool_L, -1);
+ quorum = luaL_toint64(tarantool_L, -1);
lua_pop(tarantool_L, 1);
/*
@@ -657,7 +657,7 @@ box_check_replication_synchro_quorum(void)
return 0;
}
- int quorum = cfg_geti("replication_synchro_quorum");
+ int64_t quorum = cfg_geti64("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 "
@@ -1026,13 +1026,20 @@ box_update_replication_synchro_quorum(void)
*/
int value = MAX(1, replicaset.registered_count);
quorum = box_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);
} else {
quorum = cfg_geti("replication_synchro_quorum");
}
+ /*
+ * This should never happen because the values were
+ * validated already but just to prevent from
+ * unexpected changes and because the value is too
+ * important for qsync, lets re-check (this is cheap).
+ */
+ if (quorum <= 0 || quorum >= VCLOCK_MAX)
+ panic("failed to eval/fetch replication_synchro_quorum");
+
replication_synchro_quorum = quorum;
txn_limbo_on_parameters_change(&txn_limbo);
box_raft_update_election_quorum();
diff --git a/test/replication/gh-5446-qsync-eval-quorum.result b/test/replication/gh-5446-qsync-eval-quorum.result
index 6fd31d1bc..6c3c63577 100644
--- a/test/replication/gh-5446-qsync-eval-quorum.result
+++ b/test/replication/gh-5446-qsync-eval-quorum.result
@@ -29,6 +29,18 @@ box.cfg{replication_synchro_quorum = "N-1"}
| evaluated to the quorum 0 for replica number 1, which is out of range [1;31]'
| ...
+-- Test big number value
+box.cfg{replication_synchro_quorum = '4294967297'}
+ | ---
+ | - error: 'Incorrect value for option ''replication_synchro_quorum'': the value must
+ | be greater than zero and less than maximal number of replicas'
+ | ...
+box.cfg{replication_synchro_quorum = 4294967297}
+ | ---
+ | - error: 'Incorrect value for option ''replication_synchro_quorum'': the value must
+ | be greater than zero and less than maximal number of replicas'
+ | ...
+
-- Use canonical majority formula
box.cfg { replication_synchro_quorum = "N/2+1", replication_synchro_timeout = 1000 }
| ---
@@ -36,9 +48,9 @@ box.cfg { replication_synchro_quorum = "N/2+1", replication_synchro_timeout = 10
match = 'set \'replication_synchro_quorum\' configuration option to \"N\\/2%+1'
| ---
| ...
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
| ---
- | - true
+ | - set 'replication_synchro_quorum' configuration option to "N\/2+1
| ...
-- Create a sync space we will operate on
@@ -70,9 +82,9 @@ test_run:cmd('start server replica1 with wait=True, wait_load=True')
match = 'update replication_synchro_quorum = 2'
| ---
| ...
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
| ---
- | - true
+ | - update replication_synchro_quorum = 2
| ...
test_run:cmd('create server replica2 with rpl_master=default,\
@@ -89,9 +101,9 @@ test_run:cmd('start server replica2 with wait=True, wait_load=True')
match = 'update replication_synchro_quorum = 2'
| ---
| ...
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
| ---
- | - true
+ | - update replication_synchro_quorum = 2
| ...
test_run:cmd('create server replica3 with rpl_master=default,\
@@ -108,9 +120,9 @@ test_run:cmd('start server replica3 with wait=True, wait_load=True')
match = 'update replication_synchro_quorum = 3'
| ---
| ...
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
| ---
- | - true
+ | - update replication_synchro_quorum = 3
| ...
test_run:cmd('create server replica4 with rpl_master=default,\
@@ -127,9 +139,9 @@ test_run:cmd('start server replica4 with wait=True, wait_load=True')
match = 'update replication_synchro_quorum = 3'
| ---
| ...
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
| ---
- | - true
+ | - update replication_synchro_quorum = 3
| ...
test_run:cmd('create server replica5 with rpl_master=default,\
@@ -156,9 +168,9 @@ test_run:cmd('start server replica6 with wait=True, wait_load=True')
match = 'update replication_synchro_quorum = 4'
| ---
| ...
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
| ---
- | - true
+ | - update replication_synchro_quorum = 4
| ...
-- 5 replicas left, the commit should pass
diff --git a/test/replication/gh-5446-qsync-eval-quorum.test.lua b/test/replication/gh-5446-qsync-eval-quorum.test.lua
index 56269fc43..b7ce9a9ca 100644
--- a/test/replication/gh-5446-qsync-eval-quorum.test.lua
+++ b/test/replication/gh-5446-qsync-eval-quorum.test.lua
@@ -10,10 +10,14 @@ box.cfg{replication_synchro_quorum = "aaa"}
box.cfg{replication_synchro_quorum = "N+1"}
box.cfg{replication_synchro_quorum = "N-1"}
+-- Test big number value
+box.cfg{replication_synchro_quorum = '4294967297'}
+box.cfg{replication_synchro_quorum = 4294967297}
+
-- Use canonical majority formula
box.cfg { replication_synchro_quorum = "N/2+1", replication_synchro_timeout = 1000 }
match = 'set \'replication_synchro_quorum\' configuration option to \"N\\/2%+1'
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
-- Create a sync space we will operate on
_ = box.schema.space.create('sync', {is_sync = true, engine = engine})
@@ -27,7 +31,7 @@ test_run:cmd('start server replica1 with wait=True, wait_load=True')
-- 1 replica -> replication_synchro_quorum = 2/2 + 1 = 2
match = 'update replication_synchro_quorum = 2'
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
test_run:cmd('create server replica2 with rpl_master=default,\
script="replication/replica-quorum-2.lua"')
@@ -35,7 +39,7 @@ test_run:cmd('start server replica2 with wait=True, wait_load=True')
-- 2 replicas -> replication_synchro_quorum = 3/2 + 1 = 2
match = 'update replication_synchro_quorum = 2'
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
test_run:cmd('create server replica3 with rpl_master=default,\
script="replication/replica-quorum-3.lua"')
@@ -43,7 +47,7 @@ test_run:cmd('start server replica3 with wait=True, wait_load=True')
-- 3 replicas -> replication_synchro_quorum = 4/2 + 1 = 3
match = 'update replication_synchro_quorum = 3'
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
test_run:cmd('create server replica4 with rpl_master=default,\
script="replication/replica-quorum-4.lua"')
@@ -51,7 +55,7 @@ test_run:cmd('start server replica4 with wait=True, wait_load=True')
-- 4 replicas -> replication_synchro_quorum = 5/2 + 1 = 3
match = 'update replication_synchro_quorum = 3'
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
test_run:cmd('create server replica5 with rpl_master=default,\
script="replication/replica-quorum-5.lua"')
@@ -63,7 +67,7 @@ test_run:cmd('start server replica6 with wait=True, wait_load=True')
-- 6 replicas -> replication_synchro_quorum = 7/2 + 1 = 4
match = 'update replication_synchro_quorum = 4'
-test_run:grep_log("default", match) ~= nil
+test_run:wait_log('default', match, nil, 5)
-- 5 replicas left, the commit should pass
test_run:cmd('stop server replica1')
More information about the Tarantool-patches
mailing list