From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Kirill Yukhin <kyukhin@tarantool.org> Cc: tarantool-patches@freelists.org Subject: Re: [PATCH v2 1/4] schema: refactor space_cache API Date: Thu, 25 Oct 2018 17:48:14 +0300 [thread overview] Message-ID: <20181025144814.zwqflwpqsxxjlbd3@esperanza> (raw) In-Reply-To: <1e8ea1c80fd917dc78aea785e1d95c76241cf830.1540388902.git.kyukhin@tarantool.org> On Thu, Oct 25, 2018 at 11:17:09AM +0300, Kirill Yukhin wrote: > Remove function which deletes from cache, making replace > more general: it might be used for both insertions, > deletions and replaces. Also, put assert on equality > of space pointer found in cache to old one into > replace routine. I guess this refactoring should be pushed to 1.10-features, right? > --- > src/box/alter.cc | 26 ++++++++++---------------- > src/box/schema.cc | 52 +++++++++++++++++++++++++--------------------------- > src/box/schema.h | 9 ++------- > 3 files changed, 37 insertions(+), 50 deletions(-) > > diff --git a/src/box/alter.cc b/src/box/alter.cc > index de3943c..79ff589 100644 > --- a/src/box/alter.cc > +++ b/src/box/alter.cc > @@ -1610,8 +1604,7 @@ on_drop_view_rollback(struct trigger *trigger, void *event) > * > * - space cache should be updated, and changes in the space > * cache should be reflected in Lua bindings > - * (this is done in space_cache_replace() and > - * space_cache_delete()) > + * (this is done in space_cache_replace()) This comment is obsolete. Lua bindings are updated by an on_alter_space trigger callback. Please fix. > * > * - the space which is changed should be rebuilt according > * to the nature of the modification, i.e. indexes added/dropped, > @@ -1695,7 +1688,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) > * cache right away to achieve linearisable > * execution on a replica. > */ > - (void) space_cache_replace(space); > + (void) space_cache_replace(NULL, space); (void) is not needed here anymore. > /* > * Do not forget to update schema_version right after > * inserting the space to the space_cache, since no > @@ -447,8 +446,7 @@ schema_free(void) > > struct space *space = (struct space *) > mh_i32ptr_node(spaces, i)->val; > - space_cache_delete(space_id(space)); > - space_delete(space); > + space_cache_replace(space, NULL); space_cache_replace(), just like its predecessor space_cache_delete(), doesn't delete the space so space_delete() must remain. > } > mh_i32ptr_delete(spaces); > while (mh_size(funcs) > 0) { > diff --git a/src/box/schema.h b/src/box/schema.h > index 264c16b..05f32de 100644 > --- a/src/box/schema.h > +++ b/src/box/schema.h > @@ -134,14 +134,9 @@ space_cache_find_xc(uint32_t id) > /** > * Update contents of the space cache. Typically the new space is > * an altered version of the original space. > - * Returns the old space, if any. What's the old and new spaces are for? Please improve the comment. > */ > -struct space * > -space_cache_replace(struct space *space); > - > -/** Delete a space from the space cache. */ > -struct space * > -space_cache_delete(uint32_t id); > +void > +space_cache_replace(struct space *old_space, struct space *new_space); > > void > schema_init();
next prev parent reply other threads:[~2018-10-25 14:48 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-25 8:17 [PATCH v2 0/4] Check read access while executing SQL query Kirill Yukhin 2018-10-25 8:17 ` [PATCH v2 1/4] schema: refactor space_cache API Kirill Yukhin 2018-10-25 14:48 ` Vladimir Davydov [this message] 2018-10-25 15:31 ` Kirill Yukhin 2018-10-25 16:09 ` Vladimir Davydov 2018-10-25 8:17 ` [PATCH v2 2/4] schema: add space names cache Kirill Yukhin 2018-10-26 20:55 ` Vladimir Davydov 2018-11-01 11:34 ` [tarantool-patches] " Konstantin Osipov 2018-10-25 8:17 ` [PATCH v2 3/4] sql: use space_by_name in SQL Kirill Yukhin 2018-10-26 21:00 ` Vladimir Davydov 2018-10-25 8:17 ` [PATCH v2 4/4] sql: check read access while executing SQL query Kirill Yukhin 2018-10-26 21:04 ` 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=20181025144814.zwqflwpqsxxjlbd3@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=kyukhin@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH v2 1/4] schema: refactor space_cache API' \ /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