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 E15B227B3E for ; Fri, 20 Jul 2018 19:58:42 -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 8edlMDvn1_t8 for ; Fri, 20 Jul 2018 19:58:42 -0400 (EDT) Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 308DC27B3D for ; Fri, 20 Jul 2018 19:58:42 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 3/3] Introduce storage reload evolution References: <76cb41bd-cce4-8483-9222-39176a146ce4@tarantool.org> From: Alex Khatskevich Message-ID: Date: Sat, 21 Jul 2018 02:58:33 +0300 MIME-Version: 1.0 In-Reply-To: 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: Vladislav Shpilevoy , tarantool-patches@freelists.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.