[PATCH 1/7] vinyl: fix accounting of secondary index cache statements
Vladimir Davydov
vdavydov.dev at gmail.com
Sun Sep 2 23:18:54 MSK 2018
Since commit 0c5e6cc8c80b ("vinyl: store full tuples in secondary index
cache"), we store primary index tuples in secondary index cache, but we
still account them as separate tuples. Fix that.
Follow-up #3478
Closes #3655
---
src/box/vy_cache.c | 13 ++++++++++--
src/box/vy_cache.h | 5 ++++-
src/box/vy_lsm.c | 2 +-
test/unit/vy_iterators_helper.c | 2 +-
test/unit/vy_point_lookup.c | 2 +-
test/vinyl/cache.result | 47 +++++++++++++++++++++++++++++++++++++++++
test/vinyl/cache.test.lua | 18 ++++++++++++++++
7 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/src/box/vy_cache.c b/src/box/vy_cache.c
index 4f6d0c7f..ead56e95 100644
--- a/src/box/vy_cache.c
+++ b/src/box/vy_cache.c
@@ -71,7 +71,15 @@ vy_cache_env_destroy(struct vy_cache_env *e)
static inline size_t
vy_cache_entry_size(const struct vy_cache_entry *entry)
{
- return sizeof(*entry) + tuple_size(entry->stmt);
+ size_t size = sizeof(*entry);
+ /*
+ * Tuples are shared between primary and secondary index
+ * cache so to avoid double accounting, we account only
+ * primary index tuples.
+ */
+ if (entry->cache->is_primary)
+ size += tuple_size(entry->stmt);
+ return size;
}
static struct vy_cache_entry *
@@ -128,10 +136,11 @@ vy_cache_tree_page_free(void *ctx, void *p)
void
vy_cache_create(struct vy_cache *cache, struct vy_cache_env *env,
- struct key_def *cmp_def)
+ struct key_def *cmp_def, bool is_primary)
{
cache->env = env;
cache->cmp_def = cmp_def;
+ cache->is_primary = is_primary;
cache->version = 1;
vy_cache_tree_create(&cache->cache_tree, cmp_def,
vy_cache_tree_page_alloc,
diff --git a/src/box/vy_cache.h b/src/box/vy_cache.h
index d2545152..7cb93155 100644
--- a/src/box/vy_cache.h
+++ b/src/box/vy_cache.h
@@ -160,6 +160,8 @@ struct vy_cache {
* key parts
*/
struct key_def *cmp_def;
+ /** Set if this cache is for a primary index. */
+ bool is_primary;
/* Tree of cache entries */
struct vy_cache_tree cache_tree;
/* The vesrion of state of cache_tree. Increments on every change */
@@ -174,10 +176,11 @@ struct vy_cache {
* Allocate and initialize tuple cache.
* @param env - pointer to common cache environment.
* @param cmp_def - key definition for tuple comparison.
+ * @param is_primary - set if cache is for primary index
*/
void
vy_cache_create(struct vy_cache *cache, struct vy_cache_env *env,
- struct key_def *cmp_def);
+ struct key_def *cmp_def, bool is_primary);
/**
* Destroy and deallocate tuple cache.
diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c
index cb3c436f..1994525e 100644
--- a/src/box/vy_lsm.c
+++ b/src/box/vy_lsm.c
@@ -188,7 +188,7 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env,
lsm->refs = 1;
lsm->dump_lsn = -1;
lsm->commit_lsn = -1;
- vy_cache_create(&lsm->cache, cache_env, cmp_def);
+ vy_cache_create(&lsm->cache, cache_env, cmp_def, index_def->iid == 0);
rlist_create(&lsm->sealed);
vy_range_tree_new(lsm->tree);
vy_range_heap_create(&lsm->range_heap);
diff --git a/test/unit/vy_iterators_helper.c b/test/unit/vy_iterators_helper.c
index 89603376..40d8d6a1 100644
--- a/test/unit/vy_iterators_helper.c
+++ b/test/unit/vy_iterators_helper.c
@@ -232,7 +232,7 @@ create_test_cache(uint32_t *fields, uint32_t *types,
{
*def = box_key_def_new(fields, types, key_cnt);
assert(*def != NULL);
- vy_cache_create(cache, &cache_env, *def);
+ vy_cache_create(cache, &cache_env, *def, true);
*format = tuple_format_new(&vy_tuple_format_vtab, def, 1, 0, NULL, 0,
NULL);
tuple_format_ref(*format);
diff --git a/test/unit/vy_point_lookup.c b/test/unit/vy_point_lookup.c
index eee25274..86877d7d 100644
--- a/test/unit/vy_point_lookup.c
+++ b/test/unit/vy_point_lookup.c
@@ -82,7 +82,7 @@ test_basic()
struct key_def *key_def = box_key_def_new(fields, types, 1);
isnt(key_def, NULL, "key_def is not NULL");
- vy_cache_create(&cache, &cache_env, key_def);
+ vy_cache_create(&cache, &cache_env, key_def, true);
struct tuple_format *format = tuple_format_new(&vy_tuple_format_vtab,
&key_def, 1, 0, NULL, 0,
NULL);
diff --git a/test/vinyl/cache.result b/test/vinyl/cache.result
index 2043eda9..d75de263 100644
--- a/test/vinyl/cache.result
+++ b/test/vinyl/cache.result
@@ -1077,3 +1077,50 @@ s:drop()
box.cfg{vinyl_cache = vinyl_cache}
---
...
+--
+-- gh-3655: statements are shared between primary and secondary
+-- index cache.
+--
+vinyl_cache = box.cfg.vinyl_cache
+---
+...
+box.cfg{vinyl_cache = 1024 * 1024}
+---
+...
+s = box.schema.space.create('test', {engine = 'vinyl'})
+---
+...
+_ = s:create_index('pk')
+---
+...
+_ = s:create_index('i1', {unique = false, parts = {2, 'string'}})
+---
+...
+_ = s:create_index('i2', {unique = false, parts = {3, 'string'}})
+---
+...
+for i = 1, 100 do pad = string.rep(i % 10, 1000) s:replace{i, pad, pad} end
+---
+...
+s.index.pk:count()
+---
+- 100
+...
+s.index.i1:count()
+---
+- 100
+...
+s.index.i2:count()
+---
+- 100
+...
+box.stat.vinyl().cache.used -- should be about 200 KB
+---
+- 216800
+...
+s:drop()
+---
+...
+box.cfg{vinyl_cache = vinyl_cache}
+---
+...
diff --git a/test/vinyl/cache.test.lua b/test/vinyl/cache.test.lua
index 6f414d5d..6d82249a 100644
--- a/test/vinyl/cache.test.lua
+++ b/test/vinyl/cache.test.lua
@@ -380,3 +380,21 @@ st2.put.rows - st1.put.rows
box.stat.vinyl().cache.used
s:drop()
box.cfg{vinyl_cache = vinyl_cache}
+
+--
+-- gh-3655: statements are shared between primary and secondary
+-- index cache.
+--
+vinyl_cache = box.cfg.vinyl_cache
+box.cfg{vinyl_cache = 1024 * 1024}
+s = box.schema.space.create('test', {engine = 'vinyl'})
+_ = s:create_index('pk')
+_ = s:create_index('i1', {unique = false, parts = {2, 'string'}})
+_ = s:create_index('i2', {unique = false, parts = {3, 'string'}})
+for i = 1, 100 do pad = string.rep(i % 10, 1000) s:replace{i, pad, pad} end
+s.index.pk:count()
+s.index.i1:count()
+s.index.i2:count()
+box.stat.vinyl().cache.used -- should be about 200 KB
+s:drop()
+box.cfg{vinyl_cache = vinyl_cache}
--
2.11.0
More information about the Tarantool-patches
mailing list