From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Kirill Shcherbatov Subject: [PATCH v2 3/9] box: rework func cache update machinery Date: Thu, 6 Jun 2019 15:03:59 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org, vdavydov.dev@gmail.com Cc: Kirill Shcherbatov List-ID: 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. Fixed in scope of global refactoring of func module. 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 098bc02b6..5051286a3 100644 --- a/src/box/func.c +++ b/src/box/func.c @@ -475,17 +475,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_reuse_runtime(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 db839a17a..82ac046b7 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); +/** + * Capture a given old_func's runtime an reuse in a given new + * function if possible. In case of success, the old_func + * stops holding it's runtime so delete method would not + * release it. + */ +void +func_reuse_runtime(struct func *new_func, struct func *old_func); + /** * Reload loadable module by a given name. * Returns 0 in case of success and -1 otherwise and sets the diff --git a/src/box/schema.cc b/src/box/schema.cc index 9a55c2f14..0508482a6 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_reuse_runtime(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..368463536 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_reuse_runtime() 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