[patches] [PATCH vshard 1/5] On reconfig update internals only after success box.cfg

[Vladislav Shpilevoy]

If box.cfg fails, a new configuration is applied partially. It
is incorrect.

Signed-off-by: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
---
 test/misc/reconfigure.result   | 76 ++++++++++++++++++++++++++++++++++++++++++
 test/misc/reconfigure.test.lua | 29 ++++++++++++++++
 vshard/router/init.lua         | 10 +++---
 vshard/storage/init.lua        | 23 ++++++++-----
 4 files changed, 126 insertions(+), 12 deletions(-)

diff --git a/test/misc/reconfigure.result b/test/misc/reconfigure.result
index f873299..0a014a3 100644
--- a/test/misc/reconfigure.result
+++ b/test/misc/reconfigure.result
@@ -93,6 +93,54 @@ util.check_error(vshard.storage.cfg, cfg, 'unknow uuid')
 - Local replica unknow uuid wasn't found in config
 ...
 --
+-- Ensure that in a case of error storage internals are not
+-- changed.
+--
+not vshard.storage.internal.collect_lua_garbage
+---
+- true
+...
+cfg.collect_lua_garbage = true
+---
+...
+cfg.rebalancer_max_receiving = 1000
+---
+...
+cfg.collect_bucket_garbage_interval = 100
+---
+...
+cfg.invalid_option = 'kek'
+---
+...
+vshard.storage.cfg(cfg, names.storage_1_a)
+---
+- error: 'Incorrect value for option ''invalid_option'': unexpected option'
+...
+not vshard.storage.internal.collect_lua_garbage
+---
+- true
+...
+vshard.storage.internal.rebalancer_max_receiving ~= 1000
+---
+- true
+...
+vshard.storage.internal.collect_bucket_garbage_interval ~= 100
+---
+- true
+...
+cfg.collect_lua_garbage = nil
+---
+...
+cfg.rebalancer_max_receiving = nil
+---
+...
+cfg.collect_bucket_garbage_interval = nil
+---
+...
+cfg.invalid_option = nil
+---
+...
+--
 -- gh-59: provide trigger on master enable/disable.
 --
 disable_count = 0
@@ -184,6 +232,34 @@ test_run:switch('router_1')
 ---
 - true
 ...
+--
+-- Ensure that in a case of error router internals are not
+-- changed.
+--
+not vshard.router.internal.collect_lua_garbage
+---
+- true
+...
+cfg.collect_lua_garbage = true
+---
+...
+cfg.invalid_option = 'kek'
+---
+...
+vshard.router.cfg(cfg)
+---
+- error: 'Incorrect value for option ''invalid_option'': unexpected option'
+...
+not vshard.router.internal.collect_lua_garbage
+---
+- true
+...
+cfg.invalid_option = nil
+---
+...
+cfg.collect_lua_garbage = nil
+---
+...
 vshard.router.cfg(cfg)
 ---
 ...
diff --git a/test/misc/reconfigure.test.lua b/test/misc/reconfigure.test.lua
index 65e7cef..033214a 100644
--- a/test/misc/reconfigure.test.lua
+++ b/test/misc/reconfigure.test.lua
@@ -36,6 +36,24 @@ test_run:cmd('switch storage_1_a')
 util = require('util')
 util.check_error(vshard.storage.cfg, cfg, 'unknow uuid')
 
+--
+-- Ensure that in a case of error storage internals are not
+-- changed.
+--
+not vshard.storage.internal.collect_lua_garbage
+cfg.collect_lua_garbage = true
+cfg.rebalancer_max_receiving = 1000
+cfg.collect_bucket_garbage_interval = 100
+cfg.invalid_option = 'kek'
+vshard.storage.cfg(cfg, names.storage_1_a)
+not vshard.storage.internal.collect_lua_garbage
+vshard.storage.internal.rebalancer_max_receiving ~= 1000
+vshard.storage.internal.collect_bucket_garbage_interval ~= 100
+cfg.collect_lua_garbage = nil
+cfg.rebalancer_max_receiving = nil
+cfg.collect_bucket_garbage_interval = nil
+cfg.invalid_option = nil
+
 --
 -- gh-59: provide trigger on master enable/disable.
 --
