From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 04/13] vinyl: don't pin index for iterator lifetime Date: Sat, 10 Aug 2019 13:03:31 +0300 Message-Id: <293094dad65d1edcdd008582d62c439bde286378.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: vinyl_iterator keeps a reference to the LSM tree it was created for until it is destroyed, which may take indefinitely long in case the iterator is used in Lua. Actually, we don't need to keep a reference to the index for the whole iterator lifetime, because iterator_next() wrapper guarantees that iterator->next won't be called for a dropped index. What we need to do is keep a reference while we are yielding on disk read, similarly to vinyl_index_get(). Currently, pinning an index for indefinitely long is harmless, because an LSM tree is exempted from dump/compaction as soon as it is dropped so we just pin some memory, that's all. However, following patches are going to enable dump/compaction for dropped but pinned indexes in order to implement snapshot iterator so we better relax the dependency of an iterator on an index know. While we are at it, let's remove env and lsm members of vinyl_iterator struct: lsm can be accessed via vy_read_iterator embedded in the struct while env is only needed to access iterator_pool so we better store a pointer to the pool in vinyl_iterator instead. --- src/box/vinyl.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 2d07e336..93002fdf 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -156,10 +156,8 @@ vy_gc(struct vy_env *env, struct vy_recovery *recovery, struct vinyl_iterator { struct iterator base; - /** Vinyl environment. */ - struct vy_env *env; - /** LSM tree this iterator is for. */ - struct vy_lsm *lsm; + /** Memory pool the iterator was allocated from. */ + struct mempool *pool; /** * Points either to tx_autocommit for autocommit mode * or to a multi-statement transaction active when the @@ -3727,8 +3725,6 @@ static void vinyl_iterator_close(struct vinyl_iterator *it) { vy_read_iterator_close(&it->iterator); - vy_lsm_unref(it->lsm); - it->lsm = NULL; tuple_unref(it->key.stmt); it->key = vy_entry_none(); if (it->tx == &it->tx_autocommit) { @@ -3801,10 +3797,17 @@ vinyl_iterator_primary_next(struct iterator *base, struct tuple **ret) assert(base->next = vinyl_iterator_primary_next); struct vinyl_iterator *it = (struct vinyl_iterator *)base; - assert(it->lsm->index_id == 0); + struct vy_lsm *lsm = it->iterator.lsm; + assert(lsm->index_id == 0); + /* + * Make sure the LSM tree isn't deleted while we are + * reading from it. + */ + vy_lsm_ref(lsm); if (vinyl_iterator_check_tx(it) != 0) goto fail; + struct vy_entry entry; if (vy_read_iterator_next(&it->iterator, &entry) != 0) goto fail; @@ -3817,9 +3820,11 @@ vinyl_iterator_primary_next(struct iterator *base, struct tuple **ret) tuple_bless(entry.stmt); } *ret = entry.stmt; + vy_lsm_unref(lsm); return 0; fail: vinyl_iterator_close(it); + vy_lsm_unref(lsm); return -1; } @@ -3830,9 +3835,15 @@ vinyl_iterator_secondary_next(struct iterator *base, struct tuple **ret) assert(base->next = vinyl_iterator_secondary_next); struct vinyl_iterator *it = (struct vinyl_iterator *)base; - assert(it->lsm->index_id > 0); - struct vy_entry partial, entry; + struct vy_lsm *lsm = it->iterator.lsm; + assert(lsm->index_id > 0); + /* + * Make sure the LSM tree isn't deleted while we are + * reading from it. + */ + vy_lsm_ref(lsm); + struct vy_entry partial, entry; next: if (vinyl_iterator_check_tx(it) != 0) goto fail; @@ -3846,12 +3857,11 @@ next: vinyl_iterator_account_read(it, start_time, NULL); vinyl_iterator_close(it); *ret = NULL; - return 0; + goto out; } ERROR_INJECT_YIELD(ERRINJ_VY_DELAY_PK_LOOKUP); /* Get the full tuple from the primary index. */ - if (vy_get_by_secondary_tuple(it->lsm, it->tx, - vy_tx_read_view(it->tx), + if (vy_get_by_secondary_tuple(lsm, it->tx, vy_tx_read_view(it->tx), partial, &entry) != 0) goto fail; if (entry.stmt == NULL) @@ -3861,9 +3871,12 @@ next: *ret = entry.stmt; tuple_bless(*ret); tuple_unref(*ret); +out: + vy_lsm_unref(lsm); return 0; fail: vinyl_iterator_close(it); + vy_lsm_unref(lsm); return -1; } @@ -3874,7 +3887,7 @@ vinyl_iterator_free(struct iterator *base) struct vinyl_iterator *it = (struct vinyl_iterator *)base; if (base->next != vinyl_iterator_last) vinyl_iterator_close(it); - mempool_free(&it->env->iterator_pool, it); + mempool_free(it->pool, it); } static struct iterator * @@ -3915,10 +3928,7 @@ vinyl_index_create_iterator(struct index *base, enum iterator_type type, else it->base.next = vinyl_iterator_secondary_next; it->base.free = vinyl_iterator_free; - - it->env = env; - it->lsm = lsm; - vy_lsm_ref(lsm); + it->pool = &env->iterator_pool; if (tx != NULL) { /* -- 2.20.1