[tarantool-patches] Re: [PATCH][vshard] Reload reloadable fiber

[Alex Khatskevich]

>> Fixed a problem:
>> The `reloadable_fiber_f` was running an infinite where loop and
>
> 1. What is a 'where loop'?
where -> while
>> -while test_run:grep_log('router_1', 'Failover has been reloaded') == 
>> nil do fiber.sleep(0.1) end
>> +while test_run:grep_log('router_1', 'Failover has been started') == 
>> nil do fiber.sleep(0.1) end
>
> 2. Why? Please, leave the old message. Router already writes that
> failover is started in router_cfg. In other places the same.
Discussed verbally.
After this commit, it is the reloadable fiber's responsibility to write 
"started" message, it looks like
'Worker_name has been started'.
Reloaded message looks like 'Worker_name is reloaded, restarting'
>> +    --
>> +    -- The module is loaded for the first time.
>> +    --
>> +    M = {
>> +        -- Latest versions of functions.
>> +        reloadable_fiber_f = nil,
>> +        errinj = {
>> +            RELOADABLE_STACK_MAX = nil,
>
> 3. What is the point of this error injection? It
> tests Lua, not VShard, as I think. And it takes
> too many lines in the reloadable fiber function
> complicating its understanding. So lets remove.
deleted
>> +-- @param module Module which can be reloaded.
>> +-- @param func_name Name of a function to be executed in the
>> +--        module.
>> +-- @param worker_name Name of the reloadable background subsystem.
>> +--        For example: "Garbage Collector", "Recovery", "Discovery",
>> +--        "Rebalancer". Used only for an activity logging.
>>   --
>> -local function reloadable_fiber_f(M, func_name, worker_name)
>> -    while true do
>> -        local ok, err = pcall(M[func_name], M.module_version)
>> -        if not ok then
>> -            log.error('%s has been failed: %s', worker_name, err)
>> -            fiber.yield()
>> -        else
>> -            log.info('%s has been reloaded', worker_name)
>> -            fiber.yield()
>> +local function reloadable_fiber_f(module, func_name, worker_name)
>> +    log.info('%s has been started', worker_name)
>> +    local func = module[func_name]
>> +    local ok, err = pcall(func, module.module_version)
>> +    if not ok then
>> +        log.error('%s has been failed: %s', worker_name, err)
>> +        if func ~= module[func_name] then
>> +            log.warn('%s reloadable function %s has changed',
>> +                        worker_name, func_name)
>>           end
>>       end
>> +    fiber.yield()
>> +    log.info('%s is reloading', worker_name)
>> +    if M.errinj.RELOADABLE_EXIT then
>> +        return
>
> 4. How is this error possible? There are no lines in reloadable_fiber_f
> that can terminate the fiber.
deleted
>
> 5. Now on any reload I see two messages:
>
> started
> reloading
> started
> reloading
>
> But actually the fiber is started once. Please, return the old
> messages.
>
discussed verbally
>> +++ b/test/unit/util.result
>> @@ -0,0 +1,107 @@
>> +test_run = require('test_run').new()
>> +---
>> +...
>> +util = require('vshard.util')
>> +---
>> +...
>> +test_util = require('util')
>
> 6. Unused variable?
deleted