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 586872D66E for ; Wed, 24 Oct 2018 06:14: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 LHCBMkRK8eh2 for ; Wed, 24 Oct 2018 06:14:46 -0400 (EDT) Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (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 A4F9626B63 for ; Wed, 24 Oct 2018 06:14:45 -0400 (EDT) From: Olga Arkhangelskaia Subject: [tarantool-patches] [PATCH] box: fixed comparison of old and new config options Date: Wed, 24 Oct 2018 13:14:31 +0300 Message-Id: <20181024101431.32625-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 change did not actually happened. The patch fixes it. 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 src/box/lua/load_cfg.lua | 43 +++++++++++++++-- test/replication/misc.result | 106 +++++++++++++++++++++++++++++++++++++++++ test/replication/misc.test.lua | 38 +++++++++++++++ 3 files changed, 183 insertions(+), 4 deletions(-) diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua index df0c3c6ae..98d7ff47c 100644 --- a/src/box/lua/load_cfg.lua +++ b/src/box/lua/load_cfg.lua @@ -365,6 +365,41 @@ local function apply_default_cfg(cfg, default_cfg) end end +-- Compare given config with old one. +-- Returns true if new config is similar to old one, false otherwise. +local function compare_cfg(cfg1, cfg2) + if type(cfg1) ~= "table" and type(cfg2) ~= "table" then + return cfg1 == cfg2 + end + if type(cfg1) == "table" and type(cfg2) ~= "table" then + if table.getn(cfg1) ~= 1 then + return false + else + if type(cfg1[1]) ~= "nil" and type(cfg2) ~= "nil" then + return string.formagt(cfg1[1]) == string.format(cfg2) + else return cfg1[1] == cfg2 end + end + end + if type(cfg2) == "table" and type(cfg1) ~= "table" then + if table.getn(cfg2) ~= 1 then + return false + else + if type(cfg2[1]) ~= "nil" and type(cfg1) ~= "nil" then + return string.format(cfg2[1]) == string.format(cfg1) + else return cfg2[1] == cfg1 end + end + end + if type(cfg1) == "table" and type(cfg2) == "table" then + if table.getn(cfg1) ~= table.getn(cfg2) then return false end + table.sort(cfg1) + table.sort(cfg2) + for key in pairs(cfg1) do + if not compare_cfg(cfg1[key], cfg2[key]) 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) @@ -375,9 +410,9 @@ local function reload_cfg(oldcfg, cfg) end end for key in pairs(cfg) do - local val = newcfg[key] - local oldval = oldcfg[key] - if oldval ~= val then + if not compare_cfg(oldcfg[key], newcfg[key]) then + local val = newcfg[key] + local oldval = oldcfg[key] rawset(oldcfg, key, val) if not pcall(dynamic_cfg[key]) then rawset(oldcfg, key, oldval) -- revert the old value @@ -388,7 +423,7 @@ local function reload_cfg(oldcfg, cfg) end log.info("set '%s' configuration option to %s", key, json.encode(val)) - end + else log.info("option '%s' has not change", key) end end if type(box.on_reload_configuration) == 'function' then box.on_reload_configuration() diff --git a/test/replication/misc.result b/test/replication/misc.result index f8aa8dab6..c8e50d8f9 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 +... +test_run:grep_log('replica', 'replication') +--- +- replication +... +replication = box.cfg.replication +--- +... +box.cfg{replication = {replication}} +--- +... +test_run:grep_log('replica', 'has not change') ~= nil +--- +- true +... +test_run:grep_log('replica', "set 'replication' configuration option to") == nil +--- +- true +... +box.cfg{replication = replication} +--- +... +test_run:grep_log('replica', "set 'replication' configuration option to") == nil +--- +- true +... +test_run:cmd("switch default") +--- +- true +... +test_run:cmd('stop server replica') +--- +- true +... +test_run:cmd('cleanup server test') +--- +- true +... +test_run:cmd("start server replica") +--- +- true +... +test_run:cmd("switch replica") +--- +- true +... +listen = box.cfg.listen +--- +... +replication = box.cfg.replication +--- +... +box.cfg{replication = {replication, listen}} +--- +... +test_run:grep_log('replica', "set 'replication' configuration option to") +--- +- set 'replication' configuration option to +... +test_run:grep_log('replica', "has not change") == nil +--- +- true +... +box.cfg{replication = {listen, replication}} +--- +... +test_run:grep_log('replica', "has not change") ~= nil +--- +- 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 +... +box.schema.user.revoke('guest', 'replication') +--- +... diff --git a/test/replication/misc.test.lua b/test/replication/misc.test.lua index 46726b7f4..96274b263 100644 --- a/test/replication/misc.test.lua +++ b/test/replication/misc.test.lua @@ -191,3 +191,41 @@ 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") + +test_run:grep_log('replica', 'replication') +replication = box.cfg.replication +box.cfg{replication = {replication}} +test_run:grep_log('replica', 'has not change') ~= nil +test_run:grep_log('replica', "set 'replication' configuration option to") == nil +box.cfg{replication = replication} +test_run:grep_log('replica', "set 'replication' configuration option to") == nil + +test_run:cmd("switch default") +test_run:cmd('stop server replica') +test_run:cmd('cleanup server test') +test_run:cmd("start server replica") +test_run:cmd("switch replica") + +listen = box.cfg.listen +replication = box.cfg.replication + +box.cfg{replication = {replication, listen}} +test_run:grep_log('replica', "set 'replication' configuration option to") +test_run:grep_log('replica', "has not change") == nil +box.cfg{replication = {listen, replication}} +test_run:grep_log('replica', "has not change") ~= nil + +test_run:cmd("switch default") +test_run:cmd("stop server replica") +test_run:cmd("cleanup server replica") +test_run:cmd("delete server replica") +box.schema.user.revoke('guest', 'replication') -- 2.14.3 (Apple Git-98)