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 E6EF026646 for ; Fri, 15 Jun 2018 10:02:41 -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 awLI3YAvVF-g for ; Fri, 15 Jun 2018 10:02:41 -0400 (EDT) Received: from smtp21.mail.ru (smtp21.mail.ru [94.100.179.250]) (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 47B85217F3 for ; Fri, 15 Jun 2018 10:02:41 -0400 (EDT) From: AKhatskevich Subject: [tarantool-patches] [PATCH][vshard] Prohibit reconfigure non-dynamic options Date: Fri, 15 Jun 2018 17:02:25 +0300 Message-Id: <20180615140225.2576-1-avkhatskevich@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit 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, v.shpilevoy@tarantool.org By now there are two non-dynamic options: * bucket_count * shard_index They cannot be changed by passing a new value to router/storage.cfg. Closes #114 --- Branch: https://github.com/tarantool/vshard/tree/kh/gh-114-non-dynamic Issue: https://github.com/tarantool/vshard/issues/114 test/router/router.result | 14 ++++++++++++++ test/router/router.test.lua | 6 ++++++ test/storage/storage.result | 14 ++++++++++++++ test/storage/storage.test.lua | 6 ++++++ test/unit/config.result | 20 ++++++++++++++++++++ test/unit/config.test.lua | 8 ++++++++ vshard/cfg.lua | 19 +++++++++++++++++-- vshard/router/init.lua | 4 +++- vshard/storage/init.lua | 4 +++- 9 files changed, 91 insertions(+), 4 deletions(-) diff --git a/test/router/router.result b/test/router/router.result index 2ee1bff..1aacc0e 100644 --- a/test/router/router.result +++ b/test/router/router.result @@ -1057,6 +1057,20 @@ error_messages - - Use replica:is_connected(...) instead of replica.is_connected(...) - Use replica:safe_uri(...) instead of replica.safe_uri(...) ... +-- gh-114: Check non-dynamic option change during reconfigure. +non_dynamic_cfg = table.copy(cfg) +--- +... +non_dynamic_cfg.shard_index = 'non_default_name' +--- +... +_, err = pcall(vshard.router.cfg, non_dynamic_cfg) +--- +... +err:match('Non%-dynamic.*') +--- +- Non-dynamic option shard_index is changed in new vshard config +... _ = test_run:cmd("switch default") --- ... diff --git a/test/router/router.test.lua b/test/router/router.test.lua index fae8e24..f3b18b1 100644 --- a/test/router/router.test.lua +++ b/test/router/router.test.lua @@ -389,6 +389,12 @@ end; test_run:cmd("setopt delimiter ''"); error_messages +-- gh-114: Check non-dynamic option change during reconfigure. +non_dynamic_cfg = table.copy(cfg) +non_dynamic_cfg.shard_index = 'non_default_name' +_, err = pcall(vshard.router.cfg, non_dynamic_cfg) +err:match('Non%-dynamic.*') + _ = test_run:cmd("switch default") test_run:drop_cluster(REPLICASET_2) diff --git a/test/storage/storage.result b/test/storage/storage.result index d0bf792..41e3af8 100644 --- a/test/storage/storage.result +++ b/test/storage/storage.result @@ -720,6 +720,20 @@ test_run:cmd("setopt delimiter ''"); --- - true ... +-- gh-114: Check non-dynamic option change during reconfigure. +non_dynamic_cfg = table.copy(cfg) +--- +... +non_dynamic_cfg.shard_index = 'non_default_name' +--- +... +_, err = pcall(vshard.storage.cfg, non_dynamic_cfg, names.storage_1_a) +--- +... +err:match('Non%-dynamic.*') +--- +- Non-dynamic option shard_index is changed in new vshard config +... _ = test_run:cmd("switch default") --- ... diff --git a/test/storage/storage.test.lua b/test/storage/storage.test.lua index f4bbf0e..4f3faf6 100644 --- a/test/storage/storage.test.lua +++ b/test/storage/storage.test.lua @@ -177,6 +177,12 @@ for _, new_replicaset in pairs(new_replicasets) do end; test_run:cmd("setopt delimiter ''"); +-- gh-114: Check non-dynamic option change during reconfigure. +non_dynamic_cfg = table.copy(cfg) +non_dynamic_cfg.shard_index = 'non_default_name' +_, err = pcall(vshard.storage.cfg, non_dynamic_cfg, names.storage_1_a) +err:match('Non%-dynamic.*') + _ = test_run:cmd("switch default") test_run:drop_cluster(REPLICASET_2) diff --git a/test/unit/config.result b/test/unit/config.result index 6b4f87b..0d8c740 100644 --- a/test/unit/config.result +++ b/test/unit/config.result @@ -506,3 +506,23 @@ _ = lcfg.check(cfg) replica.name = 'storage' --- ... +-- gh-114: Check non-dynamic option change during reconfigure. +non_dynamic_cfg = table.copy(cfg) +--- +... +non_dynamic_cfg_old = table.copy(cfg) +--- +... +non_dynamic_cfg.shard_index = nil +--- +... +non_dynamic_cfg_old.shard_index = 'non_default_name' +--- +... +_, err = pcall(lcfg.check, non_dynamic_cfg, non_dynamic_cfg_old) +--- +... +err:match('Non%-dynamic.*') +--- +- Non-dynamic option shard_index is changed in new vshard config +... diff --git a/test/unit/config.test.lua b/test/unit/config.test.lua index 0f549d3..972df65 100644 --- a/test/unit/config.test.lua +++ b/test/unit/config.test.lua @@ -206,3 +206,11 @@ _ = lcfg.check(cfg) replica.name = nil _ = lcfg.check(cfg) replica.name = 'storage' + +-- gh-114: Check non-dynamic option change during reconfigure. +non_dynamic_cfg = table.copy(cfg) +non_dynamic_cfg_old = table.copy(cfg) +non_dynamic_cfg.shard_index = nil +non_dynamic_cfg_old.shard_index = 'non_default_name' +_, err = pcall(lcfg.check, non_dynamic_cfg, non_dynamic_cfg_old) +err:match('Non%-dynamic.*') diff --git a/vshard/cfg.lua b/vshard/cfg.lua index f5db4c0..8d26104 100644 --- a/vshard/cfg.lua +++ b/vshard/cfg.lua @@ -222,14 +222,29 @@ local cfg_template = { -- -- Check sharding config on correctness. Check types, name and uri --- uniqueness, master count (in each replicaset must by <= 1). +-- uniqueness, master count (in each replicaset must be <= 1). -- -local function cfg_check(shard_cfg) +local function cfg_check(shard_cfg, old_cfg) if type(shard_cfg) ~= 'table' then error('Сonfig must be map of options') end shard_cfg = table.deepcopy(shard_cfg) validate_config(shard_cfg, cfg_template) + if not old_cfg then + return shard_cfg + end + assert(not old_cfg or type(old_cfg) == 'table') + -- Check non-dynamic after default values are added. + local non_dynamic = { + 'bucket_count', 'shard_index' + } + for _, f_name in pairs(non_dynamic) do + -- New option may be added in new vshard verion. + if old_cfg[f_name] and shard_cfg[f_name] ~= old_cfg[f_name] then + error(string.format('Non-dynamic option %s ' .. + 'is changed in new vshard config', f_name)) + end + end return shard_cfg end diff --git a/vshard/router/init.lua b/vshard/router/init.lua index 21093e5..f29998a 100644 --- a/vshard/router/init.lua +++ b/vshard/router/init.lua @@ -457,7 +457,8 @@ end -------------------------------------------------------------------------------- local function router_cfg(cfg) - cfg = lcfg.check(cfg) + cfg = lcfg.check(cfg, M.current_cfg) + local new_cfg = table.deepcopy(cfg) if not M.replicasets then log.info('Starting router configuration') else @@ -478,6 +479,7 @@ local function router_cfg(cfg) -- TODO: update existing route map in-place M.route_map = {} M.replicasets = new_replicasets + M.current_cfg = new_cfg -- Move connections from an old configuration to a new one. -- It must be done with no yields to prevent usage both of not -- fully moved old replicasets, and not fully built new ones. diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index 57076e1..4c5d771 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -1448,7 +1448,8 @@ local function storage_cfg(cfg, this_replica_uuid) if this_replica_uuid == nil then error('Usage: cfg(configuration, this_replica_uuid)') end - cfg = lcfg.check(cfg) + cfg = lcfg.check(cfg, M.current_cfg) + local new_cfg = table.deepcopy(cfg) if cfg.weights or cfg.zone then error('Weights and zone are not allowed for storage configuration') end @@ -1574,6 +1575,7 @@ local function storage_cfg(cfg, this_replica_uuid) M.shard_index = shard_index M.collect_bucket_garbage_interval = collect_bucket_garbage_interval M.collect_lua_garbage = collect_lua_garbage + M.current_cfg = new_cfg if was_master and not is_master then local_on_master_disable() -- 2.14.1