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 5DBCE2D091 for ; Tue, 30 Oct 2018 08:04:46 -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 OqbDMupL4Vz9 for ; Tue, 30 Oct 2018 08:04:46 -0400 (EDT) Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (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 9F1162D062 for ; Tue, 30 Oct 2018 08:04:45 -0400 (EDT) From: Olga Arkhangelskaia Subject: [tarantool-patches] [PATCH v2] box: fixed comparison of old and new config options Date: Tue, 30 Oct 2018 15:04:23 +0300 Message-Id: <20181030120423.69490-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 Due to incorrect configuration comparison, some options, especially replication, were reseted and restarted, despite that configurations were equivalent. 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 Changes in v2: - changed test - conversion from num to string now in modify_cfg - no sort table in comparison - got rid of table.getn src/box/lua/load_cfg.lua | 33 +++++++++++++++++++-- test/replication/misc.result | 65 ++++++++++++++++++++++++++++++++++++++++++ test/replication/misc.test.lua | 24 ++++++++++++++++ 3 files changed, 119 insertions(+), 3 deletions(-) diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua index df0c3c6ae..6d3c7f62c 100644 --- a/src/box/lua/load_cfg.lua +++ b/src/box/lua/load_cfg.lua @@ -145,7 +145,11 @@ local template_cfg = { } local function normalize_uri(port) - if port == nil or type(port) == 'table' then + if port == nil then return port end + if type (port) == 'table' then + for i in pairs(port) do + port[i] = tostring(port[i]) + end return port end return tostring(port); @@ -365,6 +369,29 @@ local function apply_default_cfg(cfg, default_cfg) end end +-- check whether two configurations are equivalent, returns true, if yes, +-- false otherwise. +local function equivalent (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 == 1 then return t1[1] == t2 end + return false + end + if type(t2) == "table" and type(t1) ~= "table" then + if #t2 == 1 then return t2[1] == t1 end + return false + end + if type(t1) == 'table' and type(t2) == 'table' then + if #t1 ~= #t2 then return false end + for i in pairs(t2) do + if equivalent(t1[i], t2[i]) == false 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 +404,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 equivalent(val, oldval) then rawset(oldcfg, key, val) if not pcall(dynamic_cfg[key]) then rawset(oldcfg, key, oldval) -- revert the old value @@ -452,7 +479,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 equivalent(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..2a1e9cffe 100644 --- a/test/replication/misc.result +++ b/test/replication/misc.result @@ -467,3 +467,68 @@ 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 +... +box.schema.user.revoke('guest', 'replication') +--- +... +test_run:cmd("switch replica") +--- +- true +... +box.cfg{replication_sync_timeout = 0.5} +--- +... +box.cfg{replication = {replication}} +--- +... +box.info.status == 'running' +--- +- 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..7a3020f6e 100644 --- a/test/replication/misc.test.lua +++ b/test/replication/misc.test.lua @@ -191,3 +191,27 @@ 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") +box.schema.user.revoke('guest', 'replication') +test_run:cmd("switch replica") +box.cfg{replication_sync_timeout = 0.5} +box.cfg{replication = {replication}} +box.info.status == 'running' +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)