From 05e2b278a84b166105f78127f5e4e5cb41d1dd21 Mon Sep 17 00:00:00 2001
From: Roman Tsisyk <roman@tsisyk.com>
Date: Thu, 29 Jan 2015 15:12:12 +0300
Subject: [PATCH] Fix #635: Allow dynamic box.cfg { listen = xx } option
---
src/box/box.cc | 48 +++++++++++++++++++++------------
src/box/box.h | 3 +++
src/box/errcode.h | 2 +-
src/box/iproto.cc | 17 ++++--------
src/box/iproto.h | 2 +-
src/box/lua/load_cfg.lua | 12 ++++++---
src/box/lua/snapshot_daemon.lua | 3 +--
src/coio.cc | 3 +--
src/coio.h | 1 -
src/evio.cc | 29 +++++++++++---------
src/evio.h | 9 +++++--
src/ffisyms.cc | 1 +
test/app/cfg.result | 5 +++-
test/app/cfg.test.lua | 19 +++++++++++--
test/box/cfg.result | 12 +++++----
test/box/snapshot_daemon.result | 2 +-
16 files changed, 104 insertions(+), 64 deletions(-)
diff --git a/src/box/box.cc b/src/box/box.cc
index 139796c8cd..7fb44b1946 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -50,12 +50,15 @@
#include "user.h"
#include "cfg.h"
#include "iobuf.h"
+#include "evio.h"
static void process_ro(struct request *request, struct port *port);
box_process_func box_process = process_ro;
struct recovery_state *recovery;
+static struct evio_service binary; /* iproto binary listener */
+
int snapshot_pid = 0; /* snapshot processes pid */
static void
process_ro(struct request *request, struct port *port)
@@ -94,8 +97,8 @@ recover_row(struct recovery_state *r, void *param, struct xrow_header *row)
/* {{{ configuration bindings */
-void
-box_check_replication_source(const char *source)
+static void
+box_check_uri(const char *source, const char *what)
{
if (source == NULL)
return;
@@ -103,7 +106,7 @@ box_check_replication_source(const char *source)
/* URI format is [host:]service */
if (uri_parse(&uri, source) || !uri.service) {
- tnt_raise(ClientError, ER_CFG, "replication source, "
+ tnt_raise(ClientError, ER_CFG, what,
"expected host:service or /unix.socket");
}
}
@@ -113,29 +116,28 @@ box_check_wal_mode(const char *mode_name)
{
assert(mode_name != NULL); /* checked in Lua */
int mode = strindex(wal_mode_STRS, mode_name, WAL_MODE_MAX);
- if (mode == WAL_MODE_MAX) {
- tnt_raise(ClientError, ER_CFG,
- "wal_mode is not recognized");
- }
+ if (mode == WAL_MODE_MAX)
+ tnt_raise(ClientError, ER_CFG, "wal_mode", mode_name);
}
void
box_check_config()
{
box_check_wal_mode(cfg_gets("wal_mode"));
- box_check_replication_source(cfg_gets("replication_source"));
+ box_check_uri(cfg_gets("listen"), "listen");
+ box_check_uri(cfg_gets("replication_source"), "replication_source");
/* check rows_per_wal configuration */
if (cfg_geti("rows_per_wal") <= 1) {
- tnt_raise(ClientError, ER_CFG,
- "rows_per_wal must be greater than one");
+ tnt_raise(ClientError, ER_CFG, "rows_per_wal",
+ "must be greater than one");
}
}
extern "C" void
box_set_replication_source(const char *source)
{
- box_check_replication_source(source);
+ box_check_uri(source, "replication_source");
bool old_is_replica = recovery->remote.reader;
bool new_is_replica = source != NULL;
@@ -162,6 +164,20 @@ box_set_replication_source(const char *source)
}
}
+extern "C" void
+box_set_listen(const char *uri)
+{
+ box_check_uri(uri, "listen");
+ if (evio_service_is_active(&binary))
+ evio_service_stop(&binary);
+
+ if (uri != NULL) {
+ evio_service_start(&binary, uri);
+ } else {
+ box_leave_local_standby_mode(NULL);
+ }
+}
+
extern "C" void
box_set_wal_mode(const char *mode_name)
{
@@ -170,8 +186,8 @@ box_set_wal_mode(const char *mode_name)
strindex(wal_mode_STRS, mode_name, WAL_MODE_MAX);
if (mode != recovery->wal_mode &&
(mode == WAL_FSYNC || recovery->wal_mode == WAL_FSYNC)) {
- tnt_raise(ClientError, ER_CFG,
- "wal_mode cannot switch to/from fsync");
+ tnt_raise(ClientError, ER_CFG, "wal_mode",
+ "cannot switch to/from fsync");
}
recovery_update_mode(recovery, mode);
}
@@ -455,15 +471,13 @@ box_init()
cfg_getd("wal_dir_rescan_delay"));
title("hot_standby", NULL);
+ iproto_init(&binary);
const char *listen = cfg_gets("listen");
/*
* application server configuration).
*/
- if (listen) {
- iproto_init(listen);
- } else {
+ if (listen == NULL)
box_leave_local_standby_mode(NULL);
- }
iobuf_set_readahead(cfg_geti("readahead"));
}
diff --git a/src/box/box.h b/src/box/box.h
index 4729c99d10..a7a121db5a 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -116,6 +116,9 @@ box_process_subscribe(int fd, struct xrow_header *header);
void
box_check_config();
+void
+box_set_listen(const char *uri);
+
void
box_set_replication_source(const char *source);
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 7d0f973c1a..3ab47f5754 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -110,7 +110,7 @@ struct errcode_record {
/* 56 */_(ER_USER_MAX, 2, "A limit on the total number of users has been reached: %u") \
/* 57 */_(ER_NO_SUCH_ENGINE, 2, "Space engine '%s' does not exist") \
/* 58 */_(ER_RELOAD_CFG, 2, "Can't set option '%s' dynamically") \
- /* 59 */_(ER_CFG, 2, "Incorrect option value: %s") \
+ /* 59 */_(ER_CFG, 2, "Incorrect '%s' option value: %s") \
/* 60 */_(ER_SOPHIA, 2, "%s") \
/* 61 */_(ER_LOCAL_SERVER_IS_NOT_ACTIVE,2, "Local server is not active") \
/* 62 */_(ER_UNKNOWN_SERVER, 2, "Server %u is not registered with the cluster") \
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index c9987c62c2..48d000965f 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -821,24 +821,17 @@ static void on_bind(void *arg __attribute__((unused)))
/** Initialize a read-write port. */
void
-iproto_init(const char *uri)
+iproto_init(struct evio_service *service)
{
- /* Run a primary server. */
- if (!uri)
- return;
-
- static struct evio_service primary;
- evio_service_init(loop(), &primary, "primary",
- uri,
- iproto_on_accept, NULL);
- evio_service_on_bind(&primary, on_bind, NULL);
- evio_service_start(&primary);
-
mempool_create(&iproto_request_pool, &cord()->slabc,
sizeof(struct iproto_request));
iproto_queue_init(&request_queue);
mempool_create(&iproto_connection_pool, &cord()->slabc,
sizeof(struct iproto_connection));
+
+ evio_service_init(loop(), service, "binary",
+ iproto_on_accept, NULL);
+ evio_service_on_bind(service, on_bind, NULL);
}
/* vim: set foldmethod=marker */
diff --git a/src/box/iproto.h b/src/box/iproto.h
index 9df10ceaa9..fe7d5db3cf 100644
--- a/src/box/iproto.h
+++ b/src/box/iproto.h
@@ -29,5 +29,5 @@
* SUCH DAMAGE.
*/
void
-iproto_init(const char *uri);
+iproto_init(struct evio_service *service);
#endif
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index ba8d14e7f7..2ad9abc389 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -5,6 +5,7 @@ ffi.cdef([[
void check_cfg();
void load_cfg();
void box_set_wal_mode(const char *mode);
+void box_set_listen(const char *uri);
void box_set_replication_source(const char *source);
void box_set_log_level(int level);
void box_set_io_collect_interval(double interval);
@@ -114,6 +115,7 @@ local modify_cfg = {
-- dynamically settable options
local dynamic_cfg = {
wal_mode = ffi.C.box_set_wal_mode,
+ listen = ffi.C.box_set_listen,
replication_source = ffi.C.box_set_replication_source,
log_level = ffi.C.box_set_log_level,
io_collect_interval = ffi.C.box_set_io_collect_interval,
@@ -185,14 +187,16 @@ local function apply_default_cfg(cfg, default_cfg)
end
end
-local function reload_cfg(oldcfg, newcfg)
- newcfg = prepare_cfg(newcfg, default_cfg, template_cfg, modify_cfg)
- for key, val in pairs(newcfg) do
+local function reload_cfg(oldcfg, cfg)
+ local newcfg = prepare_cfg(cfg, default_cfg, template_cfg, modify_cfg)
+ -- iterate over original table because prepare_cfg() may store NILs
+ for key in pairs(cfg) do
if dynamic_cfg[key] == nil then
box.error(box.error.RELOAD_CFG, key);
end
end
- for key, val in pairs(newcfg) do
+ for key in pairs(cfg) do
+ local val = newcfg[key]
if oldcfg[key] ~= val then
dynamic_cfg[key](val)
rawset(oldcfg, key, val)
diff --git a/src/box/lua/snapshot_daemon.lua b/src/box/lua/snapshot_daemon.lua
index 397069b2d3..5e281e3371 100644
--- a/src/box/lua/snapshot_daemon.lua
+++ b/src/box/lua/snapshot_daemon.lua
@@ -227,8 +227,7 @@ do
set_snapshot_count = function(snapshot_count)
if math.floor(snapshot_count) ~= snapshot_count then
- box.error(box.error.CFG,
- "snapshot_count must be integer")
+ box.error(box.error.CFG, "snapshot_count", "must be integer")
end
daemon.snapshot_count = snapshot_count
reload(daemon)
diff --git a/src/coio.cc b/src/coio.cc
index 81d7c4dbab..8e4f107fdc 100644
--- a/src/coio.cc
+++ b/src/coio.cc
@@ -657,10 +657,9 @@ coio_service_on_accept(struct evio_service *evio_service,
void
coio_service_init(struct coio_service *service, const char *name,
- const char *uri,
void (*handler)(va_list ap), void *handler_param)
{
- evio_service_init(loop(), &service->evio_service, name, uri,
+ evio_service_init(loop(), &service->evio_service, name,
coio_service_on_accept, service);
service->handler = handler;
service->handler_param = handler_param;
diff --git a/src/coio.h b/src/coio.h
index 715a841f42..417f05911f 100644
--- a/src/coio.h
+++ b/src/coio.h
@@ -151,7 +151,6 @@ coio_recvfrom_timeout(struct ev_io *coio, void *buf, size_t sz, int flags,
void
coio_service_init(struct coio_service *service, const char *name,
- const char *uri,
void (*handler)(va_list ap), void *handler_param);
diff --git a/src/evio.cc b/src/evio.cc
index 1d3cbead4a..211d473329 100644
--- a/src/evio.cc
+++ b/src/evio.cc
@@ -285,7 +285,6 @@ evio_service_timer_cb(ev_loop *loop, ev_timer *watcher, int /* revents */)
void
evio_service_init(ev_loop *loop,
struct evio_service *service, const char *name,
- const char *uri,
void (*on_accept)(struct evio_service *, int,
struct sockaddr *, socklen_t),
void *on_accept_param)
@@ -295,17 +294,6 @@ evio_service_init(ev_loop *loop,
service->loop = loop;
- struct uri u;
- if (uri_parse(&u, uri) || u.service == NULL)
- tnt_raise(SocketError, -1, "invalid uri for bind: %s", uri);
-
- snprintf(service->serv, sizeof(service->serv), "%.*s",
- (int) u.service_len, u.service);
- if (u.host != NULL && strncmp(u.host, "*", u.host_len) != 0) {
- snprintf(service->host, sizeof(service->host), "%.*s",
- (int) u.host_len, u.host);
- } /* else { service->host[0] = '\0'; } */
-
service->on_accept = on_accept;
service->on_accept_param = on_accept_param;
/*
@@ -323,10 +311,23 @@ evio_service_init(ev_loop *loop,
* binding periodically.
*/
void
-evio_service_start(struct evio_service *service)
+evio_service_start(struct evio_service *service, const char *uri)
{
+ struct uri u;
+ if (uri_parse(&u, uri) || u.service == NULL)
+ tnt_raise(SocketError, -1, "invalid uri for bind: %s", uri);
+
+ snprintf(service->serv, sizeof(service->serv), "%.*s",
+ (int) u.service_len, u.service);
+ if (u.host != NULL && strncmp(u.host, "*", u.host_len) != 0) {
+ snprintf(service->host, sizeof(service->host), "%.*s",
+ (int) u.host_len, u.host);
+ } /* else { service->host[0] = '\0'; } */
+
assert(! ev_is_active(&service->ev));
+ say_info("%s: started", evio_service_name(service));
+
if (evio_service_bind_and_listen(service)) {
/* Try again after a delay. */
say_warn("%s: %s is already in use, will "
@@ -345,6 +346,8 @@ evio_service_start(struct evio_service *service)
void
evio_service_stop(struct evio_service *service)
{
+ say_info("%s: stopped", evio_service_name(service));
+
if (! ev_is_active(&service->ev)) {
ev_timer_stop(service->loop, &service->timer);
} else {
diff --git a/src/evio.h b/src/evio.h
index 906b968ca8..f0ee5be47d 100644
--- a/src/evio.h
+++ b/src/evio.h
@@ -101,7 +101,6 @@ struct evio_service
void
evio_service_init(ev_loop *loop,
struct evio_service *service, const char *name,
- const char *uri,
void (*on_accept)(struct evio_service *,
int, struct sockaddr *, socklen_t),
void *on_accept_param);
@@ -118,7 +117,7 @@ evio_service_on_bind(struct evio_service *service,
/** Bind to the port and begin listening. */
void
-evio_service_start(struct evio_service *service);
+evio_service_start(struct evio_service *service, const char *uri);
/** If started, stop event flow and close the acceptor socket. */
void
@@ -130,6 +129,12 @@ evio_socket(struct ev_io *coio, int domain, int type, int protocol);
void
evio_close(ev_loop *loop, struct ev_io *evio);
+static inline bool
+evio_service_is_active(struct evio_service *service)
+{
+ return ev_is_active(&service->ev) || ev_is_active(&service->timer);
+}
+
static inline bool
evio_is_active(struct ev_io *ev)
{
diff --git a/src/ffisyms.cc b/src/ffisyms.cc
index 3e9f0f65b1..7915561f3d 100644
--- a/src/ffisyms.cc
+++ b/src/ffisyms.cc
@@ -41,6 +41,7 @@ void *ffi_symbols[] = {
(void *) password_prepare,
(void *) tarantool_error_message,
(void *) load_cfg,
+ (void *) box_set_listen,
(void *) box_set_replication_source,
(void *) box_set_wal_mode,
(void *) box_set_log_level,
diff --git a/test/app/cfg.result b/test/app/cfg.result
index 1b10cb8f4d..86808fe7f6 100644
--- a/test/app/cfg.result
+++ b/test/app/cfg.result
@@ -1,9 +1,10 @@
TAP version 13
-1..18
+1..21
ok - box is not started
ok - invalid replication_source
ok - invalid wal_mode
ok - invalid rows_per_wal
+ok - invalid listen
ok - box is not started
ok - exception on unconfigured box
ok - sophia_dir is not auto-created
@@ -18,3 +19,5 @@ ok - work_dir is invalid
ok - sophia_dir is invalid
ok - snap_dir is invalid
ok - wal_dir is invalid
+ok - dynamic listen
+ok - dynamic listen
diff --git a/test/app/cfg.test.lua b/test/app/cfg.test.lua
index 286318fdca..1b6d2c7969 100755
--- a/test/app/cfg.test.lua
+++ b/test/app/cfg.test.lua
@@ -2,7 +2,8 @@
local tap = require('tap')
local test = tap.test('cfg')
-test:plan(18)
+local socket = require('socket')
+test:plan(21)
--------------------------------------------------------------------------------
-- Invalid values
@@ -12,12 +13,13 @@ test:is(type(box.cfg), 'function', 'box is not started')
local function invalid(name, val)
local status, result = pcall(box.cfg, {[name]=val})
- test:ok(not status and result:match('Incorrect option'), 'invalid '..name)
+ test:ok(not status and result:match('Incorrect'), 'invalid '..name)
end
invalid('replication_source', '//guest@localhost:3301')
invalid('wal_mode', 'invalid')
invalid('rows_per_wal', -1)
+invalid('listen', '//!')
test:is(type(box.cfg), 'function', 'box is not started')
@@ -86,5 +88,18 @@ script:write([[ box.cfg{ logger="tarantool.log", wal_dir='invalid' } ]])
script:close()
test:isnt(os.execute("/bin/sh -c 'tarantool ./script.lua 2> /dev/null'"), 0, 'wal_dir is invalid')
+-- box.cfg { listen = xx }
+local path = './tarantool.sock'
+os.remove(path)
+box.cfg{ listen = 'unix/:'..path }
+s = socket.tcp_connect('unix/', path)
+test:isnt(s, nil, "dynamic listen")
+if s then s:close() end
+box.cfg{ listen = '' }
+s = socket.tcp_connect('unix/', path)
+test:isnil(s, 'dynamic listen')
+if s then s:close() end
+os.remove(path)
+
test:check()
os.exit(0)
diff --git a/test/box/cfg.result b/test/box/cfg.result
index fa118a555e..dccf7e612c 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -2,7 +2,7 @@
--# push filter 'admin: .*' to 'admin: <uri>'
box.cfg.nosuchoption = 1
---
-- error: '[string "-- load_cfg.lua - internal file..."]:243: Attempt to modify a read-only
+- error: '[string "-- load_cfg.lua - internal file..."]:247: Attempt to modify a read-only
table'
...
t = {} for k,v in pairs(box.cfg) do if type(v) ~= 'table' and type(v) ~= 'function' then table.insert(t, k..': '..tostring(v)) end end
@@ -35,6 +35,8 @@ t
-- must be read-only
box.cfg()
---
+- error: '[string "-- load_cfg.lua - internal file..."]:193: bad argument #1 to ''pairs''
+ (table expected, got nil)'
...
t = {} for k,v in pairs(box.cfg) do if type(v) ~= 'table' and type(v) ~= 'function' then table.insert(t, k..': '..tostring(v)) end end
---
@@ -66,23 +68,23 @@ t
-- check that cfg with unexpected parameter fails.
box.cfg{sherlock = 'holmes'}
---
-- error: '[string "-- load_cfg.lua - internal file..."]:147: Error: cfg parameter
+- error: '[string "-- load_cfg.lua - internal file..."]:149: Error: cfg parameter
''sherlock'' is unexpected'
...
-- check that cfg with unexpected type of parameter failes
box.cfg{listen = {}}
---
-- error: '[string "-- load_cfg.lua - internal file..."]:167: Error: cfg parameter
+- error: '[string "-- load_cfg.lua - internal file..."]:169: Error: cfg parameter
''listen'' should be one of types: string, number'
...
box.cfg{wal_dir = 0}
---
-- error: '[string "-- load_cfg.lua - internal file..."]:161: Error: cfg parameter
+- error: '[string "-- load_cfg.lua - internal file..."]:163: Error: cfg parameter
''wal_dir'' should be of type string'
...
box.cfg{coredump = 'true'}
---
-- error: '[string "-- load_cfg.lua - internal file..."]:161: Error: cfg parameter
+- error: '[string "-- load_cfg.lua - internal file..."]:163: Error: cfg parameter
''coredump'' should be of type boolean'
...
--# clear filter
diff --git a/test/box/snapshot_daemon.result b/test/box/snapshot_daemon.result
index 8959177f47..47160c9100 100644
--- a/test/box/snapshot_daemon.result
+++ b/test/box/snapshot_daemon.result
@@ -114,7 +114,7 @@ PERIOD
...
box.cfg{ snapshot_count = .2 }
---
-- error: 'Incorrect option value: snapshot_count must be integer'
+- error: 'Incorrect ''snapshot_count'' option value: must be integer'
...
daemon = box.internal.snapshot_daemon
---
--
GitLab