[patches] [cfg 1/1] cfg: Add constraints on box.cfg params
Konstantin Osipov
kostja at tarantool.org
Mon Mar 5 19:31:46 MSK 2018
* imarkov <imarkov at tarantool.org> [18/02/22 13:20]:
> From: IlyaMarkovMipt <markovilya197 at gmail.com>
> * Introduce limitations on combinations of box.cfg parameters
> * Add restriction on log type file and log_nonblock=true
> * Add restriction on log type syslog and log_format json
> * Each restriction logs warning in case of its violation
Can we change the default log_nonblock to nil, which means the
default is derived from log type, true for pipe/syslog, false
for file/stdout?
This would spare us from the useless warning. The patch will go to
2.x after that.
What was the issue with moving the check for valid combinations
to box_check_config()?
How does the check work when log_nonblock or some other
combination becomes invalid after initial configuration, e.g. in
this scenario:
box.cfg{}
box.cfg{log_nonblock=true}?
Why did you have to change so many tests, most of them use the
default log which is stdout/nil?
> Relates #3014 #3072
> ---
> src/box/lua/load_cfg.lua | 51 ++++++++++++++++++++++++++++++++
> test/app-tap/init_script.result | 2 +-
> test/app-tap/init_script.test.lua | 3 +-
> test/app-tap/logger.test.lua | 5 +---
> test/app/app.lua | 3 +-
> test/box-py/box.lua | 3 +-
> test/box-tap/auth.test.lua | 1 +
> test/box-tap/cfg.test.lua | 31 ++++++++++++++-----
> test/box-tap/cfgup.test.lua | 3 +-
> test/box-tap/session.test.lua | 1 +
> test/box-tap/trigger_atexit.test.lua | 1 +
> test/box-tap/trigger_yield.test.lua | 3 +-
> test/box/admin.result | 2 +-
> test/box/backup_test.lua | 2 +-
> test/box/box.lua | 1 +
> test/box/cfg.result | 4 +--
> test/box/lua/cfg_bad_vinyl_dir.lua | 3 +-
> test/box/lua/cfg_test1.lua | 1 +
> test/box/lua/cfg_test2.lua | 3 +-
> test/box/lua/cfg_test3.lua | 1 +
> test/box/lua/cfg_test4.lua | 1 +
> test/box/proxy.lua | 3 +-
> test/box/tiny.lua | 3 +-
> test/engine/box.lua | 1 +
> test/engine_long/box.lua | 1 +
> test/long_run-py/box.lua | 1 +
> test/replication-py/failed.lua | 1 +
> test/replication-py/master.lua | 1 +
> test/replication-py/replica.lua | 1 +
> test/replication/autobootstrap.lua | 1 +
> test/replication/autobootstrap_guest.lua | 1 +
> test/replication/hot_standby.lua | 1 +
> test/replication/master.lua | 1 +
> test/replication/on_replace.lua | 1 +
> test/replication/quorum.lua | 1 +
> test/replication/replica.lua | 1 +
> test/replication/replica_ack.lua | 1 +
> test/replication/replica_timeout.lua | 1 +
> test/replication/replica_uuid.lua | 1 +
> test/replication/wal_off.lua | 1 +
> test/vinyl/bad_run_indexes.lua | 1 +
> test/vinyl/force_recovery.lua | 1 +
> test/vinyl/info.lua | 1 +
> test/vinyl/join_quota.lua | 1 +
> test/vinyl/low_quota.lua | 1 +
> test/vinyl/upgrade.lua | 1 +
> test/vinyl/vinyl.lua | 1 +
> test/wal_off/wal.lua | 3 +-
> test/xlog-py/box.lua | 3 +-
> test/xlog/force_recovery.lua | 3 +-
> test/xlog/panic.lua | 3 +-
> test/xlog/replica.lua | 1 +
> test/xlog/upgrade.lua | 3 +-
> test/xlog/xlog.lua | 3 +-
> 54 files changed, 142 insertions(+), 32 deletions(-)
>
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 4ac0408..12ebff1 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -343,6 +343,28 @@ local box_cfg_guard_whitelist = {
> NULL = true;
> };
>
> +local logger_types = {
> + LOGGER_FILE = 1,
> + LOGGER_PIPE = 2,
> + LOGGER_SYSLOG = 3
> +}
Please use singular, logger_type_map.
> +
> +local function parse_logger_type(log)
> + if log == nil then
> + return nil
> + end
> + if log:match("^|") or log:match("^pipe:") then
> + return logger_types.LOGGER_PIPE
> + elseif log:match("^syslog:") then
> + return logger_types.LOGGER_SYSLOG
> + elseif log:match("^file:") or not log:match("^:") then
> + return logger_types.LOGGER_FILE
> + else
> + return box.error(box.error.ILLEGAl_PARAMS,
> + "expecting a file name or a prefix, such as '|', 'pipe:', 'syslog:'")
> + end
> +end
> +
> local box = require('box')
> -- Move all box members except 'error' to box_configured
> local box_configured = {}
> @@ -360,6 +382,34 @@ setmetatable(box, {
> end
> })
>
> +-- List of combinations that are prohibited in cfg
> +-- Each combination consists of list of parameters descriptions
> +-- Each parameter description includes parameter name, its value and
> +-- optionally function that converts box.cfg option to comparable value
> +local box_cfg_contrary_combinations = {
> + {{"log_format", "json"}, {"log", logger_types.LOGGER_SYSLOG, parse_logger_type}},
> + {{"log_nonblock", true}, {"log", logger_types.LOGGER_FILE, parse_logger_type}}
> +}
> +
> +local function verify_combinations(contrary_combinations)
> + for _, combination in pairs(contrary_combinations) do
> + local params = {}
> + for _, parameter in pairs(combination) do
> + local value = box.cfg[parameter[1]]
> + if parameter[3] ~= nil then
> + value = parameter[3](value)
> + end
> + if value ~= parameter[2] then
> + goto not_match
> + end
> + table.insert(params, parameter[1])
> + end
> + log.warn("wrong combination of " ..
> + table.concat(params, ", "))
> + ::not_match::
> + end
> +end
After giving this some thought, the only reason we're doing it here
is that we failed to refactor say.c and separate log string parser api
so that it can be called independently from box_check_config. Am
I right?
<cut> a lot of changes which will become unnecessary if we change
log_nonblock default after moving this problem to 2.0.
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.org - www.twitter.com/kostja_osipov
More information about the Tarantool-patches
mailing list