From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (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 5E3334696C8 for ; Sun, 12 Apr 2020 23:13:36 +0300 (MSK) From: Vladislav Shpilevoy Date: Sun, 12 Apr 2020 22:13:30 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 4/4] feedback: add cmake option to disable the daemon List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, alexander.turenko@tarantool.org 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)