From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [RFC PATCH 10/23] vinyl: store full tuples in secondary index cache Date: Sun, 8 Jul 2018 19:48:41 +0300 Message-Id: In-Reply-To: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: Currently, both vy_read_iterator_next() and vy_point_lookup() add the returned tuple to the tuple cache. As a result, we store partial tuples in a secondary index tuple cache although we could store full tuples (we have to retrieve them anyway when reading a secondary index). This means wasting memory. Besides, when the #2129 gets implemented, there will be tuples in a secondary index that have to be skipped as they have been overwritten in the primary index. Caching them would be inefficient and error prone. So let's call vy_cache_add() from the upper level and add only full tuples to the cache. Closes #3478 Needed for #2129 --- src/box/vinyl.c | 11 +++++++++ src/box/vy_point_lookup.c | 5 +--- src/box/vy_read_iterator.c | 61 ++++++++++++++++++++++------------------------ src/box/vy_read_iterator.h | 24 ++++++++++++++++++ 4 files changed, 65 insertions(+), 36 deletions(-) diff --git a/src/box/vinyl.c b/src/box/vinyl.c index d1b6839e..f05a4a0e 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -1302,6 +1302,10 @@ vy_get_by_secondary_tuple(struct vy_lsm *lsm, struct vy_tx *tx, say_warn("%s: key %s missing in primary index", vy_lsm_name(lsm), vy_stmt_str(tuple)); } + + if ((*rv)->vlsn == INT64_MAX) + vy_cache_add(&lsm->pk->cache, *result, NULL, tuple, ITER_EQ); + return 0; } @@ -1348,6 +1352,8 @@ vy_get(struct vy_lsm *lsm, struct vy_tx *tx, } else { *result = tuple; } + if ((*rv)->vlsn == INT64_MAX) + vy_cache_add(&lsm->cache, *result, NULL, key, ITER_EQ); return 0; } @@ -1364,6 +1370,8 @@ vy_get(struct vy_lsm *lsm, struct vy_tx *tx, if (rc != 0 || *result != NULL) break; } + if (rc == 0) + vy_read_iterator_cache_add(&itr, *result); vy_read_iterator_close(&itr); return rc; } @@ -3803,6 +3811,7 @@ vinyl_iterator_primary_next(struct iterator *base, struct tuple **ret) if (vy_read_iterator_next(&it->iterator, ret) != 0) goto fail; + vy_read_iterator_cache_add(&it->iterator, *ret); if (*ret == NULL) { /* EOF. Close the iterator immediately. */ vinyl_iterator_close(it); @@ -3838,6 +3847,7 @@ next: if (tuple == NULL) { /* EOF. Close the iterator immediately. */ + vy_read_iterator_cache_add(&it->iterator, NULL); vinyl_iterator_close(it); *ret = NULL; return 0; @@ -3857,6 +3867,7 @@ next: goto fail; if (*ret == NULL) goto next; + vy_read_iterator_cache_add(&it->iterator, *ret); tuple_bless(*ret); tuple_unref(*ret); return 0; diff --git a/src/box/vy_point_lookup.c b/src/box/vy_point_lookup.c index f2261fdf..5e43340b 100644 --- a/src/box/vy_point_lookup.c +++ b/src/box/vy_point_lookup.c @@ -280,11 +280,8 @@ done: if (rc != 0) return -1; - if (*ret != NULL) { + if (*ret != NULL) vy_stmt_counter_acct_tuple(&lsm->stat.get, *ret); - if ((*rv)->vlsn == INT64_MAX) - vy_cache_add(&lsm->cache, *ret, NULL, key, ITER_EQ); - } double latency = ev_monotonic_now(loop()) - start_time; latency_collect(&lsm->stat.latency, latency); diff --git a/src/box/vy_read_iterator.c b/src/box/vy_read_iterator.c index 160bb899..954fc0df 100644 --- a/src/box/vy_read_iterator.c +++ b/src/box/vy_read_iterator.c @@ -845,24 +845,17 @@ vy_read_iterator_next(struct vy_read_iterator *itr, struct tuple **result) ev_tstamp start_time = ev_monotonic_now(loop()); struct vy_lsm *lsm = itr->lsm; - struct tuple *stmt, *prev_stmt; + struct tuple *stmt; - /* - * Remember the statement returned by the last iteration. - * We will need it to update the cache. - */ - prev_stmt = itr->last_stmt; - if (prev_stmt != NULL) - tuple_ref(prev_stmt); - else /* first iteration */ - lsm->stat.lookup++; + if (itr->last_stmt == NULL) + lsm->stat.lookup++; /* first iteration */ next_key: if (vy_read_iterator_advance(itr) != 0) - goto err; + return -1; if (vy_read_iterator_apply_history(itr, &stmt) != 0) - goto err; + return -1; if (vy_read_iterator_track_read(itr, stmt) != 0) - goto err; + return -1; if (itr->last_stmt != NULL) tuple_unref(itr->last_stmt); @@ -877,9 +870,9 @@ next_key: * previous + current tuple as an unbroken chain. */ if (vy_stmt_lsn(stmt) == INT64_MAX) { - if (prev_stmt != NULL) - tuple_unref(prev_stmt); - prev_stmt = NULL; + if (itr->last_cached_stmt != NULL) + tuple_unref(itr->last_cached_stmt); + itr->last_cached_stmt = NULL; } goto next_key; } @@ -887,18 +880,6 @@ next_key: vy_stmt_type(stmt) == IPROTO_INSERT || vy_stmt_type(stmt) == IPROTO_REPLACE); - /* - * Store the result in the cache provided we are reading - * the latest data. - */ - if ((**itr->read_view).vlsn == INT64_MAX) { - vy_cache_add(&lsm->cache, stmt, prev_stmt, - itr->key, itr->iterator_type); - } - if (prev_stmt != NULL) - tuple_unref(prev_stmt); - - /* Update LSM tree stats. */ if (stmt != NULL) vy_stmt_counter_acct_tuple(&lsm->stat.get, stmt); @@ -914,10 +895,24 @@ next_key: *result = stmt; return 0; -err: - if (prev_stmt != NULL) - tuple_unref(prev_stmt); - return -1; +} + +void +vy_read_iterator_cache_add(struct vy_read_iterator *itr, struct tuple *stmt) +{ + if ((**itr->read_view).vlsn != INT64_MAX) { + if (itr->last_cached_stmt != NULL) + tuple_unref(itr->last_cached_stmt); + itr->last_cached_stmt = NULL; + return; + } + vy_cache_add(&itr->lsm->cache, stmt, itr->last_cached_stmt, + itr->key, itr->iterator_type); + if (stmt != NULL) + tuple_ref(stmt); + if (itr->last_cached_stmt != NULL) + tuple_unref(itr->last_cached_stmt); + itr->last_cached_stmt = stmt; } /** @@ -928,6 +923,8 @@ vy_read_iterator_close(struct vy_read_iterator *itr) { if (itr->last_stmt != NULL) tuple_unref(itr->last_stmt); + if (itr->last_cached_stmt != NULL) + tuple_unref(itr->last_cached_stmt); vy_read_iterator_cleanup(itr); free(itr->src); TRASH(itr); diff --git a/src/box/vy_read_iterator.h b/src/box/vy_read_iterator.h index 2cac1087..baab8859 100644 --- a/src/box/vy_read_iterator.h +++ b/src/box/vy_read_iterator.h @@ -65,6 +65,11 @@ struct vy_read_iterator { /** Last statement returned by vy_read_iterator_next(). */ struct tuple *last_stmt; /** + * Last statement added to the tuple cache by + * vy_read_iterator_cache_add(). + */ + struct tuple *last_cached_stmt; + /** * Copy of lsm->range_tree_version. * Used for detecting range tree changes. */ @@ -142,6 +147,25 @@ NODISCARD int vy_read_iterator_next(struct vy_read_iterator *itr, struct tuple **result); /** + * Add the last tuple returned by the read iterator to the cache. + * @param itr Read iterator + * @param stmt Last tuple returned by the iterator. + * + * We use a separate function for populating the cache rather than + * doing that right in vy_read_iterator_next() so that we can store + * full tuples in a secondary index cache, thus saving some memory. + * + * Usage pattern: + * - Call vy_read_iterator_next() to get a partial tuple. + * - Call vy_point_lookup() to get the full tuple corresponding + * to the partial tuple returned by the iterator. + * - Call vy_read_iterator_cache_add() on the full tuple to add + * the result to the cache. + */ +void +vy_read_iterator_cache_add(struct vy_read_iterator *itr, struct tuple *stmt); + +/** * Close the iterator and free resources. */ void -- 2.11.0