[PATCH v5 2/2] xlog: Remove inprogress files on start

[Ilya Markov <imarkov@tarantool.org>]

When tarantool crashes during writing to vylog, index, run or snapshot,
inprogress files remain. But garbage collector doesn't take into account
these files. So they remain until they are removed manually.

Fix this with adding deletion of such files to functions memtx_engine_end_recovery,
vinyl_engine_end_recovery and vy_run_remove_files.

- Add 3 errinj which simulate the crash before renaming inprogress files.
- Moved const string inprogress_suffix to xlog.h and replace it with
  #defiled string in order to reuse it in vinyl run and index files removal.
- Add 2 new vy_file_type in order to support formatting of inprogress
  files names.

Closes #3406
---
 src/box/memtx_engine.c   |   1 +
 src/box/vinyl.c          |   3 +
 src/box/vy_log.c         |   7 +-
 src/box/vy_run.c         |  19 ++++-
 src/box/vy_run.h         |   2 +
 src/box/xlog.c           |  29 +++++++-
 src/box/xlog.h           |  11 +++
 src/errinj.h             |   3 +
 test/box/errinj.result   | 185 +++++++++++++++++++++++++++++++++++++++++++++++
 test/box/errinj.test.lua |  72 +++++++++++++++++-
 10 files changed, 326 insertions(+), 6 deletions(-)

diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index 9a9aff5..d211bcd 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -303,6 +303,7 @@ memtx_engine_end_recovery(struct engine *engine)
 		if (space_foreach(memtx_build_secondary_keys, memtx) != 0)
 			return -1;
 	}
+	xdir_collect_inprogress(&memtx->snap_dir);
 	return 0;
 }
 
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 552d42b..c82b7cd 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -3481,6 +3481,9 @@ vy_backup_cb(const struct vy_log_record *record, void *cb_arg)
 
 	char path[PATH_MAX];
 	for (int type = 0; type < vy_file_MAX; type++) {
+		if (type == VY_FILE_RUN_INPROGRESS ||
+			type == VY_FILE_INDEX_INPROGRESS)
+			continue;
 		vy_run_snprint_path(path, sizeof(path), arg->env->path,
 				    arg->space_id, arg->index_id,
 				    record->run_id, type);
diff --git a/src/box/vy_log.c b/src/box/vy_log.c
index 51e3753..7a7993d 100644
--- a/src/box/vy_log.c
+++ b/src/box/vy_log.c
@@ -852,6 +852,10 @@ vy_log_open(struct xlog *xlog)
 	    xlog_write_row(xlog, &row) < 0)
 		goto fail_close_xlog;
 
+	ERROR_INJECT(ERRINJ_VY_LOG_FILE_RENAME, {
+		diag_set(ClientError, ER_INJECTION, "vinyl log file rename");
+		goto fail_errinj;
+	});
 	if (xlog_rename(xlog) < 0)
 		goto fail_close_xlog;
 
@@ -860,6 +864,7 @@ vy_log_open(struct xlog *xlog)
 fail_close_xlog:
 	if (unlink(xlog->filename) < 0)
 		say_syserror("failed to delete file '%s'", xlog->filename);
+fail_errinj:
 	xlog_close(xlog, false);
 fail:
 	return -1;
@@ -976,7 +981,7 @@ vy_log_end_recovery(void)
 			return -1;
 		}
 	}
