Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] feedback_daemon: fix indexing a nil value issue
@ 2021-04-19 17:53 Serge Petrenko via Tarantool-patches
  2021-04-19 22:39 ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Serge Petrenko via Tarantool-patches @ 2021-04-19 17:53 UTC (permalink / raw)
  To: avtikhon, v.shpilevoy; +Cc: tarantool-patches

When running tarantool with disabled feedback daemon the following error
appeared on each space/index create/drop:

builtin/box/feedback_daemon.lua:380: attempt to index field 'cached_events'
(a nil value)

This happened because 'cached_events' table is initialized only on
feedback daemon start.
Fix the issue by checking for type of `cached_events` and only indexing
it when it's a table.

Follow-up #5750
---

branch: https://github.com/tarantool/tarantool/tree/sp/feedback-daemon-fix

 src/box/lua/feedback_daemon.lua | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
index 7a6b0c94c..ee0315c2d 100644
--- a/src/box/lua/feedback_daemon.lua
+++ b/src/box/lua/feedback_daemon.lua
@@ -377,6 +377,10 @@ local function save_event(self, event)
     if type(event) ~= 'string' then
         error("Usage: box.internal.feedback_daemon.save_event(string)")
     end
+    if type(self.cached_events) ~= 'table' then
+        return
+    end
+
     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
-- 
2.24.3 (Apple Git-128)


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

* Re: [Tarantool-patches] [PATCH] feedback_daemon: fix indexing a nil value issue
  2021-04-19 17:53 [Tarantool-patches] [PATCH] feedback_daemon: fix indexing a nil value issue Serge Petrenko via Tarantool-patches
@ 2021-04-19 22:39 ` Vladislav Shpilevoy via Tarantool-patches
  2021-04-20  9:28   ` Serge Petrenko via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-04-19 22:39 UTC (permalink / raw)
  To: Serge Petrenko, avtikhon; +Cc: tarantool-patches

Thanks for the patch!

For bugfixes we usually add a test. Looks fine except that.
But I didn't validate (because no a test).

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

* Re: [Tarantool-patches] [PATCH] feedback_daemon: fix indexing a nil value issue
  2021-04-19 22:39 ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-04-20  9:28   ` Serge Petrenko via Tarantool-patches
  2021-04-20 19:42     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Serge Petrenko via Tarantool-patches @ 2021-04-20  9:28 UTC (permalink / raw)
  To: Vladislav Shpilevoy, avtikhon; +Cc: tarantool-patches



20.04.2021 01:39, Vladislav Shpilevoy пишет:
> Thanks for the patch!
>
> For bugfixes we usually add a test. Looks fine except that.
> But I didn't validate (because no a test).
Thanks for the review!

Here's  the test:

===========================

diff --git a/test/box-tap/gh-5750-feedback-disabled-err.test.lua 
b/test/box-tap/gh-5750-feedback-disabled-err.test.lua
new file mode 100755
index 000000000..190e93b7d
--- /dev/null
+++ b/test/box-tap/gh-5750-feedback-disabled-err.test.lua
@@ -0,0 +1,29 @@
+#!/usr/bin/env tarantool
+
+--
+-- Test that disabling feedback in initial configuration doesn't lead to
+-- "attempt to index field 'cached_events' (a nil value)" errors when 
creating
+-- or dropping a space/index.
+--
+
+local tap = require('tap')
+local test = tap.test('feedback_enabled=false')
+
+local ok, err = pcall(box.cfg, {feedback_enabled=false})
+
+-- feedback daemon may be disabled at compile time. Then all options like
+-- feedback_enabled will be undefined.
+if not ok then
+    test:plan(1)
+    test:like(err, 'unexpected option', 'feedback_enabled is undefined')
+    test:check()
+    os.exit(0)
+end
+
+test:plan(2)
+ok = pcall(box.schema.space.create, 'test')
+test:ok(ok, 'space create succeeds')
+ok = pcall(box.space.test.drop, box.space.test)
+test:ok(ok, 'space drop succeeds')
+test:check()
+os.exit(0)

-- 
Serge Petrenko


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

* Re: [Tarantool-patches] [PATCH] feedback_daemon: fix indexing a nil value issue
  2021-04-20  9:28   ` Serge Petrenko via Tarantool-patches
@ 2021-04-20 19:42     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-04-20 19:42 UTC (permalink / raw)
  To: Serge Petrenko, avtikhon; +Cc: tarantool-patches

Hi! Thanks for the patch!

LGTM.

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

end of thread, other threads:[~2021-04-20 19:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 17:53 [Tarantool-patches] [PATCH] feedback_daemon: fix indexing a nil value issue Serge Petrenko via Tarantool-patches
2021-04-19 22:39 ` Vladislav Shpilevoy via Tarantool-patches
2021-04-20  9:28   ` Serge Petrenko via Tarantool-patches
2021-04-20 19:42     ` Vladislav Shpilevoy via Tarantool-patches

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git