From: Kirill Shcherbatov <kshcherbatov@tarantool.org> To: tarantool-patches@freelists.org, vdavydov.dev@gmail.com Cc: Kirill Shcherbatov <kshcherbatov@tarantool.org> Subject: [PATCH v1 2/8] box: rework func cache update machinery Date: Thu, 30 May 2019 13:45:29 +0300 [thread overview] Message-ID: <764336db019f84a4632c1311621e65fdf09ac395.1559212747.git.kshcherbatov@tarantool.org> (raw) In-Reply-To: <cover.1559212747.git.kshcherbatov@tarantool.org> 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
next prev parent reply other threads:[~2019-05-30 10:45 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-30 10:45 [PATCH v1 0/8] box: functional indexes Kirill Shcherbatov 2019-05-30 10:45 ` [PATCH v1 1/8] box: refactor box_lua_find helper Kirill Shcherbatov 2019-05-30 10:45 ` Kirill Shcherbatov [this message] 2019-05-30 10:45 ` [PATCH v1 3/8] schema: rework _func system space format Kirill Shcherbatov 2019-05-30 12:06 ` [tarantool-patches] " Konstantin Osipov 2019-06-03 16:14 ` Vladimir Davydov 2019-05-30 13:10 ` Vladislav Shpilevoy 2019-05-30 10:45 ` [PATCH v1 4/8] box: load persistent Lua functions on creation Kirill Shcherbatov 2019-05-31 8:16 ` [tarantool-patches] " Konstantin Osipov 2019-06-03 8:26 ` [tarantool-patches] " Kirill Shcherbatov 2019-05-30 10:45 ` [PATCH v1 5/8] netbox: call persistent functions in netbox Kirill Shcherbatov 2019-05-30 10:45 ` [PATCH v1 6/8] box: export _func functions with box.func folder Kirill Shcherbatov 2019-06-03 16:22 ` Vladimir Davydov 2019-06-03 16:24 ` Vladimir Davydov 2019-05-30 10:45 ` [PATCH v1 7/8] box: introduce memtx_slab_alloc helper Kirill Shcherbatov 2019-05-30 10:45 ` [PATCH v1 8/8] box: introduce functional indexes in memtx Kirill Shcherbatov 2019-05-30 11:18 ` [tarantool-patches] [PATCH v1 0/8] box: functional indexes Vladislav Shpilevoy 2019-05-30 11:43 ` [tarantool-patches] " Kirill Shcherbatov 2019-05-30 11:47 ` [tarantool-patches] " Konstantin Osipov 2019-06-03 15:55 ` Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=764336db019f84a4632c1311621e65fdf09ac395.1559212747.git.kshcherbatov@tarantool.org \ --to=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [PATCH v1 2/8] box: rework func cache update machinery' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox