From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 1/7] vinyl: fix accounting of secondary index cache statements Date: Sun, 2 Sep 2018 23:18:54 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: 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