From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 03/13] vinyl: move reference counting from vy_lsm to index Date: Sat, 10 Aug 2019 13:03:30 +0300 Message-Id: <148a87ea2ed2c7778a528ee191bdee07c78875a4.1565430177.git.vdavydov.dev@gmail.com> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org List-ID: Now, as vy_lsm and index are basically the same object, we can implement reference counting right in struct index. This will allow us to prevent an index from destruction when a space object it belongs to is freed anywhere in the code, not just in vinyl. --- src/box/index.cc | 2 ++ src/box/index.h | 26 ++++++++++++++++++++++++++ src/box/space.c | 4 ++-- src/box/vinyl.c | 7 +------ src/box/vy_lsm.c | 2 -- src/box/vy_lsm.h | 12 ++---------- test/unit/vy_point_lookup.c | 3 ++- 7 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/box/index.cc b/src/box/index.cc index 00a1b502..4e486711 100644 --- a/src/box/index.cc +++ b/src/box/index.cc @@ -488,6 +488,7 @@ index_create(struct index *index, struct engine *engine, index->vtab = vtab; index->engine = engine; index->def = def; + index->refs = 1; index->space_cache_version = space_cache_version; return 0; } @@ -495,6 +496,7 @@ index_create(struct index *index, struct engine *engine, void index_delete(struct index *index) { + assert(index->refs == 0); /* * Free index_def after destroying the index as * engine might still need it, e.g. to check if diff --git a/src/box/index.h b/src/box/index.h index 2b1d0104..89b5e134 100644 --- a/src/box/index.h +++ b/src/box/index.h @@ -454,6 +454,8 @@ struct index { struct engine *engine; /* Description of a possibly multipart key. */ struct index_def *def; + /** Reference counter. */ + int refs; /* Space cache version at the time of construction. */ uint32_t space_cache_version; }; @@ -502,6 +504,30 @@ index_create(struct index *index, struct engine *engine, void index_delete(struct index *index); +/** + * Increment the reference counter of an index to prevent + * it from being destroyed when the space it belongs to is + * freed. + */ +static inline void +index_ref(struct index *index) +{ + assert(index->refs > 0); + index->refs++; +} + +/** + * Decrement the reference counter of an index. + * Destroy the index if it isn't used anymore. + */ +static inline void +index_unref(struct index *index) +{ + assert(index->refs > 0); + if (--index->refs == 0) + index_delete(index); +} + /** Build this index based on the contents of another index. */ int index_build(struct index *index, struct index *pk); diff --git a/src/box/space.c b/src/box/space.c index 1a646899..0d1ad3b3 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -208,7 +208,7 @@ fail_free_indexes: for (uint32_t i = 0; i <= index_id_max; i++) { struct index *index = space->index_map[i]; if (index != NULL) - index_delete(index); + index_unref(index); } fail: free(space->index_map); @@ -248,7 +248,7 @@ space_delete(struct space *space) for (uint32_t j = 0; j <= space->index_id_max; j++) { struct index *index = space->index_map[j]; if (index != NULL) - index_delete(index); + index_unref(index); } free(space->index_map); free(space->check_unique_constraint_map); diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 645e36ca..2d07e336 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -707,12 +707,7 @@ static void vinyl_index_destroy(struct index *index) { struct vy_lsm *lsm = vy_lsm(index); - /* - * There still may be a task scheduled for this LSM tree - * so postpone actual deletion until the last reference - * is gone. - */ - vy_lsm_unref(lsm); + vy_lsm_delete(lsm); } /** diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c index c67db87a..8fba1792 100644 --- a/src/box/vy_lsm.c +++ b/src/box/vy_lsm.c @@ -188,7 +188,6 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env, goto fail_mem; lsm->id = -1; - lsm->refs = 1; lsm->dump_lsn = -1; lsm->commit_lsn = -1; vy_cache_create(&lsm->cache, cache_env, cmp_def, index_def->iid == 0); @@ -245,7 +244,6 @@ vy_range_tree_free_cb(vy_range_tree_t *t, struct vy_range *range, void *arg) void vy_lsm_delete(struct vy_lsm *lsm) { - assert(lsm->refs == 0); assert(heap_node_is_stray(&lsm->in_dump)); assert(heap_node_is_stray(&lsm->in_compaction)); assert(vy_lsm_read_set_empty(&lsm->read_set)); diff --git a/src/box/vy_lsm.h b/src/box/vy_lsm.h index 327a886b..c8b0e297 100644 --- a/src/box/vy_lsm.h +++ b/src/box/vy_lsm.h @@ -182,11 +182,6 @@ struct vy_lsm { struct index base; /** Common LSM tree environment. */ struct vy_lsm_env *env; - /** - * Reference counter. Used to postpone LSM tree deletion - * until all pending operations have completed. - */ - int refs; /** Unique ID of this LSM tree. */ int64_t id; /** ID of the index this LSM tree is for. */ @@ -370,8 +365,7 @@ vy_lsm_dumps_per_compaction(struct vy_lsm *lsm) static inline void vy_lsm_ref(struct vy_lsm *lsm) { - assert(lsm->refs >= 0); - lsm->refs++; + index_ref(&lsm->base); } /** @@ -382,9 +376,7 @@ vy_lsm_ref(struct vy_lsm *lsm) static inline void vy_lsm_unref(struct vy_lsm *lsm) { - assert(lsm->refs > 0); - if (--lsm->refs == 0) - vy_lsm_delete(lsm); + index_unref(&lsm->base); } /** diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c index 9961a0f7..fb075c57 100644 --- a/test/unit/vy_point_lookup.c +++ b/test/unit/vy_point_lookup.c @@ -15,6 +15,7 @@ uint32_t schema_version; uint32_t space_cache_version; struct space *space_by_id(uint32_t id) { return NULL; } struct vy_lsm *vy_lsm(struct index *index) { return NULL; } +void index_delete(struct index *index) { unreachable(); } static int write_run(struct vy_run *run, const char *dir_name, @@ -303,7 +304,7 @@ test_basic() is(results_ok, true, "select results"); is(has_errors, false, "no errors happened"); - vy_lsm_unref(pk); + vy_lsm_delete(pk); index_def_delete(index_def); tuple_format_unref(format); vy_cache_destroy(&cache); -- 2.20.1