[tarantool-patches] Re: [tarantool-patches] [box.ctl 1/3] box.ctl: Introduce stab box.ctl.on_ctl_event

["Konstantin Belyavskiy" <k.belyavskiy@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 14645 bytes --]

Hi,
overall patch looks good with minor fixes (short summary, see explanation below):
- fix commit message
- change function name
- remove unused header

>Четверг, 14 июня 2018, 18:04 +03:00 от Ilya Markov <imarkov@tarantool.org>:
>
>Add required for trigger on_ctl_event
>structures and lua bindings.
>
>Prerequisite #3259 
#3159
>
>---
> src/box/CMakeLists.txt   |  1 +
> src/box/box.cc           |  9 ++++++
> src/box/box.h            |  1 +
> src/box/ctl.c            | 63 +++++++++++++++++++++++++++++++++++++
> src/box/ctl.h            | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/box/lua/cfg.cc       | 12 +++++++
> src/box/lua/ctl.c        | 51 ++++++++++++++++++++++++++++++
> src/box/lua/ctl.h        |  2 ++
> src/box/lua/load_cfg.lua | 10 +++++-
> src/cfg.c                | 37 ++++++++++++++++++++++
> src/cfg.h                |  4 +++
> 11 files changed, 270 insertions(+), 1 deletion(-)
> create mode 100644 src/box/ctl.c
> create mode 100644 src/box/ctl.h
>
>diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
>index 6b1ae3e..74f6b54 100644
>--- a/src/box/CMakeLists.txt
>+++ b/src/box/CMakeLists.txt
>@@ -112,6 +112,7 @@ add_library(box STATIC
>     journal.c
>     wal.c
>     call.c
>+    ctl.c
>     ${lua_sources}
>     lua/init.c
>     lua/call.c
>diff --git a/src/box/box.cc b/src/box/box.cc
>index 61bfa11..26277e7 100644
>--- a/src/box/box.cc
>+++ b/src/box/box.cc
>@@ -72,6 +72,7 @@
> #include "call.h"
> #include "func.h"
> #include "sequence.h"
>+#include "ctl.h"
>
> static char status[64] = "unknown";
>
>@@ -823,6 +824,13 @@ box_set_net_msg_max(void)
> 				IPROTO_FIBER_POOL_SIZE_FACTOR);
> }
>
>+void
>+box_set_on_ctl_event(void)
>+{
>+	if (cfg_reset_on_ctl_event() < 0)
>+		diag_raise();
>+} 
Please add _xc suffix, since function raise exception.
>
>+
> /* }}} configuration bindings */
>
> /**
>@@ -1808,6 +1816,7 @@ box_cfg_xc(void)
> 	box_set_replication_connect_timeout();
> 	box_set_replication_connect_quorum();
> 	box_set_replication_skip_conflict();
>+	box_set_on_ctl_event();
> 	replication_sync_lag = box_check_replication_sync_lag();
> 	xstream_create(&join_stream, apply_initial_join_row);
> 	xstream_create(&subscribe_stream, apply_row);
>diff --git a/src/box/box.h b/src/box/box.h
>index d396789..0325527 100644
>--- a/src/box/box.h
>+++ b/src/box/box.h
>@@ -191,6 +191,7 @@ void box_set_replication_connect_timeout(void);
> void box_set_replication_connect_quorum(void);
> void box_set_replication_skip_conflict(void);
> void box_set_net_msg_max(void);
>+void box_set_on_ctl_event(void);
>
> extern "C" {
> #endif /* defined(__cplusplus) */
>diff --git a/src/box/ctl.c b/src/box/ctl.c
>new file mode 100644
>index 0000000..fc3ae37
>--- /dev/null
>+++ b/src/box/ctl.c
>@@ -0,0 +1,63 @@
>+/*
>+ * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
>+ *
>+ * Redistribution and use in source and binary forms, with or
>+ * without modification, are permitted provided that the following
>+ * conditions are met:
>+ *
>+ * 1. Redistributions of source code must retain the above
>+ *    copyright notice, this list of conditions and the
>+ *    following disclaimer.
>+ *
>+ * 2. Redistributions in binary form must reproduce the above
>+ *    copyright notice, this list of conditions and the following
>+ *    disclaimer in the documentation and/or other materials
>+ *    provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
>+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
>+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
>+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
>+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
>+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
>+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
>+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>+ * SUCH DAMAGE.
>+ */
>+#include <lib/small/small/rlist.h>
>+#include <trigger.h>
>+#include <box/ctl.h>
>+#include "errcode.h"
>+#include "error.h"
>+#include <exception.h>
>+
>+RLIST_HEAD(on_ctl_event);
>+
>+int
>+run_on_ctl_event_triggers(const struct on_ctl_event_ctx *result) {
>+	return trigger_run(&on_ctl_event, (void *) result);
>+}
>+
>+int
>+run_on_ctl_event_trigger_type(enum ctl_event_type type)
>+{
>+	struct on_ctl_event_ctx ctx = {};
>+	ctx.type = type;
>+	return run_on_ctl_event_triggers(&ctx);
>+}
>+
>+int
>+cfg_reset_on_ctl_event()
>+{
>+	if (cfg_reset_trigger("on_ctl_event", &on_ctl_event,
>+		lbox_push_on_ctl_event, NULL) < 0) {
>+		diag_set(ClientError, ER_CFG, "on_ctl_event",
>+			 "expected function or table");
>+		return -1;
>+	}
>+	return 0;
>+}
>\ No newline at end of file
>diff --git a/src/box/ctl.h b/src/box/ctl.h
>new file mode 100644
>index 0000000..1dbb6be
>--- /dev/null
>+++ b/src/box/ctl.h
>@@ -0,0 +1,81 @@
>+#ifndef INCLUDES_TARANTOOL_CTL_H
>+#define INCLUDES_TARANTOOL_CTL_H
>+
>+/*
>+ * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file.
>+ *
>+ * Redistribution and use in source and binary forms, with or
>+ * without modification, are permitted provided that the following
>+ * conditions are met:
>+ *
>+ * 1. Redistributions of source code must retain the above
>+ *    copyright notice, this list of conditions and the
>+ *    following disclaimer.
>+ *
>+ * 2. Redistributions in binary form must reproduce the above
>+ *    copyright notice, this list of conditions and the following
>+ *    disclaimer in the documentation and/or other materials
>+ *    provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
>+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
>+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
>+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
>+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
>+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
>+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
>+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>+ * SUCH DAMAGE.
>+ */
>+
>+#include <cfg.h>
>+#include <box/lua/ctl.h>
>
>+#include "errcode.h"
>+#include "error.h"
>+#include <exception.h>
I think last three headers are useless here.
>
>+
>+/** Global on-ctl_event triggers. */
>+extern struct rlist on_ctl_event;
>+
>+enum ctl_event_type {
>+	CTL_EVENT_SYSTEM_SPACE_RECOVERY,
>+	CTL_EVENT_LOCAL_RECOVERY,
>+	CTL_EVENT_READ_ONLY,
>+	CTL_EVENT_READ_WRITE,
>+	CTL_EVENT_SHUTDOWN,
>+	CTL_EVENT_REPLICASET_ADD,
>+	CTL_EVENT_REPLICASET_REMOVE,
>+};
>+
>+struct on_ctl_event_ctx {
>+	enum ctl_event_type type;
>+	uint32_t replica_id;
>+};
>+
>+#if defined(__cplusplus)
>+extern "C" {
>+#endif /* defined(__cplusplus) */
>+
>+/**
>+ * Runs on_ctl_event triggers with specified context.
>+ */
>+int
>+run_on_ctl_event_triggers(const struct on_ctl_event_ctx *result);
>+
>+/**
>+ * Runs on_ctl_event triggers with specified type.
>+ */
>+int
>+run_on_ctl_event_trigger_type(enum ctl_event_type type);
>+
>+int
>+cfg_reset_on_ctl_event();
>+#if defined(__cplusplus)
>+} /* extern "C" */
>+#endif /* defined(__cplusplus) */
>+
>+#endif /* INCLUDES_TARANTOOL_LUA_CTL_H */
>diff --git a/src/box/lua/cfg.cc b/src/box/lua/cfg.cc
>index 5afebc9..c92478f 100644
>--- a/src/box/lua/cfg.cc
>+++ b/src/box/lua/cfg.cc
>@@ -254,6 +254,17 @@ lbox_cfg_set_net_msg_max(struct lua_State *L)
> }
>
> static int
>+lbox_cfg_set_on_ctl_event(struct lua_State *L)
>+{
>+	try {
>+		box_set_on_ctl_event();
>+	} catch (Exception *) {
>+		luaT_error(L);
>+	}
>+	return 0;
>+}
>+
>+static int
> lbox_cfg_set_worker_pool_threads(struct lua_State *L)
> {
> 	(void) L;
>@@ -331,6 +342,7 @@ box_lua_cfg_init(struct lua_State *L)
> 		{"cfg_set_replication_skip_conflict", lbox_cfg_set_replication_skip_conflict},
> 		{"cfg_set_replication_connect_timeout", lbox_cfg_set_replication_connect_timeout},
> 		{"cfg_set_net_msg_max", lbox_cfg_set_net_msg_max},
>+		{"cfg_set_on_ctl_event", lbox_cfg_set_on_ctl_event},
> 		{NULL, NULL}
> 	};
>
>diff --git a/src/box/lua/ctl.c b/src/box/lua/ctl.c
>index 9a105ed..52f320a 100644
>--- a/src/box/lua/ctl.c
>+++ b/src/box/lua/ctl.c
>@@ -35,6 +35,8 @@
> #include <lua.h>
> #include <lauxlib.h>
> #include <lualib.h>
>+#include <lua/trigger.h>
>+#include <box/ctl.h>
>
> #include "lua/utils.h"
>
>@@ -64,9 +66,37 @@ lbox_ctl_wait_rw(struct lua_State *L)
> 	return 0;
> }
>
>+
>+int
>+lbox_push_on_ctl_event(struct lua_State *L, void *event)
>+{
>+	struct on_ctl_event_ctx *ctx = (struct on_ctl_event_ctx *) event;
>+	lua_newtable(L);
>+	lua_pushstring(L, "type");
>+	lua_pushinteger(L, ctx->type);
>+	lua_settable(L, -3);
>+
>+	if (ctx->type == CTL_EVENT_REPLICASET_ADD ||
>+		ctx->type == CTL_EVENT_REPLICASET_REMOVE) {
>+		lua_pushstring(L, "replica_id");
>+		luaL_pushuint64(L, ctx->replica_id);
>+		lua_settable(L, -3);
>+	}
>+	return 1;
>+}
>+
>+static int
>+lbox_on_ctl_event(struct lua_State *L)
>+{
>+	return lbox_trigger_reset(L, 2, &on_ctl_event,
>+				  lbox_push_on_ctl_event, NULL);
>+}
>+
>+
> static const struct luaL_Reg lbox_ctl_lib[] = {
> 	{"wait_ro", lbox_ctl_wait_ro},
> 	{"wait_rw", lbox_ctl_wait_rw},
>+	{"on_ctl_event", lbox_on_ctl_event},
> 	{NULL, NULL}
> };
>
>@@ -75,4 +105,25 @@ box_lua_ctl_init(struct lua_State *L)
> {
> 	luaL_register_module(L, "box.ctl", lbox_ctl_lib);
> 	lua_pop(L, 1);
>+
>+	luaL_findtable(L, LUA_GLOBALSINDEX, "box.ctl", 1);
>+	lua_newtable(L);
>+	lua_setfield(L, -2, "event");
>+	lua_getfield(L, -1, "event");
>+
>+	lua_pushnumber(L, CTL_EVENT_SYSTEM_SPACE_RECOVERY);
>+	lua_setfield(L, -2, "SYSTEM_SPACE_RECOVERY");
>+	lua_pushnumber(L, CTL_EVENT_LOCAL_RECOVERY);
>+	lua_setfield(L, -2, "LOCAL_RECOVERY");
>+	lua_pushnumber(L, CTL_EVENT_READ_ONLY);
>+	lua_setfield(L, -2, "READ_ONLY");
>+	lua_pushnumber(L, CTL_EVENT_READ_WRITE);
>+	lua_setfield(L, -2, "READ_WRITE");
>+	lua_pushnumber(L, CTL_EVENT_SHUTDOWN);
>+	lua_setfield(L, -2, "SHUTDOWN");
>+	lua_pushnumber(L, CTL_EVENT_REPLICASET_ADD);
>+	lua_setfield(L, -2, "CTL_EVENT_REPLICASET_ADD");
>+	lua_pushnumber(L, CTL_EVENT_REPLICASET_REMOVE);
>+	lua_setfield(L, -2, "CTL_EVENT_REPLICASET_REMOVE");
>+	lua_pop(L, 2); /* box, ctl */
> }
>diff --git a/src/box/lua/ctl.h b/src/box/lua/ctl.h
>index e7c2edd..ab63232 100644
>--- a/src/box/lua/ctl.h
>+++ b/src/box/lua/ctl.h
>@@ -41,6 +41,8 @@ struct lua_State;
> void
> box_lua_ctl_init(struct lua_State *L);
>
>+int
>+lbox_push_on_ctl_event(struct lua_State *L, void *event);
> #if defined(__cplusplus)
> } /* extern "C" */
> #endif /* defined(__cplusplus) */
>diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>index 0b668cd..c4d1347 100644
>--- a/src/box/lua/load_cfg.lua
>+++ b/src/box/lua/load_cfg.lua
>@@ -1,7 +1,11 @@
> -- load_cfg.lua - internal file
>
> local log = require('log')
>-local json = require('json')
>+local json = require("json").new()
>+json.cfg{
>+    encode_use_tostring    = true,
>+}
>+
> local private = require('box.internal')
> local urilib = require('uri')
> local math = require('math')
>@@ -64,6 +68,7 @@ local default_cfg = {
>     feedback_host         = " https://feedback.tarantool.io ",
>     feedback_interval     = 3600,
>     net_msg_max           = 768,
>+    on_ctl_event          = nil,
> }
>
> -- types of available options
>@@ -125,6 +130,7 @@ local template_cfg = {
>     feedback_host         = 'string',
>     feedback_interval     = 'number',
>     net_msg_max           = 'number',
>+    on_ctl_event          = 'function, table',
> }
>
> local function normalize_uri(port)
>@@ -216,6 +222,7 @@ local dynamic_cfg = {
>     replicaset_uuid         = check_replicaset_uuid,
>     replication_skip_conflict = private.cfg_set_replication_skip_conflict,
>     net_msg_max             = private.cfg_set_net_msg_max,
>+    on_ctl_event            = private.cfg_set_on_ctl_event,
> }
>
> local dynamic_cfg_skip_at_load = {
>@@ -232,6 +239,7 @@ local dynamic_cfg_skip_at_load = {
>     force_recovery          = true,
>     instance_uuid           = true,
>     replicaset_uuid         = true,
>+    on_ctl_event            = true,
> }
>
> local function convert_gb(size)
>diff --git a/src/cfg.c b/src/cfg.c
>index 7c7d6e7..b07c1f3 100644
>--- a/src/cfg.c
>+++ b/src/cfg.c
>@@ -153,3 +153,40 @@ cfg_getarr_elem(const char *name, int i)
> 	lua_pop(tarantool_L, 2);
> 	return val;
> }
>+
>+int
>+cfg_reset_trigger(const char *name, struct rlist *list,
>+		  lbox_push_event_f push_event, lbox_pop_event_f pop_event)
>+{
>+	cfg_get(name);
>+	struct lua_State *L = tarantool_L;
>+	if (lua_isnil(L, -1))
>+		return 0;
>+	if (!lua_isfunction(L, -1) && !lua_istable(L, -1)) {
>+		return -1;
>+	}
>+	if (lua_istable(L, -1)) {
>+		lua_pushinteger(L, 1);
>+		lua_gettable(L, -2);
>+		if (!luaL_isnull(L, lua_gettop(L)) && !lua_isfunction(L, -1))
>+			return -1;
>+		bool is_nil = luaL_isnull(L, lua_gettop(L));
>+		if (is_nil) {
>+			lua_pop(L, 1);
>+			lua_pushnil(L);
>+		}
>+		lua_pushinteger(L, 2);
>+		lua_gettable(L, -3);
>+		if (!lua_isfunction(L, -1) && !lua_isnil(L, -1))
>+			return -1;
>+		if (is_nil && lua_isnil(L, -1)) {
>+			return 0;
>+		}
>+	} else {
>+		lua_pushnil(L);
>+	}
>+	int rc = lbox_trigger_reset(L, lua_gettop(L), list,
>+				  push_event, pop_event);
>+	lua_pop(L, 1);
>+	return rc;
>+}
>\ No newline at end of file
>diff --git a/src/cfg.h b/src/cfg.h
>index  8499388 ..d36465c 100644
>--- a/src/cfg.h
>+++ b/src/cfg.h
>@@ -32,6 +32,7 @@
>  */
>
> #include <stdint.h>
>+#include <lua/trigger.h>
>
> #if defined(__cplusplus)
> extern "C" {
>@@ -61,6 +62,9 @@ cfg_getarr_size(const char *name);
> const char *
> cfg_getarr_elem(const char *name, int i);
>
>+int
>+cfg_reset_trigger(const char *name, struct rlist *list,
>+		  lbox_push_event_f push_event, lbox_pop_event_f pop_event);
> #if defined(__cplusplus)
> } /* extern "C" */
> #endif /* defined(__cplusplus) */
>-- 
>2.7.4
>
>


Best regards,
Konstantin Belyavskiy
k.belyavskiy@tarantool.org

[-- Attachment #2: Type: text/html, Size: 18206 bytes --]