From: Vladimir Davydov <vdavydov.dev@gmail.com> To: tarantool-patches@freelists.org Subject: [PATCH v2 1/7] vinyl: don't pin index for iterator lifetime Date: Mon, 19 Aug 2019 19:53:14 +0300 [thread overview] Message-ID: <4bd53dc8721b00d27d187bf55b29dc8f0814bb30.1566233187.git.vdavydov.dev@gmail.com> (raw) In-Reply-To: <cover.1566233187.git.vdavydov.dev@gmail.com> 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 0dd73045..ee6b2728 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 @@ -3730,8 +3728,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) { @@ -3804,10 +3800,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; @@ -3820,9 +3823,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; } @@ -3833,9 +3838,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; @@ -3849,12 +3860,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) @@ -3864,9 +3874,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; } @@ -3877,7 +3890,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 * @@ -3918,10 +3931,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
next prev parent reply other threads:[~2019-08-19 16:53 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-19 16:53 [PATCH v2 0/7] Join replicas off the current read view Vladimir Davydov 2019-08-19 16:53 ` Vladimir Davydov [this message] 2019-08-19 20:35 ` [tarantool-patches] Re: [PATCH v2 1/7] vinyl: don't pin index for iterator lifetime Konstantin Osipov 2019-08-19 16:53 ` [PATCH v2 2/7] vinyl: don't exempt dropped indexes from dump and compaction Vladimir Davydov 2019-08-19 20:47 ` [tarantool-patches] " Konstantin Osipov 2019-08-20 8:12 ` Vladimir Davydov 2019-08-20 9:02 ` Vladimir Davydov 2019-08-20 11:52 ` Konstantin Osipov 2019-08-20 14:16 ` Vladimir Davydov 2019-08-19 16:53 ` [PATCH v2 3/7] vinyl: get rid of vy_env::join_lsn Vladimir Davydov 2019-08-19 20:49 ` [tarantool-patches] " Konstantin Osipov 2019-08-19 16:53 ` [PATCH v2 4/7] memtx: use ref counting to pin indexes for snapshot Vladimir Davydov 2019-08-19 20:50 ` [tarantool-patches] " Konstantin Osipov 2019-08-19 16:53 ` [PATCH v2 5/7] memtx: enter small delayed free mode from snapshot iterator Vladimir Davydov 2019-08-19 20:51 ` [tarantool-patches] " Konstantin Osipov 2019-08-19 16:53 ` [PATCH v2 6/7] space: get rid of apply_initial_join_row method Vladimir Davydov 2019-08-19 20:54 ` [tarantool-patches] " Konstantin Osipov 2019-08-19 16:53 ` [PATCH v2 7/7] relay: join new replicas off read view Vladimir Davydov 2019-08-19 20:57 ` [tarantool-patches] " Konstantin Osipov 2019-08-20 8:16 ` Vladimir Davydov 2019-08-20 11:53 ` Konstantin Osipov 2019-08-20 12:05 ` Vladimir Davydov 2019-08-20 13:50 ` Konstantin Osipov 2019-08-20 14:03 ` Vladimir Davydov 2019-08-21 22:08 ` Konstantin Osipov 2019-08-22 8:05 ` Vladimir Davydov 2019-08-19 16:54 ` [PATCH v2 0/7] Join replicas off the current " Vladimir Davydov 2019-08-20 8:53 ` Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4bd53dc8721b00d27d187bf55b29dc8f0814bb30.1566233187.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH v2 1/7] vinyl: don'\''t pin index for iterator lifetime' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox