[Tarantool-patches] [PATCH 4/4] feedback: add cmake option to disable the daemon
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sun Apr 12 23:13:30 MSK 2020
There is a complaint that the feedback daemon is a 'spying' tool
and because of that can't be used on Gentoo. Its default disabled
option also is not acceptable, the daemon should be eliminated
completely.
The patch introduces cmake option ENABLE_FEEDBACK_DAEMON. It is
ON by default. When set to OFF, all feedback daemon's code is not
included into the binary, its configuration options disappear.
Closes #3308
---
src/box/CMakeLists.txt | 10 ++++++++-
src/box/lua/init.c | 9 +++++++++
src/box/lua/load_cfg.lua | 29 +++++++++++++++++++++------
src/trivia/config.h.cmake | 2 ++
test/box-tap/feedback_daemon.test.lua | 23 +++++++++++++++++----
test/box/misc.result | 19 ++++++++++++++++--
test/box/misc.test.lua | 15 +++++++++++++-
7 files changed, 93 insertions(+), 14 deletions(-)
diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 56758bd2f..4827e786e 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -1,5 +1,11 @@
file(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/src/box/lua)
+# Sometimes 'spying' code is not acceptable even if it would be
+# disabled by default. That option allows to remove the feedback
+# daemon from the build completely.
+option(ENABLE_FEEDBACK_DAEMON "Feedback daemon which reports debug data to \
+ the Tarantool team" ON)
+
add_subdirectory(sql)
set(lua_sources)
@@ -7,7 +13,9 @@ lua_source(lua_sources lua/load_cfg.lua)
lua_source(lua_sources lua/schema.lua)
lua_source(lua_sources lua/tuple.lua)
lua_source(lua_sources lua/session.lua)
-lua_source(lua_sources lua/feedback_daemon.lua)
+if (ENABLE_FEEDBACK_DAEMON)
+ lua_source(lua_sources lua/feedback_daemon.lua)
+endif()
lua_source(lua_sources lua/net_box.lua)
lua_source(lua_sources lua/upgrade.lua)
lua_source(lua_sources lua/console.lua)
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 9db740de6..29217c21e 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -71,7 +71,9 @@ extern char session_lua[],
schema_lua[],
load_cfg_lua[],
xlog_lua[],
+#if ENABLE_FEEDBACK_DAEMON
feedback_daemon_lua[],
+#endif
net_box_lua[],
upgrade_lua[],
console_lua[],
@@ -81,7 +83,14 @@ static const char *lua_sources[] = {
"box/session", session_lua,
"box/tuple", tuple_lua,
"box/schema", schema_lua,
+#if ENABLE_FEEDBACK_DAEMON
+ /*
+ * It is important to initialize the daemon before
+ * load_cfg, because the latter peeks up some values
+ * from the feedback daemon.
+ */
"box/feedback_daemon", feedback_daemon_lua,
+#endif
"box/upgrade", upgrade_lua,
"box/net_box", net_box_lua,
"box/console", console_lua,
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index b671eb7a2..5d818addf 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -21,6 +21,20 @@ local function locked(f)
end
end
+--
+-- When feedback is disabled, every single mentioning of it should
+-- be eliminated. Even box.cfg{} should not accept any
+-- 'feedback_*' parameters as valid. This is why they are set to
+-- nil, when the daemon does not exist.
+--
+local function ifdef_feedback(value)
+ return private.feedback_daemon ~= nil and value or nil
+end
+
+local ifdef_feedback_set_params =
+ private.feedback_daemon ~= nil and
+ private.feedback_daemon.set_feedback_params or nil
+
-- all available options
local default_cfg = {
listen = nil,
@@ -143,9 +157,9 @@ local template_cfg = {
replication_connect_quorum = 'number',
replication_skip_conflict = 'boolean',
replication_anon = 'boolean',
- feedback_enabled = 'boolean',
- feedback_host = 'string',
- feedback_interval = 'number',
+ feedback_enabled = ifdef_feedback('boolean'),
+ feedback_host = ifdef_feedback('string'),
+ feedback_interval = ifdef_feedback('number'),
net_msg_max = 'number',
sql_cache_size = 'number',
}
@@ -236,9 +250,9 @@ local dynamic_cfg = {
checkpoint_interval = private.cfg_set_checkpoint_interval,
checkpoint_wal_threshold = private.cfg_set_checkpoint_wal_threshold,
worker_pool_threads = private.cfg_set_worker_pool_threads,
- feedback_enabled = private.feedback_daemon.set_feedback_params,
- feedback_host = private.feedback_daemon.set_feedback_params,
- feedback_interval = private.feedback_daemon.set_feedback_params,
+ feedback_enabled = ifdef_feedback_set_params,
+ feedback_host = ifdef_feedback_set_params,
+ feedback_interval = ifdef_feedback_set_params,
-- do nothing, affects new replicas, which query this value on start
wal_dir_rescan_delay = function() end,
custom_proc_title = function()
@@ -258,6 +272,9 @@ local dynamic_cfg = {
sql_cache_size = private.cfg_set_sql_cache_size,
}
+ifdef_feedback = nil
+ifdef_feedback_set_params = nil
+
--
-- For some options it is important in which order they are set.
-- For example, setting 'replication', including self, before
diff --git a/src/trivia/config.h.cmake b/src/trivia/config.h.cmake
index 226f83128..89e0d39c6 100644
--- a/src/trivia/config.h.cmake
+++ b/src/trivia/config.h.cmake
@@ -251,6 +251,8 @@
/* Cacheline size to calculate alignments */
#define CACHELINE_SIZE 64
+#cmakedefine ENABLE_FEEDBACK_DAEMON 1
+
/*
* vim: syntax=c
*/
diff --git a/test/box-tap/feedback_daemon.test.lua b/test/box-tap/feedback_daemon.test.lua
index 9e6c59efc..d4adb71f1 100755
--- a/test/box-tap/feedback_daemon.test.lua
+++ b/test/box-tap/feedback_daemon.test.lua
@@ -8,8 +8,6 @@ local fiber = require('fiber')
local test = tap.test('feedback_daemon')
local socket = require('socket')
-test:plan(11)
-
box.cfg{log = 'report.log', log_level = 6}
local feedback_save = nil
@@ -49,10 +47,27 @@ local server = socket.tcp_server("127.0.0.1", 0, http_handle)
local port = server:name().port
local interval = 0.01
-box.cfg{
+local ok, err = pcall(box.cfg, {
feedback_host = string.format("127.0.0.1:%i", port),
feedback_interval = interval,
-}
+})
+
+if not ok then
+ --
+ -- gh-3308: feedback daemon is an optional pre-compile time
+ -- defined feature, depending on CMake flags. It is not
+ -- present always. When it is disabled, is should not be
+ -- visible anywhere.
+ --
+ test:plan(2)
+ test:diag('Feedback daemon is supposed to be disabled')
+ test:like(err, 'unexpected option', 'Feedback options are undefined')
+ test:isnil(box.feedback, 'No feedback daemon - no its API')
+ test:check()
+ os.exit(0)
+end
+
+test:plan(11)
local function check(message)
while feedback_count < 1 do
diff --git a/test/box/misc.result b/test/box/misc.result
index 9b76d7665..10699adc2 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -49,8 +49,24 @@ index = space:create_index('primary', { type = 'hash' })
---
- ' lua says: hello'
...
+--
+-- gh-3308: feedback daemon is an optional pre-compile time
+-- defined feature, depending on CMake flags. It is not present
+-- always.
+--
+optional = {feedback = true}
+---
+...
-- # What's in the box?
-t = {} for n in pairs(box) do table.insert(t, tostring(n)) end table.sort(t)
+t = {}
+---
+...
+for n in pairs(box) do \
+ if not optional[n] then \
+ table.insert(t, tostring(n)) \
+ end \
+end \
+table.sort(t)
---
...
t
@@ -64,7 +80,6 @@ t
- ctl
- error
- execute
- - feedback
- func
- index
- info
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index 350f9f75d..4d7fb0a16 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -17,8 +17,21 @@ index = space:create_index('primary', { type = 'hash' })
-- Test Lua from admin console. Whenever producing output,
-- make sure it's a valid YAML.
' lua says: hello'
+
+--
+-- gh-3308: feedback daemon is an optional pre-compile time
+-- defined feature, depending on CMake flags. It is not present
+-- always.
+--
+optional = {feedback = true}
-- # What's in the box?
-t = {} for n in pairs(box) do table.insert(t, tostring(n)) end table.sort(t)
+t = {}
+for n in pairs(box) do \
+ if not optional[n] then \
+ table.insert(t, tostring(n)) \
+ end \
+end \
+table.sort(t)
t
t = nil
--
2.21.1 (Apple Git-122.3)
More information about the Tarantool-patches
mailing list