Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alex Khatskevich <avkhatskevich@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
	tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH 3/3] Introduce storage reload evolution
Date: Sat, 21 Jul 2018 02:58:33 +0300	[thread overview]
Message-ID: <a543a03b-a673-1739-9c7c-11f6832d64e2@tarantool.org> (raw)
In-Reply-To: <d7eec7a3-3531-e6f4-c367-a3191e1bcb7e@tarantool.org>


>>> 11. Just use a symbolic link to the existing storage_1_a. Same
>>> for other storages.
>>>
>>> 12. You do not need a new test suite for one storage test. Please,
>>> put it into test/storage/
>> 11-12:
>> I have modified storage_1_a to change luapath.
>> It is important to change path here, because the default Vshard 
>> initialization is performed here.
>
> 1. Then please explain why do you need in storage_1_a.lua
> * two "require('fio')";
> * '-- Check if we are running under test-run';
> * 'names' table, 'replicasets' table;
> * 'box.once("testapp:schema:1", function()';
> * ...many other things.
>
> If you do not need them, then please, cleanup your storage_1_a.lua
> and extract wait_rebalancer_state into test utils since it is now
> duplicated in storage/ and reload_evolution/ tests.
>
I have reused some code. Extra commit was added.
>>>
>>>> diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
>>>> index bf560e6..1740c98 100644
>>>> --- a/vshard/storage/init.lua
>>>> +++ b/vshard/storage/init.lua
>>>> @@ -105,6 +110,11 @@ if not M then
>>>>           -- a destination replicaset must drop already received
>>>>           -- data.
>>>>           rebalancer_sending_bucket = 0,
>>>> +
>>>> +        ------------------------- Reload -------------------------
>>>> +        -- Version of the loaded module. This number is used on
>>>> +        -- reload to determine which upgrade scripts to run.
>>>> +        reload_evolution_version = reload_evolution.version,
>>>
>>> 13. Use 'version' name.
>> But it is not a version. Version is a git tag.
>> We already have module_version and git tag. To make everything clear 
>> I would like to
>> name the variable in that or similar way.
>
> 2. 'git tag' works only when you have git. But when you install vshard 
> from a package,
> the system can have no git. Moreover, it contradicts with tarantool 
> versioning, that
> exposes the actual version via _schema and box.info.
>
> So please, use real version for this member and name it 'version'. You 
> can find
> the code to extract, store and compare versions in box/lua/upgrade.lua.
>
> 3. It is not the same as git tag. We can add here a new version and 
> tag it
> later, like it is done in tarantool, when we use the new version 
> everywhere
> in the new code, but release it when ready.
>
> For example, now vshard has version 1.2.3. We change M and set version in
> evolution of M in the 1.2.4 constant. When 1.2.4 is ready, we do git tag
> 1.2.4.
>
>
> 4. You are trying to make you life easier, but do not think about
> users. It is not useful when you are not able to get the actual version,
> because you can not use git, or you see the strange versions like 1, 
> 2, 3, 4 ...
> Nobody uses natural numbers as versions. It is weird.
>
> I think, we should consult Kostja N. for this though.
After the discussion and implementation I will create another tread for 
this patch.

      reply	other threads:[~2018-07-20 23:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-18 17:47 [tarantool-patches] [PATCH 0/3] vshard reload mechanism AKhatskevich
2018-07-18 17:47 ` [tarantool-patches] [PATCH 1/3] Add test on error during reconfigure AKhatskevich
2018-07-19 15:14   ` [tarantool-patches] " Vladislav Shpilevoy
2018-07-19 20:33     ` Alex Khatskevich
2018-07-20 11:34       ` Alex Khatskevich
2018-07-20 14:15         ` Vladislav Shpilevoy
2018-07-18 17:47 ` [tarantool-patches] [PATCH 2/3] Complete module reload AKhatskevich
2018-07-19 15:14   ` [tarantool-patches] " Vladislav Shpilevoy
2018-07-19 20:32     ` Alex Khatskevich
2018-07-20 11:34       ` Alex Khatskevich
2018-07-20 14:15         ` Vladislav Shpilevoy
2018-07-18 17:47 ` [tarantool-patches] [PATCH 3/3] Introduce storage reload evolution AKhatskevich
2018-07-19 15:14   ` [tarantool-patches] " Vladislav Shpilevoy
2018-07-20 11:32     ` Alex Khatskevich
2018-07-20 14:15       ` Vladislav Shpilevoy
2018-07-20 23:58         ` Alex Khatskevich [this message]

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=a543a03b-a673-1739-9c7c-11f6832d64e2@tarantool.org \
    --to=avkhatskevich@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH 3/3] Introduce storage reload evolution' \
    /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