Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH 0/2] feedback_daemon: add operation statistics reporting and an additional report on shutdown
@ 2020-12-24 13:34 Serge Petrenko
  2020-12-24 13:34 ` [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting Serge Petrenko
  2020-12-24 13:34 ` [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit Serge Petrenko
  0 siblings, 2 replies; 12+ messages in thread
From: Serge Petrenko @ 2020-12-24 13:34 UTC (permalink / raw)
  To: alyapunov, v.shpilevoy, mons; +Cc: tarantool-patches

https://github.com/tarantool/tarantool/issues/5589
sp/gh-5589-feedback-stats

Serge Petrenko (2):
  feedback_daemon: add operation statistics reporting
  feedback_daemon: send feedback on tarantool exit

 src/box/lua/feedback_daemon.lua       | 42 ++++++++++++++++++++++-
 test/box-tap/feedback_daemon.test.lua | 49 ++++++++++++++++++++++++++-
 2 files changed, 89 insertions(+), 2 deletions(-)

-- 
2.24.3 (Apple Git-128)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting
  2020-12-24 13:34 [Tarantool-patches] [PATCH 0/2] feedback_daemon: add operation statistics reporting and an additional report on shutdown Serge Petrenko
@ 2020-12-24 13:34 ` Serge Petrenko
  2020-12-24 17:54   ` Vladislav Shpilevoy
  2020-12-25  8:29   ` Kirill Yukhin
  2020-12-24 13:34 ` [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit Serge Petrenko
  1 sibling, 2 replies; 12+ messages in thread
From: Serge Petrenko @ 2020-12-24 13:34 UTC (permalink / raw)
  To: alyapunov, v.shpilevoy, mons; +Cc: tarantool-patches

Report box.stat().*.total, box.stat.net().*.total and
box.stat.net().*.current via feedback daemon report.
Accompany this data with the time when report was generated so that it's
possible to calculate RPS from this data on the feedback server.

`box.stat().OP_NAME.total` reside in `feedback.stats.box.OP_NAME.total`, while
`box.stat.net().OP_NAME.total` reside in `feedback.stats.net.OP_NAME.total`
The time of report generation is located at `feedback.stats.time`

Closes #5589
---
 src/box/lua/feedback_daemon.lua       | 29 +++++++++++++++-
 test/box-tap/feedback_daemon.test.lua | 49 ++++++++++++++++++++++++++-
 2 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
index 1d39012ed..1aac32bb3 100644
--- a/src/box/lua/feedback_daemon.lua
+++ b/src/box/lua/feedback_daemon.lua
@@ -284,12 +284,39 @@ local function fill_in_options(feedback)
     feedback.options = options
 end
 
+local function fill_in_stats(feedback)
+    local stats = {box={}, net={}}
+    local box_stat = box.stat()
+    local net_stat = box.stat.net()
+
+    stats.time = fiber.time64()
+    -- Send box.stat().*.total.
+    for op, tbl in pairs(box_stat) do
+        if type(tbl) == 'table' and tbl.total ~= nil then
+            stats.box[op] = {
+                total = tbl.total
+            }
+        end
+    end
+    -- Send box.stat.net().*.total and box.stat.net().*.current.
+    for val, tbl in pairs(net_stat) do
+        if type(tbl) == 'table' and (tbl.total ~= nil or tbl.current ~= nil) then
+                    stats.net[val] = {
+                        total = tbl.total,
+                        current = tbl.current
+                    }
+        end
+    end
+    feedback.stats = stats
+end
+
 local function fill_in_feedback(feedback)
     fill_in_base_info(feedback)
     fill_in_platform_info(feedback)
     fill_in_repo_url(feedback)
     fill_in_features(feedback)
     fill_in_options(feedback)
+    fill_in_stats(feedback)
 
     return feedback
 end
@@ -371,7 +398,7 @@ setmetatable(daemon, {
         end,
         -- this function is used in saving feedback in file
         generate_feedback = function()
-            return fill_in_feedback({ feedback_version = 4 })
+            return fill_in_feedback({ feedback_version = 5 })
         end,
         start = function()
             start(daemon)
diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
index 7ebc97587..1fcb4e5fa 100755
--- a/test/box-tap/feedback_daemon.test.lua
+++ b/test/box-tap/feedback_daemon.test.lua
@@ -20,6 +20,9 @@ local function feedback_reset()
 end
 
 local function http_handle(s)
+    -- When data is > 1024 bytes, curl sends "Expect: 100-continue" header,
+    -- and waits for this response before sending the actual data.
+    s:write("HTTP/1.1 100 Continue\r\n\r\n")
     s:write("HTTP/1.1 200 OK\r\n")
     s:write("Accept: */*\r\n")
     s:write("Connection: keep-alive\r\n")
@@ -67,7 +70,7 @@ if not ok then
     os.exit(0)
 end
 
-test:plan(23)
+test:plan(27)
 
 local function check(message)
     while feedback_count < 1 do
@@ -120,6 +123,9 @@ local fio = require("fio")
 local fh = fio.open("feedback.json")
 test:ok(fh, "file is created")
 local file_data = fh:read()
+-- Ignore the report time. The data should be equal other than that.
+feedback_save = string.gsub(feedback_save, 'time:(%d+)', 'time:0')
+file_data = string.gsub(feedback_save, 'time:(%d+)', 'time:0')
 test:is(file_data, feedback_save, "data is equal")
 fh:close()
 fio.unlink("feedback.json")
@@ -241,5 +247,46 @@ box.space.features_memtx_empty:drop()
 box.space.features_memtx:drop()
 box.space.features_sync:drop()
 
+function check_stats(stat)
+    local sub = test:test('feedback operation stats')
+    sub:plan(18)
+    local box_stat = box.stat()
+    local net_stat = box.stat.net()
+    for op, val in pairs(box_stat) do
+        sub:is(stat.box[op].total, val.total,
+               string.format('%s total is reported', op))
+    end
+    for op, val in pairs(net_stat) do
+        sub:is(stat.net[op].total, val.total,
+               string.format('%s total is reported', op))
+        if val.current ~= nil then
+            sub:is(stat.net[op].current, val.current,
+                   string.format('%s current is reported', op))
+        end
+    end
+    sub:check()
+end
+
+actual = daemon.generate_feedback()
+test:is(fiber.time64(), actual.stats.time, "Time of report generation is correct")
+
+-- Check that all the statistics are reported.
+check_stats(actual.stats)
+
+box.schema.space.create('test')
+box.space.test:create_index('pk')
+box.space.test:insert{1}
+box.space.test:select{}
+box.space.test:update({1}, {{'=', 2, 1}})
+box.space.test:replace{2}
+box.space.test:delete{1}
+box.space.test:drop()
+
+-- Check that all the statistics are updated.
+actual = daemon.generate_feedback()
+test:is(fiber.time64(), actual.stats.time, "Time of report generation is correct")
+
+check_stats(actual.stats)
+
 test:check()
 os.exit(0)
-- 
2.24.3 (Apple Git-128)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit
  2020-12-24 13:34 [Tarantool-patches] [PATCH 0/2] feedback_daemon: add operation statistics reporting and an additional report on shutdown Serge Petrenko
  2020-12-24 13:34 ` [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting Serge Petrenko
@ 2020-12-24 13:34 ` Serge Petrenko
  2020-12-24 17:54   ` Vladislav Shpilevoy
  2020-12-25  8:12   ` Aleksandr Lyapunov
  1 sibling, 2 replies; 12+ messages in thread
From: Serge Petrenko @ 2020-12-24 13:34 UTC (permalink / raw)
  To: alyapunov, v.shpilevoy, mons; +Cc: tarantool-patches

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")
+    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)
-- 
2.24.3 (Apple Git-128)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting
  2020-12-24 13:34 ` [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting Serge Petrenko
@ 2020-12-24 17:54   ` Vladislav Shpilevoy
  2020-12-24 21:09     ` Serge Petrenko
  2020-12-25  8:29   ` Kirill Yukhin
  1 sibling, 1 reply; 12+ messages in thread
From: Vladislav Shpilevoy @ 2020-12-24 17:54 UTC (permalink / raw)
  To: Serge Petrenko, alyapunov, mons; +Cc: tarantool-patches

Hi! Thanks for the patch!

Consider this diff which I pushed as a separate commit
in your branch:

====================
diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
index 1aac32bb3..07f114aea 100644
--- a/src/box/lua/feedback_daemon.lua
+++ b/src/box/lua/feedback_daemon.lua
@@ -285,7 +285,7 @@ local function fill_in_options(feedback)
 end
 
 local function fill_in_stats(feedback)
-    local stats = {box={}, net={}}
+    local stats = {box = {}, net = {}}
     local box_stat = box.stat()
     local net_stat = box.stat.net()
 
@@ -300,11 +300,12 @@ local function fill_in_stats(feedback)
     end
     -- Send box.stat.net().*.total and box.stat.net().*.current.
     for val, tbl in pairs(net_stat) do
-        if type(tbl) == 'table' and (tbl.total ~= nil or tbl.current ~= nil) then
-                    stats.net[val] = {
-                        total = tbl.total,
-                        current = tbl.current
-                    }
+        if type(tbl) == 'table' and
+           (tbl.total ~= nil or tbl.current ~= nil) then
+            stats.net[val] = {
+                total = tbl.total,
+                current = tbl.current
+            }
         end
     end
     feedback.stats = stats

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit
  2020-12-24 13:34 ` [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit Serge Petrenko
@ 2020-12-24 17:54   ` Vladislav Shpilevoy
  2020-12-24 21:11     ` Serge Petrenko
  2020-12-25  8:12   ` Aleksandr Lyapunov
  1 sibling, 1 reply; 12+ messages in thread
From: Vladislav Shpilevoy @ 2020-12-24 17:54 UTC (permalink / raw)
  To: Serge Petrenko, alyapunov, mons; +Cc: tarantool-patches

Technically looks good, but it seems some people don't like it.
And it is understandable - the server will get hundreds or even
thousands of reports from everywhere, including the tests we
run locally everyday.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting
  2020-12-24 17:54   ` Vladislav Shpilevoy
@ 2020-12-24 21:09     ` Serge Petrenko
  2020-12-25  8:13       ` Aleksandr Lyapunov
  0 siblings, 1 reply; 12+ messages in thread
From: Serge Petrenko @ 2020-12-24 21:09 UTC (permalink / raw)
  To: Vladislav Shpilevoy, alyapunov, mons; +Cc: tarantool-patches



24.12.2020 20:54, Vladislav Shpilevoy пишет:
> Hi! Thanks for the patch!
>
> Consider this diff which I pushed as a separate commit
> in your branch:

Thanks for the review!

I applied your diff.

Sorry you had to fix indentation for me.

>
> ====================
> diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
> index 1aac32bb3..07f114aea 100644
> --- a/src/box/lua/feedback_daemon.lua
> +++ b/src/box/lua/feedback_daemon.lua
> @@ -285,7 +285,7 @@ local function fill_in_options(feedback)
>   end
>   
>   local function fill_in_stats(feedback)
> -    local stats = {box={}, net={}}
> +    local stats = {box = {}, net = {}}
>       local box_stat = box.stat()
>       local net_stat = box.stat.net()
>   
> @@ -300,11 +300,12 @@ local function fill_in_stats(feedback)
>       end
>       -- Send box.stat.net().*.total and box.stat.net().*.current.
>       for val, tbl in pairs(net_stat) do
> -        if type(tbl) == 'table' and (tbl.total ~= nil or tbl.current ~= nil) then
> -                    stats.net[val] = {
> -                        total = tbl.total,
> -                        current = tbl.current
> -                    }
> +        if type(tbl) == 'table' and
> +           (tbl.total ~= nil or tbl.current ~= nil) then
> +            stats.net[val] = {
> +                total = tbl.total,
> +                current = tbl.current
> +            }
>           end
>       end
>       feedback.stats = stats

-- 
Serge Petrenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit
  2020-12-24 17:54   ` Vladislav Shpilevoy
@ 2020-12-24 21:11     ` Serge Petrenko
  0 siblings, 0 replies; 12+ messages in thread
From: Serge Petrenko @ 2020-12-24 21:11 UTC (permalink / raw)
  To: Vladislav Shpilevoy, alyapunov, mons; +Cc: tarantool-patches



24.12.2020 20:54, Vladislav Shpilevoy пишет:
> Technically looks good, but it seems some people don't like it.
> And it is understandable - the server will get hundreds or even
> thousands of reports from everywhere, including the tests we
> run locally everyday.

No problem, let's drop this commit then.
It's more of a proof of concept anyway.

-- 
Serge Petrenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit
  2020-12-24 13:34 ` [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit Serge Petrenko
  2020-12-24 17:54   ` Vladislav Shpilevoy
@ 2020-12-25  8:12   ` Aleksandr Lyapunov
  2020-12-25  8:50     ` Serge Petrenko
  1 sibling, 1 reply; 12+ messages in thread
From: Aleksandr Lyapunov @ 2020-12-25  8:12 UTC (permalink / raw)
  To: Serge Petrenko, v.shpilevoy, mons; +Cc: tarantool-patches

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)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting
  2020-12-24 21:09     ` Serge Petrenko
@ 2020-12-25  8:13       ` Aleksandr Lyapunov
  0 siblings, 0 replies; 12+ messages in thread
From: Aleksandr Lyapunov @ 2020-12-25  8:13 UTC (permalink / raw)
  To: Serge Petrenko, Vladislav Shpilevoy, mons; +Cc: tarantool-patches

Hi, thanks for the patch!

1/2 - LGTM!

On 25.12.2020 00:09, Serge Petrenko wrote:
>
>
> 24.12.2020 20:54, Vladislav Shpilevoy пишет:
>> Hi! Thanks for the patch!
>>
>> Consider this diff which I pushed as a separate commit
>> in your branch:
>
> Thanks for the review!
>
> I applied your diff.
>
> Sorry you had to fix indentation for me.
>
>>
>> ====================
>> diff --git a/src/box/lua/feedback_daemon.lua 
>> b/src/box/lua/feedback_daemon.lua
>> index 1aac32bb3..07f114aea 100644
>> --- a/src/box/lua/feedback_daemon.lua
>> +++ b/src/box/lua/feedback_daemon.lua
>> @@ -285,7 +285,7 @@ local function fill_in_options(feedback)
>>   end
>>     local function fill_in_stats(feedback)
>> -    local stats = {box={}, net={}}
>> +    local stats = {box = {}, net = {}}
>>       local box_stat = box.stat()
>>       local net_stat = box.stat.net()
>>   @@ -300,11 +300,12 @@ local function fill_in_stats(feedback)
>>       end
>>       -- Send box.stat.net().*.total and box.stat.net().*.current.
>>       for val, tbl in pairs(net_stat) do
>> -        if type(tbl) == 'table' and (tbl.total ~= nil or tbl.current 
>> ~= nil) then
>> -                    stats.net[val] = {
>> -                        total = tbl.total,
>> -                        current = tbl.current
>> -                    }
>> +        if type(tbl) == 'table' and
>> +           (tbl.total ~= nil or tbl.current ~= nil) then
>> +            stats.net[val] = {
>> +                total = tbl.total,
>> +                current = tbl.current
>> +            }
>>           end
>>       end
>>       feedback.stats = stats
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting
  2020-12-24 13:34 ` [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting Serge Petrenko
  2020-12-24 17:54   ` Vladislav Shpilevoy
@ 2020-12-25  8:29   ` Kirill Yukhin
  1 sibling, 0 replies; 12+ messages in thread
From: Kirill Yukhin @ 2020-12-25  8:29 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: mons, tarantool-patches, v.shpilevoy

Hello,

On 24 Dec 16:34, Serge Petrenko via Tarantool-patches wrote:
> Report box.stat().*.total, box.stat.net().*.total and
> box.stat.net().*.current via feedback daemon report.
> Accompany this data with the time when report was generated so that it's
> possible to calculate RPS from this data on the feedback server.
> 
> `box.stat().OP_NAME.total` reside in `feedback.stats.box.OP_NAME.total`, while
> `box.stat.net().OP_NAME.total` reside in `feedback.stats.net.OP_NAME.total`
> The time of report generation is located at `feedback.stats.time`
> 
> Closes #5589
> ---
>  src/box/lua/feedback_daemon.lua       | 29 +++++++++++++++-
>  test/box-tap/feedback_daemon.test.lua | 49 ++++++++++++++++++++++++++-
>  2 files changed, 76 insertions(+), 2 deletions(-)

I've checked your patch into master.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit
  2020-12-25  8:12   ` Aleksandr Lyapunov
@ 2020-12-25  8:50     ` Serge Petrenko
  2020-12-25 11:23       ` Aleksandr Lyapunov
  0 siblings, 1 reply; 12+ messages in thread
From: Serge Petrenko @ 2020-12-25  8:50 UTC (permalink / raw)
  To: Aleksandr Lyapunov, v.shpilevoy, mons; +Cc: tarantool-patches



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit
  2020-12-25  8:50     ` Serge Petrenko
@ 2020-12-25 11:23       ` Aleksandr Lyapunov
  0 siblings, 0 replies; 12+ messages in thread
From: Aleksandr Lyapunov @ 2020-12-25 11:23 UTC (permalink / raw)
  To: Serge Petrenko, v.shpilevoy, mons; +Cc: tarantool-patches


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

Looks like you are right. But I feel uncomfortable when I rely on 
feature that is
not documented and/or not covered with tests.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-12-25 11:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-24 13:34 [Tarantool-patches] [PATCH 0/2] feedback_daemon: add operation statistics reporting and an additional report on shutdown Serge Petrenko
2020-12-24 13:34 ` [Tarantool-patches] [PATCH 1/2] feedback_daemon: add operation statistics reporting Serge Petrenko
2020-12-24 17:54   ` Vladislav Shpilevoy
2020-12-24 21:09     ` Serge Petrenko
2020-12-25  8:13       ` Aleksandr Lyapunov
2020-12-25  8:29   ` Kirill Yukhin
2020-12-24 13:34 ` [Tarantool-patches] [PATCH 2/2] feedback_daemon: send feedback on tarantool exit Serge Petrenko
2020-12-24 17:54   ` Vladislav Shpilevoy
2020-12-24 21:11     ` Serge Petrenko
2020-12-25  8:12   ` Aleksandr Lyapunov
2020-12-25  8:50     ` Serge Petrenko
2020-12-25 11:23       ` Aleksandr Lyapunov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox