Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: tarantool-patches@freelists.org
Subject: [PATCH 04/13] vinyl: don't pin index for iterator lifetime
Date: Sat, 10 Aug 2019 13:03:31 +0300	[thread overview]
Message-ID: <293094dad65d1edcdd008582d62c439bde286378.1565430177.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1565430177.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 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

  parent reply	other threads:[~2019-08-10 10:03 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-10 10:03 [PATCH 00/13] Join replicas off the current read view Vladimir Davydov
2019-08-10 10:03 ` [PATCH 01/13] vinyl: embed engine in vy_env Vladimir Davydov
2019-08-12 22:14   ` [tarantool-patches] " Konstantin Osipov
2019-08-14 13:09   ` Vladimir Davydov
2019-08-10 10:03 ` [PATCH 02/13] vinyl: embed index in vy_lsm Vladimir Davydov
2019-08-12 22:14   ` [tarantool-patches] " Konstantin Osipov
2019-08-14 13:09   ` Vladimir Davydov
2019-08-10 10:03 ` [PATCH 03/13] vinyl: move reference counting from vy_lsm to index Vladimir Davydov
2019-08-12 22:16   ` [tarantool-patches] " Konstantin Osipov
2019-08-14 13:09   ` Vladimir Davydov
2019-08-10 10:03 ` Vladimir Davydov [this message]
2019-08-10 10:03 ` [PATCH 05/13] vinyl: don't exempt dropped indexes from dump and compaction Vladimir Davydov
2019-08-10 10:03 ` [PATCH 06/13] memtx: don't store pointers to index internals in iterator Vladimir Davydov
2019-08-12 22:21   ` [tarantool-patches] " Konstantin Osipov
2019-08-14 13:10   ` Vladimir Davydov
2019-08-10 10:03 ` [PATCH 07/13] memtx: use ref counting to pin indexes for snapshot Vladimir Davydov
2019-08-12 22:24   ` [tarantool-patches] " Konstantin Osipov
2019-08-13 10:56     ` Vladimir Davydov
2019-08-13 16:08       ` Georgy Kirichenko
2019-08-10 10:03 ` [PATCH 08/13] memtx: allow snapshot iterator to fail Vladimir Davydov
2019-08-12 22:25   ` [tarantool-patches] " Konstantin Osipov
2019-08-14 13:10   ` Vladimir Davydov
2019-08-10 10:03 ` [PATCH 09/13] memtx: enter small delayed free mode from snapshot iterator Vladimir Davydov
2019-08-12 22:27   ` [tarantool-patches] " Konstantin Osipov
2019-08-13 10:59     ` Vladimir Davydov
2019-08-10 10:03 ` [PATCH 10/13] wal: make wal_sync fail on write error Vladimir Davydov
2019-08-12 22:29   ` [tarantool-patches] " Konstantin Osipov
2019-08-14 16:48   ` Vladimir Davydov
2019-08-10 10:03 ` [PATCH 11/13] xrow: factor out helper for setting REPLACE request body Vladimir Davydov
2019-08-12 22:29   ` [tarantool-patches] " Konstantin Osipov
2019-08-14 13:11   ` Vladimir Davydov
2019-08-10 10:03 ` [PATCH 12/13] test: disable replication/on_schema_init Vladimir Davydov
2019-08-12 22:31   ` [tarantool-patches] " Konstantin Osipov
2019-08-10 10:03 ` [PATCH 13/13] relay: join new replicas off read view 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=293094dad65d1edcdd008582d62c439bde286378.1565430177.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 04/13] 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