[tarantool-patches] Re: [PATCH v1 1/1] box: support reload whole module
Kirill Shcherbatov
kshcherbatov at tarantool.org
Thu Jul 12 11:27:13 MSK 2018
> During our verbal discussion with Kirill, he noticed that the fact that
> box.schema.func.reload() reloads the whole module when it is passed a
> function name is rather confusing: the user may not know that and call
> box.schema.func.reload() once per each used function, in which case he
> will effectively reload the whole module that contains those functions
> multiple times, which is pointless. Turns out Kirill isn't the only one
> who finds such an API weird, see
>
> https://github.com/tarantool/tarantool/issues/910#issuecomment-331435227
>
> I talked to Kostja and he doesn't mind banning this API, i.e. making
> box.schema.func.reload() interpret its argument only as a module name.
>
> So let's rework this patch so that box_func_reload() directly calls
> module_reload(), and remove func_reload() altogether. As for the access
> checks, box_func_reload() should work only if the caller has the global
> execute privilege.
Closes #2946.
@TarantoolBot document
Title: fixed module reload
There was a bug in tarantool documentation:
https://tarantool.io/en/doc/1.7/book/box/
box_schema/#lua-function.box.schema.func.reload
Now it is allowed to reload all functions in loadable
module via one method. Legacy method including finction
name is forbidden.
box.schema.func.reload("utils") -- ok since now
box.schema.func.reload("utils.func1") -- forbidden since now
Global reload is still unsupported because it seems
to be useless.
box.schema.func.reload() -- invalid!
---
https://github.com/tarantool/tarantool/compare/kshch/gh-2946-module-reload
https://github.com/tarantool/tarantool/issues/2946
src/box/call.c | 23 +++++++++++++----------
src/box/call.h | 9 ++++++++-
src/box/errcode.h | 1 +
src/box/func.c | 16 ----------------
src/box/func.h | 11 ++++++++++-
src/box/lua/call.c | 6 +++---
src/box/lua/schema.lua | 2 +-
test/box/func_reload.result | 23 ++++++++---------------
test/box/func_reload.test.lua | 14 ++++++--------
test/box/misc.result | 1 +
10 files changed, 51 insertions(+), 55 deletions(-)
diff --git a/src/box/call.c b/src/box/call.c
index 438be19..b9750c5 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -132,20 +132,23 @@ box_c_call(struct func *func, struct call_request *request, struct port *port)
}
int
-box_func_reload(const char *name)
+box_module_reload(const char *name)
{
- size_t name_len = strlen(name);
- struct func *func = NULL;
- if ((access_check_func(name, name_len, &func)) != 0)
- return -1;
- if (func == NULL) {
- diag_set(ClientError, ER_NO_SUCH_FUNCTION, name);
+ struct credentials *credentials = effective_user();
+ if ((credentials->universal_access & (PRIV_X | PRIV_U)) !=
+ (PRIV_X | PRIV_U)) {
+ struct user *user = user_find(credentials->uid);
+ if (user != NULL)
+ diag_set(AccessDeniedError, priv_name(PRIV_U),
+ schema_object_name(SC_UNIVERSE), "",
+ user->def->name);
return -1;
}
- if (func->def->language != FUNC_LANGUAGE_C || func->func == NULL)
- return 0; /* Nothing to do */
- if (func_reload(func) == 0)
+ struct module *module = NULL;
+ if (module_reload(name, name + strlen(name), &module) == 0 &&
+ module != NULL)
return 0;
+ diag_set(ClientError, ER_NO_SUCH_MODULE, name);
return -1;
}
diff --git a/src/box/call.h b/src/box/call.h
index eabba69..1b54551 100644
--- a/src/box/call.h
+++ b/src/box/call.h
@@ -42,8 +42,15 @@ struct box_function_ctx {
struct port *port;
};
+/**
+ * Reload loadable module by name.
+ *
+ * @param name of the module to reload.
+ * @retval -1 on error.
+ * @retval 0 on success.
+ */
int
-box_func_reload(const char *name);
+box_module_reload(const char *name);
int
box_process_call(struct call_request *request, struct port *port);
diff --git a/src/box/errcode.h b/src/box/errcode.h
index c76018c..64f9af1 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -215,6 +215,7 @@ struct errcode_record {
/*160 */_(ER_ACTION_MISMATCH, "Field %d contains %s on conflict action, but %s in index parts") \
/*161 */_(ER_VIEW_MISSING_SQL, "Space declared as a view must have SQL statement") \
/*162 */_(ER_FOREIGN_KEY_CONSTRAINT, "Can not commit transaction: deferred foreign keys violations are not resolved") \
+ /*163 */_(ER_NO_SUCH_MODULE, "Module '%s' does not exist") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/func.c b/src/box/func.c
index dfbc5f3..a817851 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -298,9 +298,6 @@ module_sym(struct module *module, const char *name)
return f;
}
-/*
- * Reload a dso.
- */
int
module_reload(const char *package, const char *package_end, struct module **module)
{
@@ -436,19 +433,6 @@ func_load(struct func *func)
}
int
-func_reload(struct func *func)
-{
- struct func_name name;
- func_split_name(func->def->name, &name);
- struct module *module = NULL;
- if (module_reload(name.package, name.package_end, &module) != 0) {
- diag_log();
- return -1;
- }
- return 0;
-}
-
-int
func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
const char *args_end)
{
diff --git a/src/box/func.h b/src/box/func.h
index 0957546..8dcd61d 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -113,8 +113,17 @@ int
func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
const char *args_end);
+/**
+ * Reload dynamically loadable module.
+ *
+ * @param package name begin pointer.
+ * @param package_end package_end name end pointer.
+ * @param[out] module a pointer to store module object on success.
+ * @retval -1 on error.
+ * @retval 0 on success.
+ */
int
-func_reload(struct func *func);
+module_reload(const char *package, const char *package_end, struct module **module);
#if defined(__cplusplus)
} /* extern "C" */
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 8f3461a..d20cbbb 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -458,10 +458,10 @@ box_lua_eval(struct call_request *request, struct port *port)
}
static int
-lbox_func_reload(lua_State *L)
+lbox_module_reload(lua_State *L)
{
const char *name = luaL_checkstring(L, 1);
- if (box_func_reload(name) != 0)
+ if (box_module_reload(name) != 0)
return luaT_error(L);
return 0;
}
@@ -469,7 +469,7 @@ lbox_func_reload(lua_State *L)
static const struct luaL_Reg boxlib_internal[] = {
{"call_loadproc", lbox_call_loadproc},
{"sql_create_function", lbox_sql_create_function},
- {"func_reload", lbox_func_reload},
+ {"module_reload", lbox_module_reload},
{NULL, NULL}
};
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 87c79bd..ab8072c 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1951,7 +1951,7 @@ function box.schema.func.exists(name_or_id)
return tuple ~= nil
end
-box.schema.func.reload = internal.func_reload
+box.schema.func.reload = internal.module_reload
box.internal.collation = {}
box.internal.collation.create = function(name, coll_type, locale, opts)
diff --git a/test/box/func_reload.result b/test/box/func_reload.result
index 5aeb85c..b024cd1 100644
--- a/test/box/func_reload.result
+++ b/test/box/func_reload.result
@@ -51,8 +51,9 @@ fio.symlink(reload1_path, reload_path)
- true
...
--check not fail on non-load func
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
---
+- error: Module 'reload' does not exist
...
-- test of usual case reload. No hanging calls
box.space.test:insert{0}
@@ -74,7 +75,7 @@ fio.symlink(reload2_path, reload_path)
---
- true
...
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
---
...
c:call("reload.foo")
@@ -98,7 +99,7 @@ fio.symlink(reload1_path, reload_path)
---
- true
...
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
---
...
fibers = 10
@@ -111,7 +112,7 @@ while box.space.test:count() < fibers do fiber.sleep(0.001) end
---
...
-- double reload doesn't fail waiting functions
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
---
...
_ = fio.unlink(reload_path)
@@ -121,7 +122,7 @@ fio.symlink(reload2_path, reload_path)
---
- true
...
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
---
...
c:call("reload.foo")
@@ -208,7 +209,7 @@ _ = fiber.create(function() ch:put(c:call("reload.test_reload")) end)
while s:get({1}) == nil do fiber.yield(0.0001) end
---
...
-box.schema.func.reload("reload.test_reload")
+box.schema.func.reload("reload")
---
...
_ = fiber.create(function() ch:put(c:call("reload.test_reload")) end)
@@ -242,14 +243,6 @@ fio.symlink(reload1_path, reload_path)
---
- true
...
-s, e = pcall(box.schema.func.reload, "reload.test_reload")
----
-...
-s, string.find(tostring(e), 'test_reload_fail') ~= nil
----
-- false
-- true
-...
c:call("reload.test_reload")
---
- [[2]]
@@ -273,5 +266,5 @@ box.schema.func.reload()
...
box.schema.func.reload("non-existing")
---
-- error: Function 'non-existing' does not exist
+- error: Module 'non-existing' does not exist
...
diff --git a/test/box/func_reload.test.lua b/test/box/func_reload.test.lua
index dc56d84..8906898 100644
--- a/test/box/func_reload.test.lua
+++ b/test/box/func_reload.test.lua
@@ -20,7 +20,7 @@ _ = fio.unlink(reload_path)
fio.symlink(reload1_path, reload_path)
--check not fail on non-load func
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
-- test of usual case reload. No hanging calls
box.space.test:insert{0}
@@ -29,7 +29,7 @@ box.space.test:delete{0}
_ = fio.unlink(reload_path)
fio.symlink(reload2_path, reload_path)
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
c:call("reload.foo")
box.space.test:select{}
box.space.test:truncate()
@@ -37,18 +37,18 @@ box.space.test:truncate()
-- test case with hanging calls
_ = fio.unlink(reload_path)
fio.symlink(reload1_path, reload_path)
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
fibers = 10
for i = 1, fibers do fiber.create(function() c:call("reload.foo", {i}) end) end
while box.space.test:count() < fibers do fiber.sleep(0.001) end
-- double reload doesn't fail waiting functions
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
_ = fio.unlink(reload_path)
fio.symlink(reload2_path, reload_path)
-box.schema.func.reload("reload.foo")
+box.schema.func.reload("reload")
c:call("reload.foo")
while box.space.test:count() < 2 * fibers + 1 do fiber.sleep(0.001) end
@@ -71,7 +71,7 @@ _ = fio.unlink(reload_path)
fio.symlink(reload2_path, reload_path)
_ = fiber.create(function() ch:put(c:call("reload.test_reload")) end)
while s:get({1}) == nil do fiber.yield(0.0001) end
-box.schema.func.reload("reload.test_reload")
+box.schema.func.reload("reload")
_ = fiber.create(function() ch:put(c:call("reload.test_reload")) end)
ch:get()
ch:get()
@@ -82,8 +82,6 @@ box.schema.user.grant('guest', 'execute', 'function', 'reload.test_reload_fail')
c:call("reload.test_reload_fail")
_ = fio.unlink(reload_path)
fio.symlink(reload1_path, reload_path)
-s, e = pcall(box.schema.func.reload, "reload.test_reload")
-s, string.find(tostring(e), 'test_reload_fail') ~= nil
c:call("reload.test_reload")
c:call("reload.test_reload_fail")
diff --git a/test/box/misc.result b/test/box/misc.result
index a00d033..777b40b 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -487,6 +487,7 @@ t;
160: box.error.ACTION_MISMATCH
161: box.error.VIEW_MISSING_SQL
162: box.error.FOREIGN_KEY_CONSTRAINT
+ 163: box.error.NO_SUCH_MODULE
...
test_run:cmd("setopt delimiter ''");
---
--
2.7.4
More information about the Tarantool-patches
mailing list