From: Olga Arkhangelskaia <arkholga@tarantool.org> To: tarantool-patches@freelists.org Cc: Olga Arkhangelskaia <arkholga@tarantool.org> Subject: [tarantool-patches] [PATCH] box: fixed comparison of old and new config options Date: Wed, 24 Oct 2018 13:14:31 +0300 [thread overview] Message-ID: <20181024101431.32625-1-arkholga@tarantool.org> (raw) 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)
next reply other threads:[~2018-10-24 10:14 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-24 10:14 Olga Arkhangelskaia [this message] 2018-10-29 10:21 ` Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20181024101431.32625-1-arkholga@tarantool.org \ --to=arkholga@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] [PATCH] box: fixed comparison of old and new config options' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox