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 D033E278EF for ; Fri, 20 Jul 2018 07:34:44 -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 RVlRZtCQbDPK for ; Fri, 20 Jul 2018 07:34:44 -0400 (EDT) Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 8ADB327D32 for ; Fri, 20 Jul 2018 07:34:44 -0400 (EDT) Received: by smtp60.i.mail.ru with esmtpa (envelope-from ) id 1fgTgI-0003gc-HM for tarantool-patches@freelists.org; Fri, 20 Jul 2018 14:34:42 +0300 Subject: [tarantool-patches] Re: [PATCH 1/3] Add test on error during reconfigure References: <0a1a6702a293c8f86f38d67c92f04da1f6c6a686.1531935745.git.avkhatskevich@tarantool.org> <069d5b60-cffa-c24c-06f9-9a9af1af8fcb@tarantool.org> <95eccf6c-67fc-d9a5-f3d7-a5b276d1ea88@tarantool.org> From: Alex Khatskevich Message-ID: <10bcefd6-a696-41fa-3598-900cf96c32a3@tarantool.org> Date: Fri, 20 Jul 2018 14:34:36 +0300 MIME-Version: 1.0 In-Reply-To: <95eccf6c-67fc-d9a5-f3d7-a5b276d1ea88@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US 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 Here is a full diff: commit fd68ec9454ec6b5552a37d15bb3adb33cbf1ea84 Author: AKhatskevich Date:   Sat Jun 9 19:50:52 2018 +0300     Add test on error during reconfigure     In case reconfigure process fails, the node should continue     work properly. diff --git a/test/lua_libs/util.lua b/test/lua_libs/util.lua index f2d3b48..2d866df 100644 --- a/test/lua_libs/util.lua +++ b/test/lua_libs/util.lua @@ -69,9 +69,30 @@ local function wait_master(test_run, replicaset, master)      log.info('Slaves are connected to a master "%s"', master)  end +-- Check that data has at least all etalon's fields and they are +-- equal. +-- @param etalon Table which fields should be found in `data`. +-- @param data Table which is checked against `etalon`. +-- @return Boolean indicator of equality. +-- @return Table of names of fields which are different in `data`. +local function has_same_fields(etalon, data) +    assert(type(etalon) == 'table' and type(data) == 'table') +    local diff = {} +    for k, v in pairs(etalon) do +        if v ~= data[k] then +            table.insert(diff, k) +        end +    end +    if #diff > 0 then +        return false, diff +    end +    return true +end +  return {      check_error = check_error,      shuffle_masters = shuffle_masters,      collect_timeouts = collect_timeouts,      wait_master = wait_master, +    has_same_fields = has_same_fields,  } diff --git a/test/router/router.result b/test/router/router.result index 15f4fd0..4919962 100644 --- a/test/router/router.result +++ b/test/router/router.result @@ -1156,6 +1156,36 @@ util.check_error(vshard.router.cfg, non_dynamic_cfg)  ---  - Non-dynamic option shard_index cannot be reconfigured  ... +-- Error during reconfigure process. +vshard.router.route(1):callro('echo', {'some_data'}) +--- +- some_data +- null +- null +... +vshard.router.internal.errinj.ERRINJ_CFG = true +--- +... +old_internal = table.copy(vshard.router.internal) +--- +... +util.check_error(vshard.router.cfg, cfg) +--- +- 'Error injection: cfg' +... +vshard.router.internal.errinj.ERRINJ_CFG = false +--- +... +util.has_same_fields(old_internal, vshard.router.internal) +--- +- true +... +vshard.router.route(1):callro('echo', {'some_data'}) +--- +- some_data +- null +- null +...  _ = test_run:cmd("switch default")  ---  ... diff --git a/test/router/router.test.lua b/test/router/router.test.lua index 8006e5d..df2f381 100644 --- a/test/router/router.test.lua +++ b/test/router/router.test.lua @@ -444,6 +444,15 @@ non_dynamic_cfg = table.copy(cfg)  non_dynamic_cfg.shard_index = 'non_default_name'  util.check_error(vshard.router.cfg, non_dynamic_cfg) +-- Error during reconfigure process. +vshard.router.route(1):callro('echo', {'some_data'}) +vshard.router.internal.errinj.ERRINJ_CFG = true +old_internal = table.copy(vshard.router.internal) +util.check_error(vshard.router.cfg, cfg) +vshard.router.internal.errinj.ERRINJ_CFG = false +util.has_same_fields(old_internal, vshard.router.internal) +vshard.router.route(1):callro('echo', {'some_data'}) +  _ = test_run:cmd("switch default")  test_run:drop_cluster(REPLICASET_2) diff --git a/test/storage/storage.result b/test/storage/storage.result index 4399ff0..ff07fe9 100644 --- a/test/storage/storage.result +++ b/test/storage/storage.result @@ -732,6 +732,45 @@ util.check_error(vshard.storage.cfg, non_dynamic_cfg, names.storage_1_a)  ---  - Non-dynamic option bucket_count cannot be reconfigured  ... +-- Error during reconfigure process. +_, rs = next(vshard.storage.internal.replicasets) +--- +... +rs:callro('echo', {'some_data'}) +--- +- some_data +- null +- null +... +vshard.storage.internal.errinj.ERRINJ_CFG = true +--- +... +old_internal = table.copy(vshard.storage.internal) +--- +... +_, err = pcall(vshard.storage.cfg, cfg, names.storage_1_a) +--- +... +err:match('Error injection:.*') +--- +- 'Error injection: cfg' +... +vshard.storage.internal.errinj.ERRINJ_CFG = false +--- +... +util.has_same_fields(old_internal, vshard.storage.internal) +--- +- true +... +_, rs = next(vshard.storage.internal.replicasets) +--- +... +rs:callro('echo', {'some_data'}) +--- +- some_data +- null +- null +...  _ = test_run:cmd("switch default")  ---  ... diff --git a/test/storage/storage.test.lua b/test/storage/storage.test.lua index 72564e1..04bb608 100644 --- a/test/storage/storage.test.lua +++ b/test/storage/storage.test.lua @@ -182,6 +182,18 @@ non_dynamic_cfg = table.copy(cfg)  non_dynamic_cfg.bucket_count = require('vshard.consts').DEFAULT_BUCKET_COUNT + 1  util.check_error(vshard.storage.cfg, non_dynamic_cfg, names.storage_1_a) +-- Error during reconfigure process. +_, rs = next(vshard.storage.internal.replicasets) +rs:callro('echo', {'some_data'}) +vshard.storage.internal.errinj.ERRINJ_CFG = true +old_internal = table.copy(vshard.storage.internal) +_, err = pcall(vshard.storage.cfg, cfg, names.storage_1_a) +err:match('Error injection:.*') +vshard.storage.internal.errinj.ERRINJ_CFG = false +util.has_same_fields(old_internal, vshard.storage.internal) +_, rs = next(vshard.storage.internal.replicasets) +rs:callro('echo', {'some_data'}) +  _ = test_run:cmd("switch default")  test_run:drop_cluster(REPLICASET_2) diff --git a/vshard/router/init.lua b/vshard/router/init.lua index 4531f3a..a143070 100644 --- a/vshard/router/init.lua +++ b/vshard/router/init.lua @@ -11,6 +11,7 @@ local M = rawget(_G, '__module_vshard_router')  if not M then      M = {          errinj = { +            ERRINJ_CFG = false,              ERRINJ_FAILOVER_CHANGE_CFG = false,              ERRINJ_RELOAD = false,              ERRINJ_LONG_DISCOVERY = false, @@ -486,6 +487,12 @@ local function router_cfg(cfg)      for k, v in pairs(cfg) do          log.info({[k] = v})      end +    -- It is considered that all possible errors during cfg +    -- process occur only before this place. +    -- This check should be placed as late as possible. +    if M.errinj.ERRINJ_CFG then +        error('Error injection: cfg') +    end      box.cfg(cfg)      log.info("Box has been configured")      M.total_bucket_count = total_bucket_count diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index ff204a4..052e94f 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -33,6 +33,7 @@ if not M then          -- Bucket count stored on all replicasets.          total_bucket_count = 0,          errinj = { +            ERRINJ_CFG = false,              ERRINJ_BUCKET_FIND_GARBAGE_DELAY = false,              ERRINJ_RELOAD = false,              ERRINJ_CFG_DELAY = false, @@ -1560,6 +1561,14 @@ local function storage_cfg(cfg, this_replica_uuid)      local shard_index = cfg.shard_index      local collect_bucket_garbage_interval = cfg.collect_bucket_garbage_interval      local collect_lua_garbage = cfg.collect_lua_garbage + +    -- It is considered that all possible errors during cfg +    -- process occur only before this place. +    -- This check should be placed as late as possible. +    if M.errinj.ERRINJ_CFG then +        error('Error injection: cfg') +    end +      --      -- Sync timeout is a special case - it must be updated before      -- all other options to allow a user to demote a master with