-
+	xdir_collect_inprogress(&vy_log.dir);
 	vy_log.recovery = NULL;
 	return 0;
 }
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index 2c5c0c9..0626e28 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -66,8 +66,10 @@ enum { VY_BLOOM_VERSION = 0 };
 #define XLOG_META_TYPE_INDEX "INDEX"
 
 const char *vy_file_suffix[] = {
-	"index",	/* VY_FILE_INDEX */
-	"run",		/* VY_FILE_RUN */
+	"index",			/* VY_FILE_INDEX */
+	"index" inprogress_suffix, 	/* VY_FILE_INDEX_INPROGRESS */
+	"run",				/* VY_FILE_RUN */
+	"run" inprogress_suffix, 	/* VY_FILE_RUN_INPROGRESS */
 };
 
 /**
@@ -2021,6 +2023,11 @@ vy_run_write_index(struct vy_run *run, const char *dirpath,
 			goto fail;
 	}
 
+	ERROR_INJECT(ERRINJ_VY_INDEX_FILE_RENAME, {
+		diag_set(ClientError, ER_INJECTION,
+			 "vinyl index file rename");
+		goto fail_errinj;
+	});
 	if (xlog_tx_commit(&index_xlog) < 0 ||
 	    xlog_flush(&index_xlog) < 0 ||
 	    xlog_rename(&index_xlog) < 0)
@@ -2029,10 +2036,11 @@ vy_run_write_index(struct vy_run *run, const char *dirpath,
 	region_truncate(region, mem_used);
 	return 0;
 fail:
+	unlink(path);
+fail_errinj:
 	region_truncate(region, mem_used);
 	xlog_tx_rollback(&index_xlog);
 	xlog_close(&index_xlog, false);
-	unlink(path);
 	return -1;
 }
 
@@ -2263,6 +2271,11 @@ vy_run_writer_commit(struct vy_run_writer *writer)
 	run->info.max_key = vy_key_dup(key);
 	if (run->info.max_key == NULL)
 		goto out;
+	ERROR_INJECT(ERRINJ_VY_RUN_FILE_RENAME, {
+		diag_set(ClientError, ER_INJECTION,
+			 "vinyl run file rename");
+		goto out;
+	});
 
 	/* Sync data and link the file to the final name. */
 	if (xlog_sync(&writer->data_xlog) < 0 ||
diff --git a/src/box/vy_run.h b/src/box/vy_run.h
index b3e3d1c..6afef2a 100644
--- a/src/box/vy_run.h
+++ b/src/box/vy_run.h
@@ -378,7 +378,9 @@ vy_run_rebuild_index(struct vy_run *run, const char *dir,
 
 enum vy_file_type {
 	VY_FILE_INDEX,
+	VY_FILE_INDEX_INPROGRESS,
 	VY_FILE_RUN,
+	VY_FILE_RUN_INPROGRESS,
 	vy_file_MAX,
 };
 
diff --git a/src/box/xlog.c b/src/box/xlog.c
index 04f8587..142cdcf 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -58,7 +58,6 @@ typedef uint32_t log_magic_t;
 static const log_magic_t row_marker = mp_bswap_u32(0xd5ba0bab); /* host byte order */
 static const log_magic_t zrow_marker = mp_bswap_u32(0xd5ba0bba); /* host byte order */
 static const log_magic_t eof_marker = mp_bswap_u32(0xd510aded); /* host byte order */
-static const char inprogress_suffix[] = ".inprogress";
 
 enum {
 	/**
@@ -625,6 +624,34 @@ xdir_collect_garbage(struct xdir *dir, int64_t signature, bool use_coio)
 	return 0;
 }
 
+void
+xdir_collect_inprogress(struct xdir *xdir)
+{
+	const char *dirname = xdir->dirname;
+	DIR *dh = opendir(dirname);
+	if (dh == NULL) {
+		if (errno != ENOENT)
+			say_syserror("error reading directory '%s'", dirname);
+		return;
+	}
+
+	struct dirent *dent;
+	char path[PATH_MAX];
+	int total = snprintf(path, sizeof(path), "%s/", dirname);
+
+	while ((dent = readdir(dh)) != NULL) {
+		char *ext = strrchr(dent->d_name, '.');
+		if (ext == NULL || strcmp(ext, inprogress_suffix) != 0)
+			continue;
+		strncpy(path + total, dent->d_name, sizeof(path) - total);
+		if (unlink(path) < 0) {
+			say_syserror("error while removing %s", path);
+			return;
+		} else
+			say_info("removed %s", path);
+	}
+}
+
 /* }}} */
 
 
diff --git a/src/box/xlog.h b/src/box/xlog.h
index 973910d..3b03588 100644
--- a/src/box/xlog.h
+++ b/src/box/xlog.h
@@ -70,6 +70,11 @@ enum xdir_type {
 enum log_suffix { NONE, INPROGRESS };
 
 /**
+ * Suffix added to path of inprogress files.
+ */
+#define inprogress_suffix ".inprogress"
+
+/**
  * A handle for a data directory with write ahead logs, snapshots,
  * vylogs.
  * Can be used to find the last log in the directory, scan
@@ -181,6 +186,12 @@ int
 xdir_collect_garbage(struct xdir *dir, int64_t signature, bool use_coio);
 
 /**
+ * Remove .inprogress files in specified directory.
+ */
+void
+xdir_collect_inprogress(struct xdir *xdir);
+
+/**
  * Return LSN and vclock (unless @vclock is NULL) of the newest
  * file in a directory or -1 if the directory is empty.
  */
diff --git a/src/errinj.h b/src/errinj.h
index 78514ac..7141af9 100644
--- a/src/errinj.h
+++ b/src/errinj.h
@@ -111,6 +111,9 @@ struct errinj {
 	_(ERRINJ_IPROTO_TX_DELAY, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_LOG_ROTATE, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_SNAP_COMMIT_DELAY, ERRINJ_BOOL, {.bparam = 0}) \
+	_(ERRINJ_VY_LOG_FILE_RENAME, ERRINJ_BOOL, {.bparam = false}) \
+	_(ERRINJ_VY_INDEX_FILE_RENAME, ERRINJ_BOOL, {.bparam = false}) \
+	_(ERRINJ_VY_RUN_FILE_RENAME, ERRINJ_BOOL, {.bparam = false}) \
 
 ENUM0(errinj_id, ERRINJ_LIST);
 extern struct errinj errinjs[];
diff --git a/test/box/errinj.result b/test/box/errinj.result
index 6ced172..3147cfb 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -40,8 +40,12 @@ errinj.info()
     state: false
   ERRINJ_WAL_IO:
     state: false
+  ERRINJ_VY_INDEX_FILE_RENAME:
+    state: false
   ERRINJ_TUPLE_ALLOC:
     state: false
+  ERRINJ_VY_RUN_FILE_RENAME:
+    state: false
   ERRINJ_VY_READ_PAGE:
     state: false
   ERRINJ_RELAY_REPORT_INTERVAL:
@@ -56,6 +60,8 @@ errinj.info()
     state: false
   ERRINJ_VY_LOG_FLUSH_DELAY:
     state: false
+  ERRINJ_VY_LOG_FILE_RENAME:
+    state: false
   ERRINJ_SNAP_COMMIT_DELAY:
     state: false
   ERRINJ_RELAY_FINAL_SLEEP:
@@ -693,6 +699,185 @@ errinj.set("ERRINJ_WAL_WRITE", false)
 space:drop()
 ---
 ...
+test_run:cmd("restart server default with cleanup=1")
+errinj = box.error.injection
+---
+...
+fio = require("fio")
+---
+...
+fiber = require('fiber')
+---
+...
+errinj.set("ERRINJ_VY_LOG_FILE_RENAME", true)
+---
+- ok
+...
+s = box.schema.space.create("vinyl_test", {engine='vinyl'})
+---
+...
+_ = s:create_index("primary")
+---
+...
+vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir)
+---
+...
+while #fio.glob(vinyl_vylog) == 0 do fiber.sleep(0.001) end
+---
+...
+#fio.glob(vinyl_vylog) > 0 -- true
+---
+- true
+...
+space = box.schema.space.create("memtx_test")
+---
+...
+_ = space:create_index("primary")
+---
+...
+space:insert{1}
+---
+- [1]
+...
+errinj.set("ERRINJ_SNAP_COMMIT_DELAY", true)
+---
+- ok
+...
+_ = fiber.create(function() box.snapshot() end)
+---
+...
+memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir)
+---
+...
+while #fio.glob(memtx_snap) == 0 do fiber.sleep(0.001) end
+---
+...
+#fio.glob(memtx_snap) > 0 --true
+---
+- true
+...
+test_run:cmd("restart server default")
+errinj = box.error.injection
+---
+...
+fio = require("fio")
+---
+...
+fiber = require('fiber')
+---
+...
+vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir)
+---
+...
+#fio.glob(vinyl_vylog) == 0 -- true
+---
+- true
+...
+memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir)
+---
+...
+#fio.glob(memtx_snap) == 0 -- true
+---
+- true
+...
+s = box.space.vinyl_test
+---
+...
+vinyl_index = string.format("%s/%s/*/*.index.inprogress", box.cfg.vinyl_dir, s.id)
+---
+...
+errinj.set("ERRINJ_VY_INDEX_FILE_RENAME", true)
+---
+- ok
+...
+for i = 1, 10 do s:insert{i} end
+---
+...
+box.snapshot()
+---
+- error: Error injection 'vinyl index file rename'
+...
+while #fio.glob(vinyl_index) == 0 do fiber.sleep(0.001) end
+---
+...
+vinyl_run = string.format("%s/%s/*/*.run.inprogress", box.cfg.vinyl_dir, s.id)
+---
+...
+errinj.set("ERRINJ_VY_RUN_FILE_RENAME", true)
+---
+- ok
+...
+while #fio.glob(vinyl_run) == 0  do fiber.sleep(0.001) end
+---
+...
+#fio.glob(vinyl_run) > 0 -- true
+---
+- true
+...
+#fio.glob(vinyl_index) > 0 -- true
+---
+- true
+...
+test_run:cmd("restart server default")
+errinj = box.error.injection
+---
+...
+net_box = require('net.box')
+---
+...
+fio = require("fio")
+---
+...
+fiber = require('fiber')
+---
+...
+s = box.space.vinyl_test
+---
+...
+memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir)
+---
+...
+vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir)
+---
+...
+vinyl_run = string.format("%s/%s/*/*.run.inprogress", box.cfg.vinyl_dir, s.id)
+---
+...
+vinyl_index = string.format("%s/%s/*/*.index.inprogress", box.cfg.vinyl_dir, s.id)
+---
+...
+-- no inprogress files should be present in memtx(vinyl)_dir.
+#fio.glob(memtx_snap) == 0 -- true
+---
+- true
+...
+#fio.glob(vinyl_vylog) == 0 -- true
+---
+- true
+...
+#fio.glob(vinyl_index) == 0 -- true
+---
+- true
+...
+#fio.glob(vinyl_run) == 0 -- true
+---
+- true
+...
+box.space.vinyl_test:drop()
+---
+...
+run_dir = fio.pathjoin(box.cfg.vinyl_dir,"*/*/*")
+---
+...
+for _, file in ipairs(fio.glob(run_dir)) do fio.unlink(file) end
+---
+...
+vinyl_dir = fio.pathjoin(box.cfg.vinyl_dir,"*/")
+---
+...
+for _, dir in ipairs(fio.glob(vinyl_dir)) do fio.rmtree(dir) end
+---
+...
 --test space:bsize() in case of memory error
 utils = dofile('utils.lua')
 ---
diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
index 3af1b74..66da1cf 100644
--- a/test/box/errinj.test.lua
+++ b/test/box/errinj.test.lua
@@ -197,12 +197,82 @@ _ = space:insert{1, require'digest'.urandom(192 * 1024)}
 errinj.set("ERRINJ_WAL_WRITE_DISK", false)
 
 _ = space:insert{1}
-
 errinj.set("ERRINJ_WAL_WRITE", true)
 box.snapshot()
 errinj.set("ERRINJ_WAL_WRITE", false)
 space:drop()
 
+test_run:cmd("restart server default with cleanup=1")
+errinj = box.error.injection
+fio = require("fio")
+fiber = require('fiber')
+
+errinj.set("ERRINJ_VY_LOG_FILE_RENAME", true)
+s = box.schema.space.create("vinyl_test", {engine='vinyl'})
+_ = s:create_index("primary")
+vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir)
+while #fio.glob(vinyl_vylog) == 0 do fiber.sleep(0.001) end
+#fio.glob(vinyl_vylog) > 0 -- true
+
+space = box.schema.space.create("memtx_test")
+_ = space:create_index("primary")
+space:insert{1}
+
+errinj.set("ERRINJ_SNAP_COMMIT_DELAY", true)
+_ = fiber.create(function() box.snapshot() end)
+memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir)
+while #fio.glob(memtx_snap) == 0 do fiber.sleep(0.001) end
+#fio.glob(memtx_snap) > 0 --true
+
+test_run:cmd("restart server default")
+errinj = box.error.injection
+fio = require("fio")
+fiber = require('fiber')
+
+vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir)
+#fio.glob(vinyl_vylog) == 0 -- true
+
+memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir)
+#fio.glob(memtx_snap) == 0 -- true
+
+s = box.space.vinyl_test
+vinyl_index = string.format("%s/%s/*/*.index.inprogress", box.cfg.vinyl_dir, s.id)
+errinj.set("ERRINJ_VY_INDEX_FILE_RENAME", true)
+for i = 1, 10 do s:insert{i} end
+box.snapshot()
+
+while #fio.glob(vinyl_index) == 0 do fiber.sleep(0.001) end
+
+vinyl_run = string.format("%s/%s/*/*.run.inprogress", box.cfg.vinyl_dir, s.id)
+errinj.set("ERRINJ_VY_RUN_FILE_RENAME", true)
+while #fio.glob(vinyl_run) == 0  do fiber.sleep(0.001) end
+
+#fio.glob(vinyl_run) > 0 -- true
+#fio.glob(vinyl_index) > 0 -- true
+
+test_run:cmd("restart server default")
+errinj = box.error.injection
+net_box = require('net.box')
+fio = require("fio")
+fiber = require('fiber')
+
+s = box.space.vinyl_test
+memtx_snap = string.format("%s/*.snap.inprogress", box.cfg.memtx_dir)
+vinyl_vylog = string.format("%s/*.vylog.inprogress", box.cfg.vinyl_dir)
+vinyl_run = string.format("%s/%s/*/*.run.inprogress", box.cfg.vinyl_dir, s.id)
+vinyl_index = string.format("%s/%s/*/*.index.inprogress", box.cfg.vinyl_dir, s.id)
+
+-- no inprogress files should be present in memtx(vinyl)_dir.
+#fio.glob(memtx_snap) == 0 -- true
+#fio.glob(vinyl_vylog) == 0 -- true
+#fio.glob(vinyl_index) == 0 -- true
+#fio.glob(vinyl_run) == 0 -- true
+
+box.space.vinyl_test:drop()
+run_dir = fio.pathjoin(box.cfg.vinyl_dir,"*/*/*")
+for _, file in ipairs(fio.glob(run_dir)) do fio.unlink(file) end
+vinyl_dir = fio.pathjoin(box.cfg.vinyl_dir,"*/")
+for _, dir in ipairs(fio.glob(vinyl_dir)) do fio.rmtree(dir) end
 --test space:bsize() in case of memory error
 utils = dofile('utils.lua')
 s = box.schema.space.create('space_bsize')
-- 
2.7.4