From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (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 1BFF54765E0 for ; Fri, 25 Dec 2020 11:50:49 +0300 (MSK) References: <5912b7266d065501fe33500c640048ac26804f0f.1608816289.git.sergepetrenko@tarantool.org> From: Serge Petrenko Message-ID: <4fc3e2cc-07f2-32c0-5311-b2243227527e@tarantool.org> Date: Fri, 25 Dec 2020 11:50:48 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aleksandr Lyapunov , v.shpilevoy@tarantool.org, mons@tarantool.org Cc: tarantool-patches@dev.tarantool.org 25.12.2020 11:12, Aleksandr Lyapunov пишет: > Hi, thanks for the patch! > See one comment below. > If we have already decided to exclude this commit - nevermind. > > On 24.12.2020 16:34, Serge Petrenko wrote: >> Feedback daemon is now able to send feedback once tarantool exits. >> This helps to gather statistics for instances that shut down even before >> the first feedback_interval passes. >> --- >>   src/box/lua/feedback_daemon.lua | 13 +++++++++++++ >>   1 file changed, 13 insertions(+) >> >> diff --git a/src/box/lua/feedback_daemon.lua >> b/src/box/lua/feedback_daemon.lua >> index 1aac32bb3..f42ab5822 100644 >> --- a/src/box/lua/feedback_daemon.lua >> +++ b/src/box/lua/feedback_daemon.lua >> @@ -355,6 +355,13 @@ local function guard_loop(self) >>       self.shutdown:put("stopped") >>   end >>   +-- An on_shutdown trigger to fire on tarantool exit. >> +local function on_shutdown(self) >> +    if self.control ~= nil then >> +        self.control:put("send") > Does it always works? > I mean that you put a message to channel in on_shutdown trigger. > But the channel does not wake consumer immediately, it will wake up > in the next event loop which can never start since we shutting down. > I'm not sure btw, only suspicious. Hi! Thanks  for the  review! I may be mistaken, but as far as I understand, channel:put calls fiber_wakeup() under the hood. And fiber_wakeup() puts the fiber to execution at the end of this particular event loop iteration. The next on_shutdown trigger breaks the event loop though. And I'm not sure whether it does it immediately or waits till the event loop iteration end. Here's what the libev doc says: ` ev_break (loop, how) Can be used to make a call to|ev_run|return early (but only after it has processed all outstanding events). ` Looks like we're safe. The loop won't break until it processes the fiber_wakeup event. >> +    end >> +end >> + >>   -- these functions are used for test purposes only >>   local function start(self) >>       self:stop() >> @@ -400,6 +407,9 @@ setmetatable(daemon, { >>           generate_feedback = function() >>               return fill_in_feedback({ feedback_version = 5 }) >>           end, >> +        on_shutdown = function() >> +            on_shutdown(daemon) >> +        end, >>           start = function() >>               start(daemon) >>           end, >> @@ -437,3 +447,6 @@ if box.internal == nil then >>   else >>       box.internal[PREFIX] = daemon >>   end >> + >> +-- Send feedback on Tarantool exit. >> +box.ctl.on_shutdown(function() daemon:on_shutdown() end) -- Serge Petrenko