From: Olga Arkhangelskaia <arkholga@tarantool.org> To: tarantool-patches@freelists.org Cc: Olga Arkhangelskaia <arkholga@tarantool.org> Subject: [tarantool-patches] [PATCH v4] box: fixed comparison of old and new config options Date: Mon, 5 Nov 2018 11:57:49 +0300 [thread overview] Message-ID: <20181105085749.41482-1-arkholga@tarantool.org> (raw) 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)
next reply other threads:[~2018-11-05 9:01 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-05 8:57 Olga Arkhangelskaia [this message] 2018-11-19 10:39 ` 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=20181105085749.41482-1-arkholga@tarantool.org \ --to=arkholga@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] [PATCH v4] 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