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 AB42921935 for ; Tue, 26 Jun 2018 08:44:02 -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 8bR3ysoZBOsn for ; Tue, 26 Jun 2018 08:44:02 -0400 (EDT) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (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 6A483218CD for ; Tue, 26 Jun 2018 08:44:02 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH][vshard] Reload reloadable fiber References: <20180614114202.2634-1-avkhatskevich@tarantool.org> <45c2c224-6bfa-c54d-7654-36961a9d1c66@tarantool.org> From: Alex Khatskevich Message-ID: <0c6f7167-d61d-010e-9ba5-5c08244a5bb9@tarantool.org> Date: Tue, 26 Jun 2018 15:43:59 +0300 MIME-Version: 1.0 In-Reply-To: <45c2c224-6bfa-c54d-7654-36961a9d1c66@tarantool.org> 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 >> 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