[tarantool-patches] [PATCH v4] box: fixed comparison of old and new config options
Olga Arkhangelskaia
arkholga at tarantool.org
Mon Nov 5 11:57:49 MSK 2018
box.cfg() updates only those options that have actually changed.
However, for replication it is not always true: box.cfg{replication = x}
and box.cfg{replication = {x}} are treated differently, and as
the result - replication is restarted. The patch fixes such behaviour.
Closes #3711
---
Issue:
https://github.com/tarantool/tarantool/issues/3711
Branch:
https://github.com/tarantool/tarantool/tree/OKriw/gh-3711-do-not-restart-replication-if-config-did-not-change-1.10
v1:
https://www.freelists.org/post/tarantool-patches/PATCH-box-fixed-comparison-of-old-and-new-config-options
v2:
https://www.freelists.org/post/tarantool-patches/PATCH-v2-box-fixed-comparison-of-old-and-new-config-options
v3:
https://www.freelists.org/post/tarantool-patches/PATCH-v3-box-fixed-comparison-of-old-and-new-config-options
Changes in v2:
- changed test
- conversion from num to string now in modify_cfg
- no sort table in comparison
- got rid of table.getn
Changes in v3:
- changed test
- fixed comments, names, identation
- added normalize_uri_list
Changes in v3:
- fixed test
- now we transform value 1234 into {'1234'} for replication
- fixed indentaion
src/box/lua/load_cfg.lua | 40 +++++++++++++--
test/replication/errinj.result | 2 +-
test/replication/errinj.test.lua | 2 +-
test/replication/misc.result | 102 ++++++++++++++++++++++++++++++++++++++-
test/replication/misc.test.lua | 41 +++++++++++++++-
5 files changed, 180 insertions(+), 7 deletions(-)
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index f62f4dc1e..33221362c 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -151,10 +151,22 @@ local function normalize_uri(port)
return tostring(port);
end
+local function normalize_uri_list(port_list)
+ local ports_table = {}
+ if type(port_list) == 'table' then
+ for i in pairs(port_list) do
+ table.insert(ports_table, normalize_uri(port_list[i]))
+ end
+ else
+ table.insert(ports_table, normalize_uri(port_list))
+ end
+ return ports_table;
+end
+
-- options that require special handling
local modify_cfg = {
listen = normalize_uri,
- replication = normalize_uri,
+ replication = normalize_uri_list,
}
local function purge_password_from_uri(uri)
@@ -365,6 +377,28 @@ local function apply_default_cfg(cfg, default_cfg)
end
end
+-- Return true if two configurations are equivalent.
+local function compare_cfg(t1, t2)
+ if type(t1) ~= 'table' and type(t2) ~= 'table' then
+ return t1 == t2
+ end
+ if (type(t1) == 'table' and type(t2) ~= 'table') or
+ (type(t2) == 'table' and type(t1) ~= 'table') then
+ return false
+ end
+ if type(t1) == 'table' and type(t2) == 'table' then
+ if #t1 ~= #t2 then
+ return false
+ end
+ for i, j in pairs(t2) do
+ if t1[i] ~= j then
+ return false
+ end
+ end
+ return true
+ end
+end
+
local function reload_cfg(oldcfg, cfg)
cfg = upgrade_cfg(cfg, translate_cfg)
local newcfg = prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)
@@ -377,7 +411,7 @@ local function reload_cfg(oldcfg, cfg)
for key in pairs(cfg) do
local val = newcfg[key]
local oldval = oldcfg[key]
- if oldval ~= val then
+ if not compare_cfg(val, oldval) then
rawset(oldcfg, key, val)
if not pcall(dynamic_cfg[key]) then
rawset(oldcfg, key, oldval) -- revert the old value
@@ -452,7 +486,7 @@ local function load_cfg(cfg)
local val = cfg[key]
if val ~= nil and not dynamic_cfg_skip_at_load[key] then
fun()
- if val ~= default_cfg[key] then
+ if not compare_cfg(val, default_cfg[key]) then
log.info("set '%s' configuration option to %s", key, json.encode(val))
end
end
diff --git a/test/replication/errinj.result b/test/replication/errinj.result
index ce6add8d4..2e7d367c7 100644
--- a/test/replication/errinj.result
+++ b/test/replication/errinj.result
@@ -383,7 +383,7 @@ while box.info.replication[1].upstream ~= nil do fiber.sleep(0.0001) end
---
...
-- reconnect
-box.cfg{replication = {old_repl}}
+box.cfg{replication = old_repl}
---
...
while box.info.replication[1].upstream.status ~= 'disconnected' do fiber.sleep(0.0001) end
diff --git a/test/replication/errinj.test.lua b/test/replication/errinj.test.lua
index e1e96a0c4..32e0be912 100644
--- a/test/replication/errinj.test.lua
+++ b/test/replication/errinj.test.lua
@@ -160,7 +160,7 @@ old_repl = box.cfg.replication
box.cfg{replication = {}, replication_timeout = 0.1}
while box.info.replication[1].upstream ~= nil do fiber.sleep(0.0001) end
-- reconnect
-box.cfg{replication = {old_repl}}
+box.cfg{replication = old_repl}
while box.info.replication[1].upstream.status ~= 'disconnected' do fiber.sleep(0.0001) end
while box.info.replication[1].upstream.status ~= 'follow' do fiber.sleep(0.0001) end
diff --git a/test/replication/misc.result b/test/replication/misc.result
index f8aa8dab6..ec0a94c1d 100644
--- a/test/replication/misc.result
+++ b/test/replication/misc.result
@@ -413,7 +413,7 @@ test_run:cmd("switch replica")
---
- true
...
-replication = box.cfg.replication
+replication = box.cfg.replication[1]
---
...
box.cfg{replication = {replication, replication}}
@@ -467,3 +467,103 @@ test_run:cmd("delete server replica")
box.schema.user.revoke('guest', 'replication')
---
...
+--
+-- gh-3711 Do not restart replication on box.cfg if the configuration didn't change
+--
+box.schema.user.grant('guest', 'replication')
+---
+...
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+---
+- true
+...
+test_run:cmd("start server replica")
+---
+- true
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+replication = box.cfg.replication[1]
+---
+...
+test_run:cmd("switch default")
+---
+- true
+...
+-- Access rights are checked only during reconnect. If new config is equivalent
+-- to old one, replication will continue working.
+box.schema.user.revoke('guest', 'replication')
+---
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+box.cfg{replication = {replication}}
+---
+...
+box.info.status == 'running'
+---
+- true
+...
+box.cfg{replication = replication}
+---
+...
+box.info.status == 'running'
+---
+- true
+...
+-- check table
+test_run:cmd("switch default")
+---
+- true
+...
+box.schema.user.grant('guest', 'replication')
+---
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+replication = box.cfg.replication
+---
+...
+table.insert(replication, box.cfg.listen)
+---
+...
+test_run:cmd("switch default")
+---
+- true
+...
+box.schema.user.revoke('guest', 'replication')
+---
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+box.cfg{replication = replication}
+---
+...
+box.info.status == 'running'
+---
+- true
+...
+test_run:cmd("switch default")
+---
+- true
+...
+test_run:cmd("stop server replica")
+---
+- true
+...
+test_run:cmd("cleanup server replica")
+---
+- true
+...
+test_run:cmd("delete server replica")
+---
+- true
+...
diff --git a/test/replication/misc.test.lua b/test/replication/misc.test.lua
index 46726b7f4..cbdfc7cdd 100644
--- a/test/replication/misc.test.lua
+++ b/test/replication/misc.test.lua
@@ -171,7 +171,7 @@ box.schema.user.grant('guest', 'replication')
test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
test_run:cmd("start server replica")
test_run:cmd("switch replica")
-replication = box.cfg.replication
+replication = box.cfg.replication[1]
box.cfg{replication = {replication, replication}}
-- Check the case when duplicate connection is detected in the background.
@@ -191,3 +191,42 @@ test_run:cmd("stop server replica")
test_run:cmd("cleanup server replica")
test_run:cmd("delete server replica")
box.schema.user.revoke('guest', 'replication')
+
+
+--
+-- gh-3711 Do not restart replication on box.cfg if the configuration didn't change
+--
+box.schema.user.grant('guest', 'replication')
+
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+test_run:cmd("start server replica")
+test_run:cmd("switch replica")
+replication = box.cfg.replication[1]
+test_run:cmd("switch default")
+-- Access rights are checked only during reconnect. If new config is equivalent
+-- to old one, replication will continue working.
+box.schema.user.revoke('guest', 'replication')
+test_run:cmd("switch replica")
+box.cfg{replication = {replication}}
+box.info.status == 'running'
+box.cfg{replication = replication}
+box.info.status == 'running'
+
+-- check table
+test_run:cmd("switch default")
+box.schema.user.grant('guest', 'replication')
+test_run:cmd("switch replica")
+replication = box.cfg.replication
+table.insert(replication, box.cfg.listen)
+
+test_run:cmd("switch default")
+box.schema.user.revoke('guest', 'replication')
+
+test_run:cmd("switch replica")
+box.cfg{replication = replication}
+box.info.status == 'running'
+
+test_run:cmd("switch default")
+test_run:cmd("stop server replica")
+test_run:cmd("cleanup server replica")
+test_run:cmd("delete server replica")
--
2.14.3 (Apple Git-98)
More information about the Tarantool-patches
mailing list