From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 3D7842EABF for ; Mon, 5 Nov 2018 04:01:45 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1H7gz-L0TqQB for ; Mon, 5 Nov 2018 04:01:45 -0500 (EST) Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 519662EAC2 for ; Mon, 5 Nov 2018 04:01:44 -0500 (EST) From: Olga Arkhangelskaia Subject: [tarantool-patches] [PATCH v4] box: fixed comparison of old and new config options Date: Mon, 5 Nov 2018 11:57:49 +0300 Message-Id: <20181105085749.41482-1-arkholga@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Olga Arkhangelskaia 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)