From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 2F2D56EC5D; Tue, 6 Apr 2021 11:18:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2F2D56EC5D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617697117; bh=iIVMtH0cg0HSXsXh9bSXdyBa2+eLj0pI4l56a+HsWrs=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=HC8dY2pQwbyohjdcH28xRuGu0O+8saJ5qC6wXwG5mcfUmnaKXEQOnusPalQrPfDnK h1MszVMY6OiOSdHHo5OYbuvuVsTavBL6nkIBPryhvh70stmrmLF4c73Rv7SCcM7h6w nYs4ykdHxAcQQ70OGXukw4pTvt0v76JHKEGqnxW8= Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 C45A06EC5D for ; Tue, 6 Apr 2021 11:18:35 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C45A06EC5D Received: by smtp48.i.mail.ru with esmtpa (envelope-from ) id 1lTguw-0004QU-Uy; Tue, 06 Apr 2021 11:18:35 +0300 To: Vladislav Shpilevoy , alexander.turenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org References: <44f4b260e0ac435fe78282c1f6f4607159914935.1617375300.git.sergepetrenko@tarantool.org> <6216debc-5807-ede1-df21-823d60a1e70b@tarantool.org> <36dca5a2-9e18-e517-78d2-827b85cbc3e1@tarantool.org> Message-ID: <55ccac3e-1c39-46b4-2278-d101aac1f08f@tarantool.org> Date: Tue, 6 Apr 2021 11:18:34 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <36dca5a2-9e18-e517-78d2-827b85cbc3e1@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9ED7173E37F4E32947A0146560F8BA70927CAA5B950F38D9F182A05F538085040C7918A5EF374E64AF10A5FBD267416A4967253BB86F48043E593992D54DCF510 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE79B4AFF82F0254274EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637D919B403F35891598638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95C16EE06F5A270FE6A6109B72B0CE018C55D1248891792501CA471835C12D1D9774AD6D5ED66289B5278DA827A17800CE70F3DDF2BBF19B93A9FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3A12191B5F2BB8629117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CFEEA082C9A12FE45576E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8BB4A6D9B00E37F4B33AA81AA40904B5D9DBF02ECDB25306B2201CA6A4E26CD07C3BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4931B544F03EFBC4D57FC839A7D10C5E1E9C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A5245E6BD1ED39F6F629390FADDFF5EB29461A71EE16DCEBCED59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D8C933888226C8412159C95B001BED35C5C8E1FAFBA749358B880F6FB38E212FFCDEA2F31813315A1D7E09C32AA3244C62EAA3CE9C44F06AC12CDCB3F349B463D08D48398F32B4A6FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojM00ve/f+0ome6mZFGmOnUQ== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769310BB5A2F8068DC9F10A5FBD267416A40D95629463F16A6A32C2A64043BFB05F66FEC6BF5C9C28D9D2F9AC31ED4B18F0B80F102789B70DF27402F9BA4338D657ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 4/4] feedback_daemon: generate report right before sending X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 05.04.2021 19:23, Vladislav Shpilevoy пишет: > 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. Well, it was requested in the ticked, and I've asked Artur personally. They want to monitor user's progress in tutorials or something alike. So, they want to receive feedback as soon as the user starts tarantool (calls box.cfg in our case), creates the first space, creates the first index and so on. I've discussed this with Mons, and he suggested to count such events always. This patch adds all the needed machinery for that task. > > 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: Oops, I forgot to drop this line. Sorry. > > ==================== > @@ -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. Sure, let's wrap it in a function. Incremental diff below. =================================================== diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua index ca93b7574..fb0e0df35 100644 --- a/src/box/lua/feedback_daemon.lua +++ b/src/box/lua/feedback_daemon.lua @@ -367,7 +367,6 @@ 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          -- The first occurred event of this type triggers report dispatch diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index cf1b070af..93ecf7440 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -321,6 +321,12 @@ local function update_param_table(table, defaults)      return new_table  end +local function feedback_save_event(event) +    if internal.feedback_daemon ~= nil then +        internal.feedback_daemon.save_event(event) +    end +end +  box.begin = function()      if builtin.box_txn_begin() == -1 then          box.error() @@ -463,9 +469,7 @@ 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") -    end +    feedback_save_event('create_space')      return box.space[id], "created"  end @@ -535,9 +539,8 @@ box.schema.space.drop = function(space_id, space_name, opts)              box.error(box.error.NO_SUCH_SPACE, space_name)          end      end -    if internal.feedback_daemon ~= nil then -        internal.feedback_daemon.save_event("drop_space") -    end + +    feedback_save_event('drop_space')  end  box.schema.space.rename = function(space_id, space_name) @@ -1245,10 +1248,7 @@ box.schema.index.create = function(space_id, name, options)          _func_index:insert{space_id, iid, index_opts.func}      end -    if internal.feedback_daemon ~= nil then -        internal.feedback_daemon.save_event("create_index") -    end - +    feedback_save_event('create_index')      return space.index[name]  end @@ -1270,9 +1270,7 @@ box.schema.index.drop = function(space_id, index_id)      end      _index:delete{space_id, index_id} -    if internal.feedback_daemon ~= nil then -        internal.feedback_daemon.save_event("drop_index") -    end +    feedback_save_event('drop_index')  end  box.schema.index.rename = function(space_id, index_id, name) -- Serge Petrenko