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

[Alex Khatskevich]

On 30.07.2018 14:55, Vladislav Shpilevoy wrote:
> Thanks for the patch! See 3 comments below.
>
>> diff --git a/test/lua_libs/git_util.lua b/test/lua_libs/git_util.lua
>> new file mode 100644
>> index 0000000..a75bb08
>> --- /dev/null
>> +++ b/test/lua_libs/git_util.lua
>> @@ -0,0 +1,51 @@
>> +--
>> +-- Lua bridge for some of the git commands.
>> +--
>> +local os = require('os')
>> +
>> +local temp_file = 'some_strange_rare_unique_file_name_for_git_util'
>
> 1. C library has a ready solution: tmpfile() and tmpnam() functions
> that have Lua API: io.tmpfile() to open a tmp file and automatically
> delete it on close, and os.tmpname that generates really unique tmp
> file name. Please, use.
io.tmpfile() is used.
>
>> +
>> +--
>> +-- 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)
>> +    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
>> +
>> +local function log_hashes(params)
>> +    params.args = "--format='%h' " .. params.args
>> +    local local_temp_file = string.format('%s/%s', os.getenv('PWD'), 
>> temp_file)
>> +    params.fout = local_temp_file
>> +    params.cmd = 'log'
>> +    exec_cmd(params)
>> +    local lines = {}
>> +    for line in io.lines(local_temp_file) do
>> +        table.insert(lines, line)
>> +    end
>> +    os.remove(local_temp_file)
>> +    return lines
>> +end
>> +
>> +
>
> 2. Too many empty lines.
deleted
>
>> +return {
>> +    exec_cmd = exec_cmd,
>> +    log_hashes = log_hashes
>> +}
>
> 3. What about router evolution?
Reload evolution is created to make able to reload from one version to 
another in
in case of noticeable internal changes.
As restarting router is much cheaper than restarting a storage and 
reload evolution
adds extra complexity, let us make users to restart the server in case 
of the
noticeable changes.