Re: [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit

[Serge Petrenko <sergepetrenko@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