Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: tarantool-patches@freelists.org
Subject: [PATCH 05/10] vinyl: don't purge deleted runs from vylog on compaction
Date: Fri, 17 May 2019 17:52:39 +0300	[thread overview]
Message-ID: <67424f76249971cf6c4fc5a361f294a0b3c88283.1558103547.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1558103547.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1558103547.git.vdavydov.dev@gmail.com>

After compacting runs, we first mark them as dropped (VY_LOG_DROP_RUN),
then try to delete their files unless they are needed for recovery from
the checkpoint, and finally mark them as not needed in the vylog
(VY_LOG_FORGET_RUN). There's a potential race sitting here: the problem
is the garbage collector might kick in after files are dropped, but
before they are marked as not needed. If this happens, there will be
runs that have two VY_LOG_FORGET_RUN records, which will break recovery:

  Run XX is forgotten, but not registered

The following patches make the race more likely to happen so let's
eliminate it by making the garbage collector the only one who can mark
runs as not needed (i.e. write VY_LOG_FORGET_RUN record). There will
be no warnings, because the garbage collector silently ignores ENOENT
errors, see vy_gc().

Another good thing about this patch is that now we never yield inside
a vylog transaction, which makes it easier to remove the vylog latch
blocking implementation of transactional DDL.
---
 src/box/vy_scheduler.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index fabb4bb4..0180331e 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -1523,16 +1523,17 @@ vy_task_compaction_complete(struct vy_task *task)
 	 * Remove compacted run files that were created after
 	 * the last checkpoint (and hence are not referenced
 	 * by any checkpoint) immediately to save disk space.
+	 *
+	 * Don't bother logging it to avoid a potential race
+	 * with a garbage collection task, which may be cleaning
+	 * up concurrently. The log will be cleaned up on the
+	 * next checkpoint.
 	 */
-	vy_log_tx_begin();
 	rlist_foreach_entry(run, &unused_runs, in_unused) {
-		if (run->dump_lsn > gc_lsn &&
-		    vy_run_remove_files(lsm->env->path, lsm->space_id,
-					lsm->index_id, run->id) == 0) {
-			vy_log_forget_run(run->id);
-		}
+		if (run->dump_lsn > gc_lsn)
+			vy_run_remove_files(lsm->env->path, lsm->space_id,
+					    lsm->index_id, run->id);
 	}
-	vy_log_tx_try_commit();
 
 	/*
 	 * Account the new run if it is not empty,
-- 
2.11.0

  parent reply	other threads:[~2019-05-17 14:52 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-17 14:52 [PATCH 00/10] vinyl: don't yield in DDL on_commit triggers Vladimir Davydov
2019-05-17 14:52 ` [PATCH 01/10] box: zap atfork callback Vladimir Davydov
2019-05-18 18:37   ` [tarantool-patches] " Konstantin Osipov
2019-05-20  8:13     ` Vladimir Davydov
2019-06-01  8:16     ` Konstantin Osipov
2019-06-06 10:04       ` Vladimir Davydov
2019-05-17 14:52 ` [PATCH 02/10] vinyl: add a separate thread for vylog Vladimir Davydov
2019-05-18 18:39   ` [tarantool-patches] " Konstantin Osipov
2019-05-20  8:17     ` Vladimir Davydov
2019-06-01  8:26   ` Konstantin Osipov
2019-06-06 10:20     ` Vladimir Davydov
2019-05-17 14:52 ` [PATCH 03/10] vinyl: move vylog recovery to vylog thread Vladimir Davydov
2019-06-01  8:36   ` [tarantool-patches] " Konstantin Osipov
2019-06-06 10:23     ` Vladimir Davydov
2019-06-07 13:39       ` Konstantin Osipov
2019-06-10 15:24         ` Vladimir Davydov
2019-06-07 13:40       ` Konstantin Osipov
2019-05-17 14:52 ` [PATCH 04/10] vinyl: rework vylog transaction backlog implementation Vladimir Davydov
2019-06-01  8:38   ` [tarantool-patches] " Konstantin Osipov
2019-06-06 11:58     ` Vladimir Davydov
2019-05-17 14:52 ` Vladimir Davydov [this message]
2019-05-18 18:47   ` [tarantool-patches] Re: [PATCH 05/10] vinyl: don't purge deleted runs from vylog on compaction Konstantin Osipov
2019-05-20  8:27     ` Vladimir Davydov
2019-06-01  8:39   ` Konstantin Osipov
2019-06-06 12:40     ` Vladimir Davydov
2019-05-17 14:52 ` [PATCH 06/10] vinyl: lock out compaction while checkpointing is in progress Vladimir Davydov
2019-05-17 14:52 ` [PATCH 07/10] vinyl: don't access last vylog signature outside vylog implementation Vladimir Davydov
2019-05-17 14:52 ` [PATCH 08/10] vinyl: zap ERRINJ_VY_LOG_FLUSH_DELAY Vladimir Davydov
2019-05-17 14:52 ` [PATCH 09/10] key_def: pass alloc callback to key_def_dump_parts Vladimir Davydov
2019-05-18 18:52   ` [tarantool-patches] " Konstantin Osipov
2019-05-20  8:34     ` Vladimir Davydov
2019-06-01  8:41     ` Konstantin Osipov
2019-06-10 15:28       ` Vladimir Davydov
2019-06-16 14:57         ` Konstantin Osipov
2019-05-17 14:52 ` [PATCH 10/10] vinyl: get rid of the latch protecting vylog buffer Vladimir Davydov
2019-06-01  8:44   ` [tarantool-patches] " Konstantin Osipov
2019-06-06 13:15     ` Vladimir Davydov
2019-05-18 18:35 ` [tarantool-patches] Re: [PATCH 00/10] vinyl: don't yield in DDL on_commit triggers Konstantin Osipov
2019-05-20  8:09   ` Vladimir Davydov
2019-06-01  8:09 ` Konstantin Osipov

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=67424f76249971cf6c4fc5a361f294a0b3c88283.1558103547.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 05/10] vinyl: don'\''t purge deleted runs from vylog on compaction' \
    /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