[Tarantool-patches] [PATCH v7 07/11] lua/log: use log module settings inside box.cfg
Oleg Babin
olegrok at tarantool.org
Tue Jun 2 10:51:28 MSK 2020
Hi! Thanks for your patch.
I've found a strange case:
```
tarantool> log.cfg({log = '1.txt'})
---
...
tarantool> log.cfg({log = '2.txt'}) -- error - OK
---
- error: 'builtin/log.lua:273: log: ''log'' can''t be set dynamically'
...
tarantool> box.cfg{log = '2.txt'} -- no error (why?)
---
...
tarantool> log.error('test')
---
...
```
log.cfg and box.cfg shows:
```
tarantool> log.cfg.log
---
- 2.txt
...
tarantool> box.cfg.log
---
- 2.txt
...
```
But actually "1.txt" is my log file.
Also I suggest to cover such situation with a test :)
On 02/06/2020 01:25, Cyrill Gorcunov wrote:
> Currently box module carries configuration settings in box.cfg
> variable which is created dinamically on demand.
>
> The default values are kept in default_cfg variable. Since we're
> going to make the log module to work on its own, we need it to
> provide default settings to the box.cfg interface.
>
> For this sake we export log:box_api table which the main box
> module use when needed.
>
> Part-of #689
>
> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
> ---
> src/box/lua/load_cfg.lua | 26 ++++++++++++++----
> src/lua/log.lua | 59 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 78 insertions(+), 7 deletions(-)
>
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 5d818addf..7612deb90 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -59,10 +59,6 @@ local default_cfg = {
> vinyl_range_size = nil, -- set automatically
> vinyl_page_size = 8 * 1024,
> vinyl_bloom_fpr = 0.05,
> - log = nil,
> - log_nonblock = nil,
> - log_level = 5,
> - log_format = "plain",
> io_collect_interval = nil,
> readahead = 16320,
> snap_io_rate_limit = nil, -- no limit
> @@ -233,8 +229,8 @@ end
> local dynamic_cfg = {
> listen = private.cfg_set_listen,
> replication = private.cfg_set_replication,
> - log_level = private.cfg_set_log_level,
> - log_format = private.cfg_set_log_format,
> + log_level = log.box_api.set_log_level,
> + log_format = log.box_api.set_log_format,
> io_collect_interval = private.cfg_set_io_collect_interval,
> readahead = private.cfg_set_readahead,
> too_long_threshold = private.cfg_set_too_long_threshold,
> @@ -470,6 +466,21 @@ local function apply_default_cfg(cfg, default_cfg)
> end
> end
>
> +-- Fetch default settings from modules.
> +local function modules_apply_default_cfg(cfg)
> + log.box_api.on_apply_default_cfg(cfg)
> +end
> +
> +-- Propagate settings to modules.
> +local function modules_load_cfg(cfg)
> + log.box_api.on_load_cfg(cfg)
> +end
> +
> +-- Propagate settings to modules.
> +local function modules_reload_cfg(cfg)
> + log.box_api.on_reload_cfg(cfg)
> +end
> +
> -- Return true if two configurations are equivalent.
> local function compare_cfg(cfg1, cfg2)
> if type(cfg1) ~= type(cfg2) then
> @@ -517,6 +528,7 @@ local function reload_cfg(oldcfg, cfg)
> json.encode(val))
> end
> end
> + modules_reload_cfg(cfg)
> if type(box.on_reload_configuration) == 'function' then
> box.on_reload_configuration()
> end
> @@ -554,6 +566,7 @@ local function load_cfg(cfg)
> cfg = upgrade_cfg(cfg, translate_cfg)
> cfg = prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)
> apply_default_cfg(cfg, default_cfg);
> + modules_apply_default_cfg(cfg)
> -- Save new box.cfg
> box.cfg = cfg
> if not pcall(private.cfg_check) then
> @@ -573,6 +586,7 @@ local function load_cfg(cfg)
> end,
> __call = locked(reload_cfg),
> })
> + modules_load_cfg(box.cfg)
> private.cfg_load()
> for key, fun in pairs(dynamic_cfg) do
> local val = cfg[key]
> diff --git a/src/lua/log.lua b/src/lua/log.lua
> index 746e0d82f..33e98fd84 100644
> --- a/src/lua/log.lua
> +++ b/src/lua/log.lua
> @@ -101,6 +101,24 @@ local function fmt_list()
> return table.concat(keyset, ',')
> end
>
> +-- Default options. The keys are part of
> +-- user API, so change with caution.
> +local log_cfg = {
> + log = nil,
> + nonblock = nil,
> + level = S_INFO,
> + format = fmt_num2str[ffi.C.SF_PLAIN],
> +}
> +
> +-- Name mapping from box to log module.
> +-- Make sure all required fields are covered!
> +local log2box_keys = {
> + ['log'] = 'log',
> + ['nonblock'] = 'log_nonblock',
> + ['level'] = 'log_level',
> + ['format'] = 'log_format',
> +}
> +
> local function say(level, fmt, ...)
> if ffi.C.log_level < level then
> -- don't waste cycles on debug.getinfo()
> @@ -150,7 +168,8 @@ local function log_rotate()
> end
>
> local function log_level(level)
> - return ffi.C.say_set_log_level(level)
> + ffi.C.say_set_log_level(level)
> + rawset(log_cfg, 'level', level)
> end
>
> local function log_format(name)
> @@ -170,12 +189,31 @@ local function log_format(name)
> else
> ffi.C.say_set_log_format(ffi.C.SF_PLAIN)
> end
> + rawset(log_cfg, 'format', name)
> end
>
> local function log_pid()
> return tonumber(ffi.C.log_pid)
> end
>
> +-- Apply defaut config to the box module
> +local function box_on_apply_default_cfg(box_cfg)
> + for k, v in pairs(log2box_keys) do
> + if box_cfg[v] == nil then
> + box_cfg[v] = log_cfg[k]
> + end
> + end
> +end
> +
> +-- Update own values from box interface.
> +local function box_on_load_cfg(box_cfg)
> + for k, v in pairs(log2box_keys) do
> + if box_cfg[v] ~= log_cfg[k] then
> + log_cfg[k] = box_cfg[v]
> + end
> + end
> +end
> +
> local compat_warning_said = false
> local compat_v16 = {
> logger_pid = function()
> @@ -197,9 +235,28 @@ local log = {
> pid = log_pid,
> level = log_level,
> log_format = log_format,
> +
> + -- Internal API to box module, not for users,
> + -- names can be changed.
> + box_api = {
> + set_log_level = function()
> + log_level(box.cfg.log_level)
> + end,
> + set_log_format = function()
> + log_format(box.cfg.log_format)
> + end,
> + on_load_cfg = box_on_load_cfg,
> + on_reload_cfg = box_on_load_cfg,
> + on_apply_default_cfg = box_on_apply_default_cfg,
> + },
> }
>
> setmetatable(log, {
> + __serialize = function(self)
> + local res = table.copy(self)
> + res.box_api = nil
> + return setmetatable(res, {})
> + end,
> __index = compat_v16;
> })
>
>
More information about the Tarantool-patches
mailing list