[PATCH v1 2/8] box: rework func cache update machinery
Kirill Shcherbatov
kshcherbatov at tarantool.org
Thu May 30 13:45:29 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. It is not good approach itself and
moreover we a going to implement a persistent function object
that would assemble itself on creation that might fail.
Needed for #4182, #1260
---
src/box/alter.cc | 43 ++++++++++++++++++++++++++-----------------
src/box/func.c | 13 +++++++------
src/box/func.h | 12 +++++++++---
src/box/schema.cc | 46 ++++++++++++++++++++--------------------------
src/box/schema.h | 15 +++++++++------
5 files changed, 71 insertions(+), 58 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index ed9e55907..3b57a7d82 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2463,24 +2463,22 @@ 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);
+ struct func *old_func = (struct func *) trigger->data;
+ func_cache_delete(old_func->def->fid);
+ func_delete(old_func);
}
/** Replace a function in the function cache */
static void
-func_cache_replace_func(struct trigger * /* trigger */, void *event)
+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 *new_func = (struct func *) trigger->data;
+ struct func *old_func;
+ func_cache_replace(new_func, &old_func);
+ assert(old_func != NULL);
+ func_delete(old_func);
}
/**
@@ -2501,13 +2499,20 @@ 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_delete(func); });
def_guard.is_active = false;
+ struct func *old_func = NULL;
+ func_cache_replace(func, &old_func);
+ assert(old_func == NULL);
+ 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;
@@ -2525,15 +2530,19 @@ 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 func *func = func_new(def);
+ if (func == NULL)
+ diag_raise();
+ def_guard.is_active = false;
struct trigger *on_commit =
- txn_alter_trigger_new(func_cache_replace_func, NULL);
+ txn_alter_trigger_new(func_cache_replace_func, func);
txn_on_commit(txn, on_commit);
}
}
diff --git a/src/box/func.c b/src/box/func.c
index a817851fd..f7465be7e 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -452,17 +452,18 @@ func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
}
void
-func_update(struct func *func, struct func_def *def)
+func_delete(struct func *func)
{
func_unload(func);
free(func->def);
- func->def = def;
+ free(func);
}
void
-func_delete(struct func *func)
+func_capture_module(struct func *new_func, struct func *old_func)
{
- func_unload(func);
- free(func->def);
- free(func);
+ new_func->module = old_func->module;
+ new_func->func = old_func->func;
+ old_func->module = NULL;
+ old_func->func = NULL;
}
diff --git a/src/box/func.h b/src/box/func.h
index 8dcd61d7b..a4a758b58 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);
@@ -113,6 +110,15 @@ int
func_call(struct func *func, box_function_ctx_t *ctx, const char *args,
const char *args_end);
+/**
+ * Take over the modules that belonged to the old function.
+ * Reset module and function pointers in old function.
+ * This helper allows you to inherit already loaded function
+ * objects, that is useful on update the function cache.
+*/
+void
+func_capture_module(struct func *new_func, struct func *old_func);
+
/**
* Reload dynamically loadable module.
*
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 9a55c2f14..b70992297 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -533,40 +533,35 @@ schema_free(void)
}
void
-func_cache_replace(struct func_def *def)
+func_cache_replace(struct func *new_func, struct func **old_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;
- }
- size_t def_name_len = strlen(func->def->name);
- uint32_t name_hash = mh_strn_hash(func->def->name, def_name_len);
+
+ mh_int_t k1, k2;
+ struct mh_i32ptr_node_t old_node, *old_node_ptr = &old_node;
+ const struct mh_i32ptr_node_t node = { new_func->def->fid, new_func };
+ size_t def_name_len = strlen(new_func->def->name);
+ uint32_t name_hash = mh_strn_hash(new_func->def->name, def_name_len);
const struct mh_strnptr_node_t strnode = {
- func->def->name, def_name_len, name_hash, func };
+ new_func->def->name, def_name_len, name_hash, new_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;
+ if (old_node_ptr != NULL)
+ *old_func = (struct func *)old_node_ptr->val;
+ 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;
}
+ if (*old_func != NULL)
+ func_capture_module(new_func, *old_func);
+ return;
+error:
+ panic_syserror("Out of memory for the data dictionary cache "
+ "(stored function).");
}
void
@@ -582,7 +577,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..ffc41f401 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -163,14 +163,17 @@ 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.
+ * Replace an existent (if any) function or insert a new one
+ * in function cache.
+ * @param func_new Function object to insert. In case of success,
+ * when old function object is exists, all loaded
+ * modules data are inherent with
+ * func_capture_module() when possible.
+ * @param func_old[out] The replaced function object if any.
+ * @retval Returns 0 on success, -1 otherwise.
*/
void
-func_cache_replace(struct func_def *def);
+func_cache_replace(struct func *new_func, struct func **old_func);
void
func_cache_delete(uint32_t fid);
--
2.21.0
More information about the Tarantool-patches
mailing list