[tarantool-patches] [PATCH v2] box: fixed comparison of old and new config options

[Olga Arkhangelskaia <arkholga@tarantool.org>]

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)