From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Alex Khatskevich <avkhatskevich@tarantool.org>, tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH][vshard] Prohibit reconfigure non-dynamic options Date: Tue, 26 Jun 2018 14:11:48 +0300 [thread overview] Message-ID: <9d1392dc-c923-4e49-b3b5-41ad7d099ccd@tarantool.org> (raw) In-Reply-To: <f334db42-6113-023d-9ec7-a8731c4ba864@tarantool.org> Hello. Thanks for the fixes! See 4 comments below. >> 2. Same. And try to test here bucket_count instead of shard_index. > fixed 1. Not fixed. I do not see a test for bucket_count. > > commit 38d504e5d91835916618c743ba3b34ca0fd7ca22 > Author: AKhatskevich <avkhatskevich@tarantool.org> > Date: Fri Jun 15 16:53:06 2018 +0300 > > Prohibit reconfigure non-dynamic options > > 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 > > diff --git a/test/router/router.result b/test/router/router.result > index 2ee1bff..5940e45 100644 > --- a/test/router/router.result > +++ b/test/router/router.result > @@ -1057,6 +1057,17 @@ 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' > +--- > +... > +util.check_error(vshard.router.cfg, non_dynamic_cfg) > +--- > +- Non-dynamic option shard_index is changed in new vshard config 2. This does not look like an error message. What is wrong here? Please, rephrase to make it clear what is wrong. > +... > _ = test_run:cmd("switch default") > --- > ...> diff --git a/vshard/cfg.lua b/vshard/cfg.lua > index f5db4c0..f9d6d52 100644 > --- a/vshard/cfg.lua > +++ b/vshard/cfg.lua > @@ -220,16 +220,34 @@ local cfg_template = { > }}, > } > > +-- > +-- Names of options which cnnot be changed during reconfigure. 3. Typo. > +-- > +local non_dynamic_options = { > + 'bucket_count', 'shard_index' > +} > + > -- > -- 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 > + -- Check non-dynamic after default values are added. > + for _, f_name in pairs(non_dynamic_options) do > + -- New option may be added in new vshard version. > + if old_cfg[f_name] and shard_cfg[f_name] ~= old_cfg[f_name] then 4. See point 8 from the previous review. All the same. > + error(string.format('Non-dynamic option %s ' .. > + 'is changed in new vshard config', f_name)) > + end > + end > return shard_cfg > end >
next prev parent reply other threads:[~2018-06-26 11:11 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-06-15 14:02 [tarantool-patches] " AKhatskevich 2018-06-21 14:26 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-25 11:48 ` Alex Khatskevich 2018-06-26 11:11 ` Vladislav Shpilevoy [this message] 2018-06-26 15:24 ` Alex Khatskevich 2018-06-27 12:03 ` Vladislav Shpilevoy 2018-06-27 12:59 ` Alex Khatskevich 2018-06-27 19:25 ` Alex Khatskevich 2018-06-28 11:06 ` Alex Khatskevich 2018-06-29 11:27 ` Vladislav Shpilevoy
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=9d1392dc-c923-4e49-b3b5-41ad7d099ccd@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=avkhatskevich@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH][vshard] Prohibit reconfigure non-dynamic 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