[PATCH 03/13] vinyl: move reference counting from vy_lsm to index

Vladimir Davydov vdavydov.dev at gmail.com
Sat Aug 10 13:03:30 MSK 2019


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




More information about the Tarantool-patches mailing list