[PATCH v3 1/6] box: rework func cache update machinery
Kirill Shcherbatov
kshcherbatov at tarantool.org
Thu Jun 13 17:08:21 MSK 2019
Tarantool used to assume that func_new call must not fail and
it used to build a new func object by given definition just on
func cache replace operation. We need to fix it to perform
user-dependent risky actions like Lua function assemble in
further patches.
The replace method is disallowed for _func space because it is
redundant and difficult to maintain in case of functions that
have pre-compiled runtime.
Needed for #4182, #1260
---
src/box/alter.cc | 57 +++++++++++++++++++------------------
src/box/func.c | 8 ------
src/box/func.h | 3 --
src/box/func_def.c | 17 +++++++++++
src/box/func_def.h | 12 ++++++++
src/box/schema.cc | 37 +++++++++---------------
src/box/schema.h | 10 +++----
test/box/function1.result | 4 +++
test/box/function1.test.lua | 1 +
9 files changed, 81 insertions(+), 68 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index a37a68ce4..18b50a5da 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2574,24 +2574,11 @@ func_def_new_from_tuple(struct tuple *tuple)
/** Remove a function from function cache */
static void
-func_cache_remove_func(struct trigger * /* trigger */, void *event)
+func_cache_remove_func(struct trigger *trigger, void * /* event */)
{
- struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
- uint32_t fid = tuple_field_u32_xc(stmt->old_tuple ?
- stmt->old_tuple : stmt->new_tuple,
- BOX_FUNC_FIELD_ID);
- func_cache_delete(fid);
-}
-
-/** Replace a function in the function cache */
-static void
-func_cache_replace_func(struct trigger * /* trigger */, void *event)
-{
- struct txn_stmt *stmt = txn_last_stmt((struct txn*) event);
- struct func_def *def = func_def_new_from_tuple(stmt->new_tuple);
- auto def_guard = make_scoped_guard([=] { free(def); });
- func_cache_replace(def);
- def_guard.is_active = false;
+ struct func *old_func = (struct func *) trigger->data;
+ func_cache_delete(old_func->def->fid);
+ func_delete(old_func);
}
/**
@@ -2612,13 +2599,21 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
struct func *old_func = func_by_id(fid);
if (new_tuple != NULL && old_func == NULL) { /* INSERT */
struct func_def *def = func_def_new_from_tuple(new_tuple);
+ auto def_guard = make_scoped_guard([=] { free(def); });
access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
PRIV_C);
- auto def_guard = make_scoped_guard([=] { free(def); });
- func_cache_replace(def);
+ struct func *func = func_new(def);
+ if (func == NULL)
+ diag_raise();
+ auto func_guard = make_scoped_guard([=] {
+ func_cache_delete(def->fid);
+ func_delete(func);
+ });
def_guard.is_active = false;
+ func_cache_insert(func);
+ func_guard.is_active = false;
struct trigger *on_rollback =
- txn_alter_trigger_new(func_cache_remove_func, NULL);
+ txn_alter_trigger_new(func_cache_remove_func, func);
txn_on_rollback(txn, on_rollback);
} else if (new_tuple == NULL) { /* DELETE */
uint32_t uid;
@@ -2636,16 +2631,22 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
"function has grants");
}
struct trigger *on_commit =
- txn_alter_trigger_new(func_cache_remove_func, NULL);
+ txn_alter_trigger_new(func_cache_remove_func, old_func);
txn_on_commit(txn, on_commit);
} else { /* UPDATE, REPLACE */
- struct func_def *def = func_def_new_from_tuple(new_tuple);
- auto def_guard = make_scoped_guard([=] { free(def); });
- access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
- PRIV_A);
- struct trigger *on_commit =
- txn_alter_trigger_new(func_cache_replace_func, NULL);
- txn_on_commit(txn, on_commit);
+ assert(new_tuple != NULL && old_tuple != NULL);
+ /**
+ * Allow an alter that doesn't change the
+ * definition to support upgrade script.
+ */
+ struct func_def *new_def = func_def_new_from_tuple(new_tuple);
+ auto new_def_guard = make_scoped_guard([=] { free(new_def); });
+ struct func_def *old_def = func_def_new_from_tuple(old_tuple);
+ auto old_def_guard = make_scoped_guard([=] { free(old_def); });
+ if (func_def_cmp(new_def, old_def) != 0) {
+ tnt_raise(ClientError, ER_UNSUPPORTED, "function",
+ "alter");
+ }
}
}
diff --git a/src/box/func.c b/src/box/func.c
index a817851fd..d1b8879ad 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -451,14 +451,6 @@ func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
return rc;
}
-void
-func_update(struct func *func, struct func_def *def)
-{
- func_unload(func);
- free(func->def);
- func->def = def;
-}
-
void
func_delete(struct func *func)
{
diff --git a/src/box/func.h b/src/box/func.h
index 8dcd61d7b..fa4d738ab 100644
--- a/src/box/func.h
+++ b/src/box/func.h
@@ -100,9 +100,6 @@ module_free(void);
struct func *
func_new(struct func_def *def);
-void
-func_update(struct func *func, struct func_def *def);
-
void
func_delete(struct func *func);
diff --git a/src/box/func_def.c b/src/box/func_def.c
index 76ed77b24..2b135e2d7 100644
--- a/src/box/func_def.c
+++ b/src/box/func_def.c
@@ -1,3 +1,20 @@
#include "func_def.h"
+#include "string.h"
const char *func_language_strs[] = {"LUA", "C"};
+
+int
+func_def_cmp(struct func_def *def1, struct func_def *def2)
+{
+ if (def1->fid != def2->fid)
+ return def1->fid - def2->fid;
+ if (def1->uid != def2->uid)
+ return def1->uid - def2->uid;
+ if (def1->setuid != def2->setuid)
+ return def1->setuid - def2->setuid;
+ if (def1->language != def2->language)
+ return def1->language - def2->language;
+ if (strcmp(def1->name, def2->name) != 0)
+ return strcmp(def1->name, def2->name);
+ return 0;
+}
diff --git a/src/box/func_def.h b/src/box/func_def.h
index 5b52ab498..4c9738aea 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -34,6 +34,10 @@
#include "trivia/util.h"
#include <stdbool.h>
+#ifdef __cplusplus
+extern "C" {
+#endif
+
/**
* The supported language of the stored function.
*/
@@ -79,6 +83,10 @@ func_def_sizeof(uint32_t name_len)
return sizeof(struct func_def) + name_len + 1;
}
+/** Compare two given function definitions. */
+int
+func_def_cmp(struct func_def *def1, struct func_def *def2);
+
/**
* API of C stored function.
*/
@@ -86,4 +94,8 @@ typedef struct box_function_ctx box_function_ctx_t;
typedef int (*box_function_f)(box_function_ctx_t *ctx,
const char *args, const char *args_end);
+#ifdef __cplusplus
+}
+#endif
+
#endif /* TARANTOOL_BOX_FUNC_DEF_H_INCLUDED */
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 6d3153815..f834610a5 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -541,40 +541,32 @@ schema_free(void)
}
void
-func_cache_replace(struct func_def *def)
+func_cache_insert(struct func *func)
{
- struct func *old = func_by_id(def->fid);
- if (old) {
- func_update(old, def);
- return;
- }
if (mh_size(funcs) >= BOX_FUNCTION_MAX)
tnt_raise(ClientError, ER_FUNCTION_MAX, BOX_FUNCTION_MAX);
- struct func *func = func_new(def);
- if (func == NULL) {
-error:
- panic_syserror("Out of memory for the data "
- "dictionary cache (stored function).");
- }
- const struct mh_i32ptr_node_t node = { def->fid, func };
- mh_int_t k1 = mh_i32ptr_put(funcs, &node, NULL, NULL);
- if (k1 == mh_end(funcs)) {
- func->def = NULL;
- func_delete(func);
- goto error;
- }
+
+ mh_int_t k1, k2;
+ struct mh_i32ptr_node_t old_node, *old_node_ptr = &old_node;
+ const struct mh_i32ptr_node_t node = { func->def->fid, func };
size_t def_name_len = strlen(func->def->name);
uint32_t name_hash = mh_strn_hash(func->def->name, def_name_len);
const struct mh_strnptr_node_t strnode = {
func->def->name, def_name_len, name_hash, func };
- mh_int_t k2 = mh_strnptr_put(funcs_by_name, &strnode, NULL, NULL);
+ k1 = mh_i32ptr_put(funcs, &node, &old_node_ptr, NULL);
+ if (k1 == mh_end(funcs))
+ goto error;
+ assert(old_node_ptr == NULL);
+ k2 = mh_strnptr_put(funcs_by_name, &strnode, NULL, NULL);
if (k2 == mh_end(funcs_by_name)) {
mh_i32ptr_del(funcs, k1, NULL);
- func->def = NULL;
- func_delete(func);
goto error;
}
+ return;
+error:
+ panic_syserror("Out of memory for the data dictionary cache "
+ "(stored function).");
}
void
@@ -590,7 +582,6 @@ func_cache_delete(uint32_t fid)
strlen(func->def->name));
if (k != mh_end(funcs))
mh_strnptr_del(funcs_by_name, k, NULL);
- func_delete(func);
}
struct func *
diff --git a/src/box/schema.h b/src/box/schema.h
index 6f9a96117..d7b3c4c2d 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -163,14 +163,12 @@ schema_free();
struct space *schema_space(uint32_t id);
/**
- * Insert a new function or update the old one.
- *
- * @param def Function definition. In a case of success the ownership
- * of @a def is transfered to the data dictionary, thus the caller
- * must not delete it.
+ * Insert a new function object in the function cache.
+ * @param func Function object to insert.
+ * @retval Returns 0 on success, -1 otherwise.
*/
void
-func_cache_replace(struct func_def *def);
+func_cache_insert(struct func *func);
void
func_cache_delete(uint32_t fid);
diff --git a/test/box/function1.result b/test/box/function1.result
index 5b04fa97d..cadeb0467 100644
--- a/test/box/function1.result
+++ b/test/box/function1.result
@@ -16,6 +16,10 @@ c = net.connect(os.getenv("LISTEN"))
box.schema.func.create('function1', {language = "C"})
---
...
+box.space._func:replace{2, 1, 'function1', 0, 'LUA'}
+---
+- error: function does not support alter
+...
box.schema.user.grant('guest', 'execute', 'function', 'function1')
---
...
diff --git a/test/box/function1.test.lua b/test/box/function1.test.lua
index 855dd5382..e983495b6 100644
--- a/test/box/function1.test.lua
+++ b/test/box/function1.test.lua
@@ -7,6 +7,7 @@ net = require('net.box')
c = net.connect(os.getenv("LISTEN"))
box.schema.func.create('function1', {language = "C"})
+box.space._func:replace{2, 1, 'function1', 0, 'LUA'}
box.schema.user.grant('guest', 'execute', 'function', 'function1')
_ = box.schema.space.create('test')
_ = box.space.test:create_index('primary')
--
2.21.0
More information about the Tarantool-patches
mailing list