Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: tarantool-patches@freelists.org
Subject: [PATCH 06/12] vinyl: don't add dropped LSM trees to the scheduler during recovery
Date: Tue, 15 Jan 2019 17:17:15 +0300	[thread overview]
Message-ID: <6a4b0667410c9295508c5ff9ca0721443d04fa37.1547558871.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1547558871.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1547558871.git.vdavydov.dev@gmail.com>

During local recovery we may encounter an LSM tree marked as dropped.
This means that the LSM tree was dropped before restart and hence will
be deleted before recovery completion. There's no need to add such trees
to the vinyl scheduler - it looks confusing and can potentially result
in mistakes when the code gets modified.
---
 src/box/vinyl.c        | 42 +++++++++++++++++++++++++++---------------
 src/box/vy_scheduler.c |  2 ++
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 8028fd2b..5965700b 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -749,18 +749,16 @@ vinyl_index_open(struct index *index)
 		diag_set(SystemError, "can not access vinyl data directory");
 		return -1;
 	}
-	int rc;
 	switch (env->status) {
 	case VINYL_ONLINE:
 		/*
 		 * The recovery is complete, simply
 		 * create a new index.
 		 */
-		rc = vy_lsm_create(lsm);
-		if (rc == 0) {
-			/* Make sure reader threads are up and running. */
-			vy_run_env_enable_coio(&env->run_env);
-		}
+		if (vy_lsm_create(lsm) != 0)
+			return -1;
+		/* Make sure reader threads are up and running. */
+		vy_run_env_enable_coio(&env->run_env);
 		break;
 	case VINYL_INITIAL_RECOVERY_REMOTE:
 	case VINYL_FINAL_RECOVERY_REMOTE:
@@ -769,7 +767,8 @@ vinyl_index_open(struct index *index)
 		 * exist locally, and we should create the
 		 * index directory from scratch.
 		 */
-		rc = vy_lsm_create(lsm);
+		if (vy_lsm_create(lsm) != 0)
+			return -1;
 		break;
 	case VINYL_INITIAL_RECOVERY_LOCAL:
 	case VINYL_FINAL_RECOVERY_LOCAL:
@@ -779,17 +778,30 @@ vinyl_index_open(struct index *index)
 		 * have already been created, so try to load
 		 * the index files from it.
 		 */
-		rc = vy_lsm_recover(lsm, env->recovery, &env->run_env,
-				    vclock_sum(env->recovery_vclock),
-				    env->status == VINYL_INITIAL_RECOVERY_LOCAL,
-				    env->force_recovery);
+		if (vy_lsm_recover(lsm, env->recovery, &env->run_env,
+				   vclock_sum(env->recovery_vclock),
+				   env->status == VINYL_INITIAL_RECOVERY_LOCAL,
+				   env->force_recovery) != 0)
+			return -1;
 		break;
 	default:
 		unreachable();
 	}
-	if (rc == 0)
+	/*
+	 * Add the new LSM tree to the scheduler so that it can
+	 * be dumped and compacted.
+	 *
+	 * Note, during local recovery an LSM tree may be marked
+	 * as dropped, which means that it will be dropped before
+	 * recovery is complete. In this case there's no need in
+	 * letting the scheduler know about it.
+	 */
+	if (!lsm->is_dropped)
 		vy_scheduler_add_lsm(&env->scheduler, lsm);
-	return rc;
+	else
+		assert(env->status == VINYL_INITIAL_RECOVERY_LOCAL ||
+		       env->status == VINYL_FINAL_RECOVERY_LOCAL);
+	return 0;
 }
 
 static void
@@ -911,8 +923,6 @@ vinyl_index_commit_drop(struct index *index, int64_t lsn)
 	struct vy_env *env = vy_env(index->engine);
 	struct vy_lsm *lsm = vy_lsm(index);
 
