From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olegrok@tarantool.org>
Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id E1D0E4696C3
 for <tarantool-patches@dev.tarantool.org>;
 Mon, 23 Mar 2020 09:35:04 +0300 (MSK)
References: <cover.1584817081.git.v.shpilevoy@tarantool.org>
 <364ea64d7bf6beef8cea83ff2fbb432ba967cd80.1584817081.git.v.shpilevoy@tarantool.org>
 <41dcbae6-b009-5f2e-ca28-a71ea4541488@tarantool.org>
 <ecaf3a2c-cbdc-12bf-550a-7804e501daae@tarantool.org>
From: Oleg Babin <olegrok@tarantool.org>
Message-ID: <d9cd3502-b3fc-c09f-2c23-e6e9ebeb883b@tarantool.org>
Date: Mon, 23 Mar 2020 09:35:02 +0300
MIME-Version: 1.0
In-Reply-To: <ecaf3a2c-cbdc-12bf-550a-7804e501daae@tarantool.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH vshard 1/2] storage: introduce
	upgrade strategy
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, tarantool-patches@dev.tarantool.org, yaroslav.dynnikov@tarantool.org

Hi! Thanks for the changes! One nit and LGTM.

On 22/03/2020 22:12, Vladislav Shpilevoy wrote:
> But for 0.1.16.0 the atomicity is possible, I added
> box.begin/commit/rollback.
> 
> -    for _, handler in ipairs(handlers) do
> +    for _, handler in ipairs(schema_upgrade_handlers) do
>           local next_version = handler.version
>           if schema_compare_versions(next_version, version) > 0 then
>               local next_version_str = table.concat(next_version, '.')
>               log.info("Upgrade vshard to {%s}", next_version_str)
> -            handler.func(username, password)
> +            local ok, err = pcall(handler.func, username, password)
> +            if not ok then
> +                log.info("Couldn't upgrade to {%s}: %s", next_version_str, err)

Seems that you've missed box.rollback here. And I think that it should 
be something like "if box.is_in_txn() then box.rollback() end"

I'm not sure but is it possible to drop last xlogs for user and start to 
use previous version of schema (and vshard version as well) as 
workaround in case of "fatal error"?