@@ -73,6 +91,17 @@ test_run:switch('storage_2_b')
 vshard.storage.cfg(cfg, names['storage_2_b'])
 
 test_run:switch('router_1')
+--
+-- Ensure that in a case of error router internals are not
+-- changed.
+--
+not vshard.router.internal.collect_lua_garbage
+cfg.collect_lua_garbage = true
+cfg.invalid_option = 'kek'
+vshard.router.cfg(cfg)
+not vshard.router.internal.collect_lua_garbage
+cfg.invalid_option = nil
+cfg.collect_lua_garbage = nil
 vshard.router.cfg(cfg)
 
 test_run:cmd('switch default')
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index c01cb5a..eb12edf 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -492,10 +492,8 @@ local function router_cfg(cfg)
         log.info('Starting router reconfiguration')
     end
     local new_replicasets = lreplicaset.buildall(cfg)
-    -- TODO: update existing route map in-place
-    M.route_map = {}
-    M.total_bucket_count = cfg.bucket_count
-    M.collect_lua_garbage = cfg.collect_lua_garbage
+    local total_bucket_count = cfg.bucket_count
+    local collect_lua_garbage = cfg.collect_lua_garbage
     lcfg.remove_non_box_options(cfg)
     -- Force net.box connection on cfg()
     for _, replicaset in pairs(new_replicasets) do
@@ -509,6 +507,10 @@ local function router_cfg(cfg)
     end
     box.cfg(cfg)
     log.info("Box has been configured")
+    M.total_bucket_count = total_bucket_count
+    M.collect_lua_garbage = collect_lua_garbage
+    -- TODO: update existing route map in-place
+    M.route_map = {}
     M.replicasets = new_replicasets
     if M.failover_fiber == nil then
         log.info('Start failover fiber')
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 7c978b8..408e559 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -1342,21 +1342,28 @@ local function storage_cfg(cfg, this_replica_uuid)
     end
     cfg.instance_uuid = this_replica.uuid
     cfg.replicaset_uuid = this_replicaset.uuid
-    M.total_bucket_count = cfg.bucket_count
-    M.rebalancer_disbalance_threshold = cfg.rebalancer_disbalance_threshold
-    M.rebalancer_max_receiving = cfg.rebalancer_max_receiving
-    M.shard_index = cfg.shard_index
-    M.collect_bucket_garbage_interval = cfg.collect_bucket_garbage_interval
-    M.collect_lua_garbage = cfg.collect_lua_garbage
+    local total_bucket_count = cfg.bucket_count
+    local rebalancer_disbalance_threshold = cfg.rebalancer_disbalance_threshold
+    local rebalancer_max_receiving = cfg.rebalancer_max_receiving
+    local shard_index = cfg.shard_index
+    local collect_bucket_garbage_interval = cfg.collect_bucket_garbage_interval
+    local collect_lua_garbage = cfg.collect_lua_garbage
     lcfg.remove_non_box_options(cfg)
 
     box.cfg(cfg)
     log.info("Box has been configured")
+    local uri = luri.parse(this_replica.uri)
+    box.once("vshard:storage:1", storage_schema_v1, uri.login, uri.password)
+
     M.replicasets = new_replicasets
     M.this_replicaset = this_replicaset
     M.this_replica = this_replica
-    local uri = luri.parse(this_replica.uri)
-    box.once("vshard:storage:1", storage_schema_v1, uri.login, uri.password)
+    M.total_bucket_count = total_bucket_count
+    M.rebalancer_disbalance_threshold = rebalancer_disbalance_threshold
+    M.rebalancer_max_receiving = rebalancer_max_receiving
+    M.shard_index = shard_index
+    M.collect_bucket_garbage_interval = collect_bucket_garbage_interval
+    M.collect_lua_garbage = collect_lua_garbage
 
     if was_master and not is_master then
         local_on_master_disable()
-- 
2.14.3 (Apple Git-98)