Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: kostja@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: Re: [PATCH] vinyl: remove runs not referenced by any checkpoint immediately
Date: Thu, 17 May 2018 19:35:53 +0300	[thread overview]
Message-ID: <20180517163553.4m3ok6g6drfbamw2@esperanza> (raw)
In-Reply-To: <50ccdb32ac67e8cd3f90acd998adf0e61861ae75.1526573189.git.vdavydov.dev@gmail.com>

On Thu, May 17, 2018 at 07:09:52PM +0300, Vladimir Davydov wrote:
> If a compacted run was created after the last checkpoint, it is not
> needed to recover from any checkpoint and hence can be deleted right
> away to save disk space.
> 
> Closes #3407
> ---
> https://github.com/tarantool/tarantool/issues/3407
> https://github.com/tarantool/tarantool/commits/gh-3407-vy-remove-unreferenced-runs-immediately
> 
>  src/box/vy_scheduler.c | 27 +++++++++--------
>  test/vinyl/gc.result   | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  test/vinyl/gc.test.lua | 35 ++++++++++++++++++++++
>  3 files changed, 126 insertions(+), 14 deletions(-)
> 
> diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
> index e1853e5d..4c9103cf 100644
> --- a/src/box/vy_scheduler.c
> +++ b/src/box/vy_scheduler.c
> @@ -1139,22 +1139,21 @@ vy_task_compact_complete(struct vy_scheduler *scheduler, struct vy_task *task)
>  		return -1;
>  	}
>  
> -	if (gc_lsn < 0) {
> -		/*
> -		 * If there is no last snapshot, i.e. we are in
> -		 * the middle of join, we can delete compacted
> -		 * run files right away.
> -		 */
> -		vy_log_tx_begin();
> -		rlist_foreach_entry(run, &unused_runs, in_unused) {
> -			if (vy_run_remove_files(index->env->path,
> -						index->space_id, index->id,
> -						run->id) == 0) {
> -				vy_log_forget_run(run->id);
> -			}
> +	/*
> +	 * Remove compacted run files that were created after
> +	 * the last checkpoint (and hence are not referenced
> +	 * by any checkpoint) immediately to save disk space.
> +	 */
> +	vy_log_tx_begin();
> +	rlist_foreach_entry(run, &unused_runs, in_unused) {
> +		if (run->dump_lsn > gc_lsn &&
> +		    vy_run_remove_files(index->env->path,
> +					index->space_id, index->id,
> +					run->id) == 0) {
> +			vy_log_forget_run(run->id);
>  		}
> -		vy_log_tx_try_commit();
>  	}
> +	vy_log_tx_try_commit();
>  
>  	/*
>  	 * Account the new run if it is not empty,

Oops, this patch breaks vinyl/errinj_gc test, which doesn't expect
compaction to remove any files if ERRINJ_VY_GC is set. Fixed on the
branch. Here's the incremental diff:

diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 55a68104..552d42ba 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -3389,10 +3389,6 @@ vy_gc_cb(const struct vy_log_record *record, void *cb_arg)
 		goto out;
 	}
 
-	ERROR_INJECT(ERRINJ_VY_GC,
-		     {say_error("error injection: vinyl run %lld not deleted",
-				(long long)record->run_id); goto out;});
-
 	/* Try to delete files. */
 	if (vy_run_remove_files(arg->env->path, arg->space_id,
 				arg->index_id, record->run_id) != 0)
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index edae5270..980bc4d2 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -2434,6 +2434,9 @@ int
 vy_run_remove_files(const char *dir, uint32_t space_id,
 		    uint32_t iid, int64_t run_id)
 {
+	ERROR_INJECT(ERRINJ_VY_GC,
+		     {say_error("error injection: vinyl run %lld not deleted",
+				(long long)run_id); return -1;});
 	int ret = 0;
 	char path[PATH_MAX];
 	for (int type = 0; type < vy_file_MAX; type++) {

  reply	other threads:[~2018-05-17 16:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-17 16:09 Vladimir Davydov
2018-05-17 16:35 ` Vladimir Davydov [this message]
2018-05-17 20:40 ` Konstantin Osipov
2018-05-17 22:39   ` 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=20180517163553.4m3ok6g6drfbamw2@esperanza \
    --to=vdavydov.dev@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH] vinyl: remove runs not referenced by any checkpoint immediately' \
    /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