From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 25 Oct 2018 17:48:14 +0300 From: Vladimir Davydov Subject: Re: [PATCH v2 1/4] schema: refactor space_cache API Message-ID: <20181025144814.zwqflwpqsxxjlbd3@esperanza> References: <1e8ea1c80fd917dc78aea785e1d95c76241cf830.1540388902.git.kyukhin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1e8ea1c80fd917dc78aea785e1d95c76241cf830.1540388902.git.kyukhin@tarantool.org> To: Kirill Yukhin Cc: tarantool-patches@freelists.org List-ID: 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();