[PATCH v2 1/4] schema: refactor space_cache API
Vladimir Davydov
vdavydov.dev at gmail.com
Thu Oct 25 17:48:14 MSK 2018
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();
More information about the Tarantool-patches
mailing list