[tarantool-patches] Re: [PATCH 4/4] Introduce storage reload evolution

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch! See 2 comments below.

> diff --git a/test/lua_libs/git_util.lua b/test/lua_libs/git_util.lua
> new file mode 100644
> index 0000000..8826c4c
> --- /dev/null
> +++ b/test/lua_libs/git_util.lua
> @@ -0,0 +1,49 @@
> +--
> +-- Lua bridge for some of the git commands.
> +--
> +local os = require('os')
> +
> +--
> +-- Exec a git command.
> +-- @param params Table of parameters:
> +--        * options - git options.
> +--        * cmd - git command.
> +--        * args - command arguments.
> +--        * dir - working directory.
> +--        * fout - write output to the file.
> +local function exec_cmd(params)

1. Lets rename it to just 'exec'. Obviously it executes
'cmd'. Also lets make 'cmd' be first mandatory non-named
parameter and the second is params: args, dir etc.

     local function exec(cmd, params)

> +    local fout = params.fout
> +    local shell_cmd = {'git'}
> +    for _, param in pairs({'options', 'cmd', 'args'}) do
> +        table.insert(shell_cmd, params[param])
> +    end
> +    if fout then
> +        table.insert(shell_cmd, ' >' .. fout)
> +    end
> +    shell_cmd = table.concat(shell_cmd, ' ')
> +    if params.dir then
> +        shell_cmd = string.format('cd %s && %s', params.dir, shell_cmd)
> +    end
> +    local res = os.execute(shell_cmd)
> +    assert(res == 0, 'Git cmd error: ' .. res)
> +end
> +
> diff --git a/vshard/storage/reload_evolution.lua b/vshard/storage/reload_evolution.lua
> new file mode 100644
> index 0000000..8502a33
> --- /dev/null
> +++ b/vshard/storage/reload_evolution.lua
> @@ -0,0 +1,58 @@
> +--
> +-- This module is used to upgrade the vshard.storage on the fly.
> +-- It updates internal Lua structures in case they are changed
> +-- in a commit.
> +--
> +local log = require('log')
> +
> +--
> +-- Array of upgrade functions.
> +-- migrations[version] = function which upgrades module version
> +-- from `version` to `version + 1`.
> +--
> +local migrations = {}
> +
> +-- Initialize reload_upgrade mechanism
> +migrations[#migrations + 1] = function (M)

2. Redundant white space after 'function'.

> +    -- Code to update Lua objects.
> +end
> +
> +--
> +-- Perform an update based on a version stored in `M` (internals).
> +-- @param M Old module internals which should be updated.
> +--
> +local function upgrade(M)
> +    local start_version = M.reload_version or 1
> +    if start_version > #migrations then
> +        local err_msg = string.format(
> +            'vshard.storage.reload_evolution: ' ..
> +            'auto-downgrade is not implemented; ' ..
> +            'loaded version is %d, upgrade script version is %d',
> +            start_version, #migrations
> +        )
> +        log.error(err_msg)
> +        error(err_msg)
> +    end
> +    for i = start_version, #migrations  do
> +        local ok, err = pcall(migrations[i], M)
> +        if ok then
> +            log.info('vshard.storage.reload_evolution: upgraded to %d version',
> +                     i)
> +        else
> +            local err_msg = string.format(
> +                'vshard.storage.reload_evolution: ' ..
> +                'error during upgrade to %d version: %s', i, err
> +            )
> +            log.error(err_msg)
> +            error(err_msg)
> +        end
> +        -- Update the version just after upgrade to have an
> +        -- actual version in case of an error.
> +        M.reload_version = i
> +    end
> +end
> +
> +return {
> +    version = #migrations,
> +    upgrade = upgrade,
> +}