-	vy_scheduler_remove_lsm(&env->scheduler, lsm);
-
 	/*
 	 * We can't abort here, because the index drop request has
 	 * already been written to WAL. So if we fail to write the
@@ -924,6 +934,8 @@ vinyl_index_commit_drop(struct index *index, int64_t lsn)
 	if (env->status == VINYL_FINAL_RECOVERY_LOCAL && lsm->is_dropped)
 		return;
 
+	vy_scheduler_remove_lsm(&env->scheduler, lsm);
+
 	lsm->is_dropped = true;
 
 	vy_log_tx_begin();
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index 63ac948f..f431eb24 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -503,6 +503,7 @@ vy_scheduler_destroy(struct vy_scheduler *scheduler)
 void
 vy_scheduler_add_lsm(struct vy_scheduler *scheduler, struct vy_lsm *lsm)
 {
+	assert(!lsm->is_dropped);
 	assert(lsm->in_dump.pos == UINT32_MAX);
 	assert(lsm->in_compaction.pos == UINT32_MAX);
 	vy_dump_heap_insert(&scheduler->dump_heap, &lsm->in_dump);
@@ -513,6 +514,7 @@ vy_scheduler_add_lsm(struct vy_scheduler *scheduler, struct vy_lsm *lsm)
 void
 vy_scheduler_remove_lsm(struct vy_scheduler *scheduler, struct vy_lsm *lsm)
 {
+	assert(!lsm->is_dropped);
 	assert(lsm->in_dump.pos != UINT32_MAX);
 	assert(lsm->in_compaction.pos != UINT32_MAX);
 	vy_dump_heap_delete(&scheduler->dump_heap, &lsm->in_dump);
-- 
2.11.0

  parent reply	other threads:[~2019-01-15 14:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-15 14:17 [PATCH 00/12] vinyl: statistics improvements Vladimir Davydov
2019-01-15 14:17 ` [PATCH 01/12] test: rename vinyl/info to vinyl/stat Vladimir Davydov
2019-01-17 11:32   ` [tarantool-patches] " Konstantin Osipov
2019-01-15 14:17 ` [PATCH 02/12] test: split vinyl/errinj Vladimir Davydov
2019-01-17 11:33   ` [tarantool-patches] " Konstantin Osipov
2019-01-15 14:17 ` [PATCH 03/12] vinyl: rename dump/compact in/out to input/output Vladimir Davydov
2019-01-17 11:33   ` [tarantool-patches] " Konstantin Osipov
2019-01-15 14:17 ` [PATCH 04/12] vinyl: rename compact to compaction Vladimir Davydov
2019-01-17 11:34   ` [tarantool-patches] " Konstantin Osipov
2019-01-17 12:08     ` Vladimir Davydov
2019-01-17 13:51       ` Konstantin Osipov
2019-01-15 14:17 ` [PATCH 05/12] vinyl: bump range version in vy_range.c Vladimir Davydov
2019-01-15 14:17 ` Vladimir Davydov [this message]
2019-01-15 14:17 ` [PATCH 07/12] vinyl: move global dump/compaction statistics to scheduler Vladimir Davydov
2019-01-16 16:36   ` Vladimir Davydov
2019-01-15 14:17 ` [PATCH 08/12] vinyl: add dump count to global scheduler statistics Vladimir Davydov
2019-01-15 14:17 ` [PATCH 09/12] vinyl: don't account secondary indexes to scheduler.dump_input Vladimir Davydov
2019-01-15 14:17 ` [PATCH 10/12] vinyl: add task accounting to global scheduler statistics Vladimir Davydov
2019-01-15 14:17 ` [PATCH 11/12] vinyl: add dump/compaction time to statistics Vladimir Davydov
2019-01-15 14:17 ` [PATCH 12/12] vinyl: add last level size " Vladimir Davydov
2019-01-17 11:35   ` [tarantool-patches] " Konstantin Osipov
2019-01-17 11:32 ` [tarantool-patches] Re: [PATCH 00/12] vinyl: statistics improvements Konstantin Osipov
2019-01-17 12:06   ` Vladimir Davydov
2019-01-20 21:16 ` 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=6a4b0667410c9295508c5ff9ca0721443d04fa37.1547558871.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 06/12] vinyl: don'\''t add dropped LSM trees to the scheduler during recovery' \
    /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