[patches] [cfg 1/1] cfg: Add constraints on box.cfg params

Ilya Markov imarkov at tarantool.org
Tue Mar 13 12:00:29 MSK 2018


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 creates error in case of its violation.
* Change log_nonblock default value to nil, which means default values
of log_nonblock corresponding to type of logger.
* Add box_getb function for receiving bool parameters from cfg.

Relates #3014 #3072
---
 src/box/box.cc                  | 42 +++++++++++++++++++++++-----
 src/box/lua/load_cfg.lua        |  2 +-
 src/cfg.c                       | 12 ++++++++
 src/cfg.h                       |  7 +++++
 src/main.cc                     |  2 +-
 src/say.c                       |  7 +++--
 src/say.h                       |  2 +-
 test/app-tap/init_script.result | 61 ++++++++++++++++++++---------------------
 test/box-tap/cfg.test.lua       | 15 ++++++++--
 test/box/admin.result           |  2 --
 test/box/cfg.result             |  4 ---
 11 files changed, 104 insertions(+), 52 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index 4cf7dd1..a3b9d80 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -327,18 +327,47 @@ apply_initial_join_row(struct xstream *stream, struct xrow_header *row)
 /* {{{ configuration bindings */
 
 static void
-box_check_log(const char *log)
+box_check_say()
 {
+	const char *log = cfg_gets("log");
 	if (log == NULL)
 		return;
+	enum say_logger_type type;
+	if (say_parse_logger_type(&log, &type) < 0) {
+		tnt_raise(ClientError, ER_CFG, "log",
+			  diag_last_error(diag_get())->errmsg);
+	}
+
 	if (say_check_init_str(log) == -1) {
-		if (diag_last_error(diag_get())->type ==
-		    &type_IllegalParams) {
-			tnt_raise(ClientError, ER_CFG, "log",
-				 diag_last_error(diag_get())->errmsg);
+
+		diag_raise();
+	}
+
+	if (type == SAY_LOGGER_SYSLOG) {
+		struct say_syslog_opts opts;
+		if (say_parse_syslog_opts(log, &opts) < 0) {
+			if (diag_last_error(diag_get())->type ==
+			    &type_IllegalParams) {
+				tnt_raise(ClientError, ER_CFG, "log",
+					  diag_last_error(diag_get())->errmsg);
+			}
 		}
+		say_free_syslog_opts(&opts);
 		diag_raise();
 	}
+
+	const char *log_format = cfg_gets("log_format");
+	enum say_format format = say_format_by_name(log_format);
+	if (format == say_format_MAX)
+		diag_set(ClientError, ER_CFG, "log_format",
+			 "expected 'plain' or 'json'");
+	if (type == SAY_LOGGER_SYSLOG && format == SF_JSON) {
+		tnt_raise(ClientError, ER_ILLEGAL_PARAMS, "log, log_format");
+	}
+	int log_nonblock = cfg_getb("log_nonblock");
+	if (log_nonblock == 1 && type == SAY_LOGGER_FILE) {
+		tnt_raise(ClientError, ER_ILLEGAL_PARAMS, "log, log_nonblock");
+	}
 }
 
 static enum say_format
@@ -536,8 +565,7 @@ void
 box_check_config()
 {
 	struct tt_uuid uuid;
-	box_check_log(cfg_gets("log"));
-	box_check_log_format(cfg_gets("log_format"));
+	box_check_say();
 	box_check_uri(cfg_gets("listen"), "listen");
 	box_check_instance_uuid(&uuid);
 	box_check_replicaset_uuid(&uuid);
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index d4f2128..9e70b8b 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -30,7 +30,7 @@ local default_cfg = {
     vinyl_page_size           = 8 * 1024,
     vinyl_bloom_fpr           = 0.05,
     log                 = nil,
-    log_nonblock        = true,
+    log_nonblock        = nil,
     log_level           = 5,
     log_format          = "plain",
     io_collect_interval = nil,
diff --git a/src/cfg.c b/src/cfg.c
index 7c7d6e7..78654be 100644
--- a/src/cfg.c
+++ b/src/cfg.c
@@ -57,6 +57,18 @@ cfg_geti(const char *param)
 }
 
 int
+cfg_getb(const char *param)
+{
+	cfg_get(param);
+	int val;
+	if (lua_isnil(tarantool_L, -1))
+		return -1;
+	val = lua_toboolean(tarantool_L, -1);
+	lua_pop(tarantool_L, 1);
+	return val;
+}
+
+int
 cfg_geti_default(const char *param, int default_val)
 {
 	cfg_get(param);
diff --git a/src/cfg.h b/src/cfg.h
index 8499388..140bddd 100644
--- a/src/cfg.h
+++ b/src/cfg.h
@@ -40,6 +40,13 @@ extern "C" {
 int
 cfg_geti(const char *param);
 
+/**
+ * Gets boolean parameter of cfg.
+ * Returns -1 in case of nil
+ */
+int
+cfg_getb(const char *param);
+
 int
 cfg_geti_default(const char *param, int default_val);
 
diff --git a/src/main.cc b/src/main.cc
index ec06103..0c198b3 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -498,7 +498,7 @@ load_cfg()
 	 */
 	say_logger_init(log,
 			cfg_geti("log_level"),
-			cfg_geti("log_nonblock"),
+			cfg_getb("log_nonblock"),
 			log_format,
 			background);
 	systemd_init();
diff --git a/src/say.c b/src/say.c
index 65808a6..8c591ab 100644
--- a/src/say.c
+++ b/src/say.c
@@ -30,6 +30,7 @@
  */
 #include "say.h"
 #include "fiber.h"
+#include "cfg.h"
 
 #include <errno.h>
 #include <stdarg.h>
@@ -456,14 +457,13 @@ log_syslog_init(struct log *log, const char *init_str)
  * Initialize logging subsystem to use in daemon mode.
  */
 int
-log_create(struct log *log, const char *init_str, bool nonblock)
+log_create(struct log *log, const char *init_str, int nonblock)
 {
 	log->pid = 0;
 	log->syslog_ident = NULL;
 	log->path = NULL;
 	log->format_func = NULL;
 	log->level = S_INFO;
-	log->nonblock = nonblock;
 	setvbuf(stderr, NULL, _IONBF, 0);
 
 	if (init_str != NULL) {
@@ -475,13 +475,16 @@ log_create(struct log *log, const char *init_str, bool nonblock)
 		int rc;
 		switch (type) {
 		case SAY_LOGGER_PIPE:
+			log->nonblock = (nonblock >= 0)? nonblock: true;
 			rc = log_pipe_init(log, init_str);
 			break;
 		case SAY_LOGGER_SYSLOG:
+			log->nonblock = (nonblock >= 0)? nonblock: true;
 			rc = log_syslog_init(log, init_str);
 			break;
 		case SAY_LOGGER_FILE:
 		default:
+			log->nonblock = (nonblock >= 0)? nonblock: false;
 			rc = log_file_init(log, init_str);
 			break;
 		}
diff --git a/src/say.h b/src/say.h
index 1b63229..6a65960 100644
--- a/src/say.h
+++ b/src/say.h
@@ -131,7 +131,7 @@ struct log {
  * the diagnostics area
  */
 int
-log_create(struct log *log, const char *init_str, bool nonblock);
+log_create(struct log *log, const char *init_str, int nonblock);
 
 void
 log_destroy(struct log *log);
diff --git a/test/app-tap/init_script.result b/test/app-tap/init_script.result
index 80153e3..1091a9d 100644
--- a/test/app-tap/init_script.result
+++ b/test/app-tap/init_script.result
@@ -13,37 +13,36 @@ box.cfg
 8	log:tarantool.log
 9	log_format:plain
 10	log_level:5
-11	log_nonblock:true
-12	memtx_dir:.
-13	memtx_max_tuple_size:1048576
-14	memtx_memory:107374182
-15	memtx_min_tuple_size:16
-16	pid_file:box.pid
-17	read_only:false
-18	readahead:16320
-19	replication_connect_timeout:4
-20	replication_sync_lag:10
-21	replication_timeout:1
-22	rows_per_wal:500000
-23	slab_alloc_factor:1.05
-24	too_long_threshold:0.5
-25	vinyl_bloom_fpr:0.05
-26	vinyl_cache:134217728
-27	vinyl_dir:.
-28	vinyl_max_tuple_size:1048576
-29	vinyl_memory:134217728
-30	vinyl_page_size:8192
-31	vinyl_range_size:1073741824
-32	vinyl_read_threads:1
-33	vinyl_run_count_per_level:2
-34	vinyl_run_size_ratio:3.5
-35	vinyl_timeout:60
-36	vinyl_write_threads:2
-37	wal_dir:.
-38	wal_dir_rescan_delay:2
-39	wal_max_size:268435456
-40	wal_mode:write
-41	worker_pool_threads:4
+11	memtx_dir:.
+12	memtx_max_tuple_size:1048576
+13	memtx_memory:107374182
+14	memtx_min_tuple_size:16
+15	pid_file:box.pid
+16	read_only:false
+17	readahead:16320
+18	replication_connect_timeout:4
+19	replication_sync_lag:10
+20	replication_timeout:1
+21	rows_per_wal:500000
+22	slab_alloc_factor:1.05
+23	too_long_threshold:0.5
+24	vinyl_bloom_fpr:0.05
+25	vinyl_cache:134217728
+26	vinyl_dir:.
+27	vinyl_max_tuple_size:1048576
+28	vinyl_memory:134217728
+29	vinyl_page_size:8192
+30	vinyl_range_size:1073741824
+31	vinyl_read_threads:1
+32	vinyl_run_count_per_level:2
+33	vinyl_run_size_ratio:3.5
+34	vinyl_timeout:60
+35	vinyl_write_threads:2
+36	wal_dir:.
+37	wal_dir_rescan_delay:2
+38	wal_max_size:268435456
+39	wal_mode:write
+40	worker_pool_threads:4
 --
 -- Test insert from detached fiber
 --
diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
index 453b616..8496929 100755
--- a/test/box-tap/cfg.test.lua
+++ b/test/box-tap/cfg.test.lua
@@ -6,7 +6,7 @@ local socket = require('socket')
 local fio = require('fio')
 local uuid = require('uuid')
 local msgpack = require('msgpack')
-test:plan(89)
+test:plan(91)
 
 --------------------------------------------------------------------------------
 -- Invalid values
@@ -47,6 +47,14 @@ invalid('vinyl_run_size_ratio', 1)
 invalid('vinyl_bloom_fpr', 0)
 invalid('vinyl_bloom_fpr', 1.1)
 
+local function invalid_combinations(name, val)
+    local status, result = pcall(box.cfg, val)
+    test:ok(not status and result:match('Illegal'), 'invalid '..name)
+end
+
+invalid_combinations("log, log_nonblock", {log = "1.log", log_nonblock = true})
+invalid_combinations("log, log_format", {log = "syslog:identity=tarantool", log_format = 'json'})
+
 test:is(type(box.cfg), 'function', 'box is not started')
 
 --------------------------------------------------------------------------------
@@ -79,6 +87,7 @@ test:ok(status and result[1] == 1, "box.tuple without box.cfg")
 os.execute("rm -rf vinyl")
 box.cfg{
     log="tarantool.log",
+    log_nonblock=false,
     memtx_memory=104857600,
     wal_mode = "", -- "" means default value
 }
@@ -163,7 +172,7 @@ function run_script(code)
 end
 
 -- gh-715: Cannot switch to/from 'fsync'
-code = [[ box.cfg{ log="tarantool.log", wal_mode = 'fsync' }; ]]
+code = [[ box.cfg{ log="tarantool.log", log_nonblock = false, wal_mode = 'fsync' }; ]]
 test:is(run_script(code), 0, 'wal_mode fsync')
 
 code = [[ box.cfg{ wal_mode = 'fsync' }; box.cfg { wal_mode = 'fsync' }; ]]
@@ -190,7 +199,7 @@ test:is(run_script(code), PANIC, 'snap_dir is invalid')
 code = [[ box.cfg{ wal_dir='invalid' } ]]
 test:is(run_script(code), PANIC, 'wal_dir is invalid')
 
-test:is(box.cfg.log_nonblock, true, "log_nonblock default value")
+test:isnil(box.cfg.log_nonblock, "log_nonblock default value")
 code = [[
 box.cfg{log_nonblock = false }
 os.exit(box.cfg.log_nonblock == false and 0 or 1)
diff --git a/test/box/admin.result b/test/box/admin.result
index 7a3e937..0743c4d 100644
--- a/test/box/admin.result
+++ b/test/box/admin.result
@@ -38,8 +38,6 @@ cfg_filter(box.cfg)
     - plain
   - - log_level
     - 5
-  - - log_nonblock
-    - true
   - - memtx_dir
     - <hidden>
   - - memtx_max_tuple_size
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 67539cd..68c0dee 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -34,8 +34,6 @@ cfg_filter(box.cfg)
     - plain
   - - log_level
     - 5
-  - - log_nonblock
-    - true
   - - memtx_dir
     - <hidden>
   - - memtx_max_tuple_size
@@ -123,8 +121,6 @@ cfg_filter(box.cfg)
     - plain
   - - log_level
     - 5
-  - - log_nonblock
-    - true
   - - memtx_dir
     - <hidden>
   - - memtx_max_tuple_size
-- 
2.7.4




More information about the Tarantool-patches mailing list