From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <alyapunov@tarantool.org>
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
 (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 61A004765E0
 for <tarantool-patches@dev.tarantool.org>;
 Fri, 25 Dec 2020 11:12:38 +0300 (MSK)
References: <cover.1608816289.git.sergepetrenko@tarantool.org>
 <5912b7266d065501fe33500c640048ac26804f0f.1608816289.git.sergepetrenko@tarantool.org>
From: Aleksandr Lyapunov <alyapunov@tarantool.org>
Message-ID: <d7b94760-0d79-9bb9-7997-c1224c632787@tarantool.org>
Date: Fri, 25 Dec 2020 11:12:36 +0300
MIME-Version: 1.0
In-Reply-To: <5912b7266d065501fe33500c640048ac26804f0f.1608816289.git.sergepetrenko@tarantool.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Subject: Re: [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback
	on tarantool exit
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Serge Petrenko <sergepetrenko@tarantool.org>, v.shpilevoy@tarantool.org, mons@tarantool.org
Cc: tarantool-patches@dev.tarantool.org

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.
> +    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)