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 0BDB02D270 for ; Wed, 31 Oct 2018 07:32:34 -0400 (EDT) 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 JDKmB2BJZIVo for ; Wed, 31 Oct 2018 07:32:33 -0400 (EDT) Received: from smtp55.i.mail.ru (smtp55.i.mail.ru [217.69.128.35]) (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 BF90B2CEEB for ; Wed, 31 Oct 2018 07:32:33 -0400 (EDT) From: Olga Arkhangelskaia Subject: [tarantool-patches] [PATCH v3] box: fixed comparison of old and new config options Date: Wed, 31 Oct 2018 14:32:14 +0300 Message-Id: <20181031113214.79611-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-2.1 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 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 src/box/lua/load_cfg.lua | 39 +++++++++++++-- test/replication/misc.result | 106 +++++++++++++++++++++++++++++++++++++++++ test/replication/misc.test.lua | 41 ++++++++++++++++ 3 files changed, 183 insertions(+), 3 deletions(-) diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua index df0c3c6ae..0dcb62f26 100644 --- a/src/box/lua/load_cfg.lua +++ b/src/box/lua/load_cfg.lua @@ -151,10 +151,25 @@ local function normalize_uri(port) return tostring(port); end +local function normalize_uri_list(port) + if port == nil then + return port + end + if type(port) == 'table' then + if #port == 1 then + return tostring(port[1]) + end + for i in pairs(port) do + port[i] = tostring(port[i]) + end + return port + end + return tostring(port); +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 +380,24 @@ 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' 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 +410,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 +485,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/misc.result b/test/replication/misc.result index f8aa8dab6..c9de813da 100644 --- a/test/replication/misc.result +++ b/test/replication/misc.result @@ -467,3 +467,109 @@ 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 +--- +... +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 +... +listen = box.cfg.replication +--- +... +a = string.split(listen, ':') +--- +... +self = string.format('localhost:%s', a[2] + 1) +--- +... +box.cfg{listen = self, replication = {self, replication}} +--- +... +test_run:cmd("switch default") +--- +- true +... +box.schema.user.revoke('guest', 'replication') +--- +... +test_run:cmd("switch replica") +--- +- true +... +box.cfg{replication = {self, 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..bf7408e96 100644 --- a/test/replication/misc.test.lua +++ b/test/replication/misc.test.lua @@ -191,3 +191,44 @@ 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 +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") +listen = box.cfg.replication +a = string.split(listen, ':') +self = string.format('localhost:%s', a[2] + 1) +box.cfg{listen = self, replication = {self, replication}} + +test_run:cmd("switch default") +box.schema.user.revoke('guest', 'replication') + +test_run:cmd("switch replica") +box.cfg{replication = {self, 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)