Re: [Tarantool-patches] [PATCH 4/4] feedback_daemon: generate report right before sending

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the patch!

The patch is mostly fine, but I don't see why would we need it, as well
as why do we need to know when a first space is created. I don't argue,
only making a note. Unless it would affect the perf or normal code base
complexity anyhow.

See 2 comments below.

> diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
> index 0aab189b6..ca93b7574 100644
> --- a/src/box/lua/feedback_daemon.lua
> +++ b/src/box/lua/feedback_daemon.lua
> @@ -358,12 +363,26 @@ local function guard_loop(self)
>      self.shutdown:put("stopped")
>  end
> 
> +local function save_event(self, event)
> +    if type(event) ~= 'string' then
> +        error("Usage: box.internal.feedback_daemon.save_event(string)")
> +    end
> +    local cnt = self.cached_events[event] or 0

1. cnt is unused. Did it pass luacheck? I suppose you wanted something like
this:

====================
@@ -367,9 +367,9 @@ local function save_event(self, event)
     if type(event) ~= 'string' then
         error("Usage: box.internal.feedback_daemon.save_event(string)")
     end
-    local cnt = self.cached_events[event] or 0
-    self.cached_events[event] = (self.cached_events[event] or 0) + 1
-    if self.cached_events[event] == 1 then
+    local cnt = (self.cached_events[event] + 1) or 1
+    self.cached_events[event] = cnt
+    if cnt == 1 then
         -- The first occurred event of this type triggers report dispatch
         -- immediately.
         self.send()
====================

> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 96503a50e..cf1b070af 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -462,6 +462,10 @@ box.schema.space.create = function(name, options)
>      })
>      _space:insert{id, uid, name, options.engine, options.field_count,
>          space_options, format}
> +
> +    if internal.feedback_daemon ~= nil then
> +        internal.feedback_daemon.save_event("create_space")

2. To make it less ugly you could save feedback_daemon into a local
variable in this file, and access it as an upvalue in all the
usages. Or wrap the entire 'save_event' thing here into a function
which checks that the daemon is not nil, and then call its
save_event function.