[PATCH v2 0/6] Create empty xlog on shutdown

[PATCH v2 0/6] Create empty xlog on shutdown

[Vladimir Davydov]

In order to determine if the instance needs to be rebootstrapped,
we need to know its vclock and we need to know it before we start
recovery (see #461). We are planning to retrieve the vclock by
rescanning the last xlog. In order not to degrade recovery time
in case rebootstrap is not needed, let's create a new empty xlog
on shutdown.

Patches 1-4 of the series improve the LSN gap check so that it
doesn't trigger in case an xlog has a gap at the end due to write
errors. They are needed to make the tests pass after patch 5 is
applied.

Patch 5 makes WAL thread create an empty xlog on shutdown and
reopen it on restart.

Patch 6 is optional: it moves XlogGapError to box/error.h, where
other exceptions are defined.

https://github.com/tarantool/tarantool/issues/461
https://github.com/tarantool/tarantool/commits/wal-make-new-xlog-on-shutdown

Changes in v2:
 - Drop the patch that makes WAL rollback LSN on write failure.
   Instead add PrevVclock to xlog header to improve gap check,
   as suggested by Kostja.
 - Improve comments and refactor the code as per review.

v1: https://www.freelists.org/post/tarantool-patches/PATCH-03-Speed-up-recovery-in-case-rebootstrap-is-not-needed

Vladimir Davydov (6):
  xlog: store prev vclock in xlog header
  xlog: differentiate between closed and never opened cursor
  recovery: make LSN gap check more thorough
  recovery: promote recovery clock even if the WAL is empty
  wal: create empty xlog on shutdown
  error: move XlogGapError to box/error.h

 src/box/CMakeLists.txt                |   2 +-
 src/box/error.cc                      |  36 ++++++++--
 src/box/error.h                       |  16 +++++
 src/box/recovery.cc                   | 118 +++++++++++++++-----------------
 src/box/wal.c                         |  94 ++++++++++++++++++++++++-
 src/box/xlog.c                        |  70 ++++++++++++++-----
 src/box/xlog.h                        |  17 ++++-
 src/diag.h                            |   2 -
 test/replication/gc.result            |  23 +------
 test/replication/gc.test.lua          |   8 +--
 test/replication/hot_standby.result   |  12 ++--
 test/replication/hot_standby.test.lua |   4 +-
 test/xlog-py/dup_key.result           |  20 ++----
 test/xlog-py/dup_key.test.py          |  29 +++-----
 test/xlog/errinj.result               |   1 +
 test/xlog/header.result               |  41 +++++++++++
 test/xlog/header.test.lua             |  13 ++++
 test/xlog/panic_on_lsn_gap.result     | 125 ++++++++++++++++++++++++++++------
 test/xlog/panic_on_lsn_gap.test.lua   |  33 +++++++--
 test/xlog/panic_on_wal_error.result   |  23 +------
 test/xlog/panic_on_wal_error.test.lua |   9 +--
 21 files changed, 480 insertions(+), 216 deletions(-)

-- 
2.11.0

[PATCH v2 1/6] xlog: store prev vclock in xlog header

[Vladimir Davydov]

This patch adds a new key to xlog header, PrevVclock, which contains the
vclock of the previous xlog file in the xlog directory. It is set by
xdir_create_xlog() to the last vclock in xdir::index. The new key is
only present in XLOG files (it doesn't make sense for SNAP or VYLOG
anyway). It will be used to make the check for xlog gaps more thorough.
---
 src/box/xlog.c            | 70 ++++++++++++++++++++++++++++++++++++-----------
 src/box/xlog.h            |  8 ++++++
 test/xlog/header.result   | 41 +++++++++++++++++++++++++++
 test/xlog/header.test.lua | 13 +++++++++
 4 files changed, 116 insertions(+), 16 deletions(-)

diff --git a/src/box/xlog.c b/src/box/xlog.c
index 142262b4..db35f474 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -94,6 +94,7 @@ enum {
 #define INSTANCE_UUID_KEY_V12 "Server"
 #define VCLOCK_KEY "VClock"
 #define VERSION_KEY "Version"
+#define PREV_VCLOCK_KEY "PrevVClock"
 
 static const char v13[] = "0.13";
 static const char v12[] = "0.12";
@@ -117,19 +118,50 @@ xlog_meta_format(const struct xlog_meta *meta, char *buf, int size)
 	if (vstr == NULL)
 		return -1;
 	char *instance_uuid = tt_uuid_str(&meta->instance_uuid);
-	int total = snprintf(buf, size,
+	int total = 0;
+	SNPRINT(total, snprintf, buf, size,
 		"%s\n"
 		"%s\n"
 		VERSION_KEY ": %s\n"
 		INSTANCE_UUID_KEY ": %s\n"
-		VCLOCK_KEY ": %s\n\n",
+		VCLOCK_KEY ": %s\n",
 		meta->filetype, v13, PACKAGE_VERSION, instance_uuid, vstr);
-	assert(total > 0);
 	free(vstr);
+	if (meta->has_prev_vclock) {
+		vstr = vclock_to_string(&meta->prev_vclock);
+		SNPRINT(total, snprintf, buf, size,
+			PREV_VCLOCK_KEY ": %s\n", vstr);
+		free(vstr);
+	}
+	SNPRINT(total, snprintf, buf, size, "\n");
+	assert(total > 0);
 	return total;
 }
 
 /**
+ * Parse vclock from xlog meta.
+ */
+static int
+parse_vclock(const char *val, const char *val_end, struct vclock *vclock)
+{
+	if (val_end - val > VCLOCK_STR_LEN_MAX) {
+		diag_set(XlogError, "can't parse vclock");
+		return -1;
+	}
+	char str[VCLOCK_STR_LEN_MAX + 1];
+	memcpy(str, val, val_end - val);
+	str[val_end - val] = '\0';
+	size_t off = vclock_from_string(vclock, str);
+	ERROR_INJECT(ERRINJ_XLOG_META, { off = 1; });
+	if (off != 0) {
+		diag_set(XlogError, "invalid vclock at "
+			 "offset %zd", off);
+		return -1;
+	}
+	return 0;
+}
+
+/**
  * Parse xlog meta from buffer, update buffer read
  * position in case of success
  *
@@ -224,21 +256,15 @@ xlog_meta_parse(struct xlog_meta *meta, const char **data,
 			/*
 			 * VClock: <vclock>
 			 */
-			if (val_end - val > VCLOCK_STR_LEN_MAX) {
-				diag_set(XlogError, "can't parse vclock");
+			if (parse_vclock(val, val_end, &meta->vclock) != 0)
 				return -1;
-			}
-			char vclock[VCLOCK_STR_LEN_MAX + 1];
-			memcpy(vclock, val, val_end - val);
-			vclock[val_end - val] = '\0';
-			size_t off = vclock_from_string(&meta->vclock, vclock);
-			ERROR_INJECT(ERRINJ_XLOG_META, {
-				off = 1;});
-			if (off != 0) {
-				diag_set(XlogError, "invalid vclock at "
-					  "offset %zd", off);
+		} else if (memcmp(key, PREV_VCLOCK_KEY, key_end - key) == 0) {
+			/*
+			 * PrevVClock: <vclock>
+			 */
+			if (parse_vclock(val, val_end, &meta->prev_vclock) != 0)
 				return -1;
-			}
+			meta->has_prev_vclock = true;
 		} else if (memcmp(key, VERSION_KEY, key_end - key) == 0) {
 			/* Ignore Version: for now */
 		} else {
@@ -868,6 +894,18 @@ xdir_create_xlog(struct xdir *dir, struct xlog *xlog,
 	meta.instance_uuid = *dir->instance_uuid;
 	vclock_copy(&meta.vclock, vclock);
 
+	/*
+	 * For WAL dir: store vclock of the previous xlog file
+	 * to check for gaps on recovery.
+	 */
+	if (dir->type == XLOG && !vclockset_empty(&dir->index)) {
+		vclock_copy(&meta.prev_vclock, vclockset_last(&dir->index));
+		meta.has_prev_vclock = true;
+	} else {
+		vclock_create(&meta.prev_vclock);
+		meta.has_prev_vclock = false;
+	}
+
 	if (xlog_create(xlog, filename, dir->open_wflags, &meta) != 0)
 		return -1;
 
diff --git a/src/box/xlog.h b/src/box/xlog.h
index 4b9a5765..bff3275f 100644
--- a/src/box/xlog.h
+++ b/src/box/xlog.h
@@ -229,6 +229,14 @@ struct xlog_meta {
 	 * is vector clock *at the time the snapshot is taken*.
 	 */
 	struct vclock vclock;
+	/**
+	 * Text file header: vector clock of the previous
+	 * file at the directory. Used for checking the
+	 * directory for missing WALs.
+	 */
+	struct vclock prev_vclock;
+	/** Set if @prev_vclock is present. */
+	bool has_prev_vclock;
 };
 
 /* }}} */
diff --git a/test/xlog/header.result b/test/xlog/header.result
index 4b8431e8..6520e469 100644
--- a/test/xlog/header.result
+++ b/test/xlog/header.result
@@ -103,6 +103,47 @@ dump_header(fio.pathjoin(box.cfg.wal_dir, xlog_name))
   - 'Version: <version>'
   - 'Instance: <instance_uuid>'
   - 'VClock: {1: 2}'
+  - 'PrevVClock: {}'
+...
+box.space._schema:delete({"layout_test"})
+---
+- ['layout_test']
+...
+box.snapshot()
+---
+- ok
+...
+checkpoint_lsn = box.info.lsn
+---
+...
+-- SNAP files
+snap_name = string.format("%020d.snap", checkpoint_lsn)
+---
+...
+dump_header(fio.pathjoin(box.cfg.memtx_dir, snap_name))
+---
+- - SNAP
+  - '0.13'
+  - 'Version: <version>'
+  - 'Instance: <instance_uuid>'
+  - 'VClock: {1: 4}'
+...
+-- XLOG files
+box.space._schema:insert({"layout_test"})
+---
+- ['layout_test']
+...
+xlog_name = string.format("%020d.xlog", checkpoint_lsn)
+---
+...
+dump_header(fio.pathjoin(box.cfg.wal_dir, xlog_name))
+---
+- - XLOG
+  - '0.13'
+  - 'Version: <version>'
+  - 'Instance: <instance_uuid>'
+  - 'VClock: {1: 4}'
+  - 'PrevVClock: {1: 2}'
 ...
 box.space._schema:delete({"layout_test"})
 ---
diff --git a/test/xlog/header.test.lua b/test/xlog/header.test.lua
index 1e7ec794..686d2bed 100644
--- a/test/xlog/header.test.lua
+++ b/test/xlog/header.test.lua
@@ -44,4 +44,17 @@ xlog_name = string.format("%020d.xlog", checkpoint_lsn)
 dump_header(fio.pathjoin(box.cfg.wal_dir, xlog_name))
 box.space._schema:delete({"layout_test"})
 
+box.snapshot()
+checkpoint_lsn = box.info.lsn
+
+-- SNAP files
+snap_name = string.format("%020d.snap", checkpoint_lsn)
+dump_header(fio.pathjoin(box.cfg.memtx_dir, snap_name))
+
+-- XLOG files
+box.space._schema:insert({"layout_test"})
+xlog_name = string.format("%020d.xlog", checkpoint_lsn)
+dump_header(fio.pathjoin(box.cfg.wal_dir, xlog_name))
+box.space._schema:delete({"layout_test"})
+
 test_run:cmd("clear filter")
-- 
2.11.0

[PATCH v2 2/6] xlog: differentiate between closed and never opened cursor

[Vladimir Davydov]

Currently, a cursor that has never been opened and a cursor that was
properly closed share the same state, XLOG_CURSOR_CLOSED. Let's add a
new state, XLOG_CURSOR_UNINITIALIZED, so that we can differentiate
between those two. This new state will be used by the next patch.
---
 src/box/xlog.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/box/xlog.h b/src/box/xlog.h
index bff3275f..9b5c8555 100644
--- a/src/box/xlog.h
+++ b/src/box/xlog.h
@@ -545,8 +545,8 @@ xlog_tx_decode(const char *data, const char *data_end,
 /* {{{ xlog_cursor - read rows from a log file */
 
 enum xlog_cursor_state {
-	/* Cursor is closed */
-	XLOG_CURSOR_CLOSED = 0,
+	/* The cursor was never opened. */
+	XLOG_CURSOR_UNINITIALIZED = 0,
 	/* The cursor is open but no tx is read */
 	XLOG_CURSOR_ACTIVE = 1,
 	/* The Cursor is open and a tx is read */
@@ -555,6 +555,8 @@ enum xlog_cursor_state {
 	XLOG_CURSOR_EOF = 3,
 	/* The cursor was closed after reaching EOF. */
 	XLOG_CURSOR_EOF_CLOSED = 4,
+	/* The cursor was closed before reaching EOF. */
+	XLOG_CURSOR_CLOSED = 5,
 };
 
 /**
@@ -586,7 +588,8 @@ struct xlog_cursor
 static inline bool
 xlog_cursor_is_open(const struct xlog_cursor *cursor)
 {
-	return (cursor->state != XLOG_CURSOR_CLOSED &&
+	return (cursor->state != XLOG_CURSOR_UNINITIALIZED &&
+		cursor->state != XLOG_CURSOR_CLOSED &&
 		cursor->state != XLOG_CURSOR_EOF_CLOSED);
 }
 
-- 
2.11.0

[PATCH v2 3/6] recovery: make LSN gap check more thorough

[Vladimir Davydov]

Currently, the lsn gap check is rather sloppy: when we open an xlog file
for recovery, we check that its vclock equals the vclock of the last
replayed row (see recover_remaining_wals), so if there were WAL write
errors at the end of an xlog file, we will report a false-positive gap
error (because wal doesn't rollback lsn counter). Let's use PrevVclock
xlog meta key introduced earlier to improve the check.
---
 src/box/recovery.cc                 | 62 +++++++++++++++++++++++++------------
 test/xlog/panic_on_lsn_gap.result   | 34 ++++++++++++++++++++
 test/xlog/panic_on_lsn_gap.test.lua | 13 ++++++++
 3 files changed, 90 insertions(+), 19 deletions(-)

diff --git a/src/box/recovery.cc b/src/box/recovery.cc
index 71f6bd8c..8ac89cc2 100644
--- a/src/box/recovery.cc
+++ b/src/box/recovery.cc
@@ -152,6 +152,48 @@ recovery_close_log(struct recovery *r)
 	trigger_run_xc(&r->on_close_log, NULL);
 }
 
+static void
+recovery_open_log(struct recovery *r, const struct vclock *vclock)
+{
+	XlogGapError *e;
+	struct xlog_meta meta = r->cursor.meta;
+	enum xlog_cursor_state state = r->cursor.state;
+
+	recovery_close_log(r);
+
+	xdir_open_cursor_xc(&r->wal_dir, vclock_sum(vclock), &r->cursor);
+
+	if (state == XLOG_CURSOR_UNINITIALIZED &&
+	    vclock_compare(vclock, &r->vclock) > 0) {
+		/*
+		 * This is the first WAL we are about to scan
+		 * and the best clock we could find is greater
+		 * or is incomparable with the initial recovery
+		 * position.
+		 */
+		goto gap_error;
+	}
+
+	if (state != XLOG_CURSOR_UNINITIALIZED &&
+	    r->cursor.meta.has_prev_vclock &&
+	    vclock_compare(&r->cursor.meta.prev_vclock, &meta.vclock) != 0) {
+		/*
+		 * WALs are missing between the last scanned WAL
+		 * and the next one.
+		 */
+		goto gap_error;
+	}
+	return;
+
+gap_error:
+	e = tnt_error(XlogGapError, &r->vclock, vclock);
+	if (!r->wal_dir.force_recovery)
+		throw e;
+	/* Ignore missing WALs if force_recovery is set. */
+	e->log();
+	say_warn("ignoring a gap in LSN");
+}
+
 void
 recovery_delete(struct recovery *r)
 {
@@ -277,25 +319,7 @@ recover_remaining_wals(struct recovery *r, struct xstream *stream,
 			continue;
 		}
 
-		if (vclock_compare(clock, &r->vclock) > 0) {
-			/**
-			 * The best clock we could find is
-			 * greater or is incomparable with the
-			 * current state of recovery.
-			 */
-			XlogGapError *e =
-				tnt_error(XlogGapError, &r->vclock, clock);
-
-			if (!r->wal_dir.force_recovery)
-				throw e;
-			e->log();
-			/* Ignore missing WALs */
-			say_warn("ignoring a gap in LSN");
-		}
-
-		recovery_close_log(r);
-
-		xdir_open_cursor_xc(&r->wal_dir, vclock_sum(clock), &r->cursor);
+		recovery_open_log(r, clock);
 
 		say_info("recover from `%s'", r->cursor.name);
 
diff --git a/test/xlog/panic_on_lsn_gap.result b/test/xlog/panic_on_lsn_gap.result
index 313850a6..c93fcdd6 100644
--- a/test/xlog/panic_on_lsn_gap.result
+++ b/test/xlog/panic_on_lsn_gap.result
@@ -280,6 +280,40 @@ box.space._schema:select{'key'}
 ---
 - - ['key', 'test 4']
 ...
+--
+-- Check that if there's an LSN gap between two WALs
+-- that appeared due to a disk error and no files is
+-- actually missing, we won't panic on recovery.
+--
+box.space._schema:replace{'key', 'test 4'} -- creates new WAL
+---
+- ['key', 'test 4']
+...
+box.error.injection.set("ERRINJ_WAL_WRITE_DISK", true)
+---
+- ok
+...
+box.space._schema:replace{'key', 'test 5'} -- fails, makes gap
+---
+- error: Failed to write to disk
+...
+box.snapshot() -- fails, rotates WAL
+---
+- error: Error injection 'xlog write injection'
+...
+box.error.injection.set("ERRINJ_WAL_WRITE_DISK", false)
+---
+- ok
+...
+box.space._schema:replace{'key', 'test 5'} -- creates new WAL
+---
+- ['key', 'test 5']
+...
+test_run:cmd("restart server panic")
+box.space._schema:select{'key'}
+---
+- - ['key', 'test 5']
+...
 test_run:cmd('switch default')
 ---
 - true
diff --git a/test/xlog/panic_on_lsn_gap.test.lua b/test/xlog/panic_on_lsn_gap.test.lua
index 248a3e63..b1ede320 100644
--- a/test/xlog/panic_on_lsn_gap.test.lua
+++ b/test/xlog/panic_on_lsn_gap.test.lua
@@ -108,6 +108,19 @@ require('fio').glob(name .. "/*.xlog")
 -- restart is ok
 test_run:cmd("restart server panic")
 box.space._schema:select{'key'}
+--
+-- Check that if there's an LSN gap between two WALs
+-- that appeared due to a disk error and no files is
+-- actually missing, we won't panic on recovery.
+--
+box.space._schema:replace{'key', 'test 4'} -- creates new WAL
+box.error.injection.set("ERRINJ_WAL_WRITE_DISK", true)
+box.space._schema:replace{'key', 'test 5'} -- fails, makes gap
+box.snapshot() -- fails, rotates WAL
+box.error.injection.set("ERRINJ_WAL_WRITE_DISK", false)
+box.space._schema:replace{'key', 'test 5'} -- creates new WAL
+test_run:cmd("restart server panic")
+box.space._schema:select{'key'}
 test_run:cmd('switch default')
 test_run:cmd("stop server panic")
 test_run:cmd("cleanup server panic")
-- 
2.11.0

[PATCH v2 4/6] recovery: promote recovery clock even if the WAL is empty

[Vladimir Davydov]

Currently, if the last WAL in the directory happens to be corrupted or
empty so that we don't recover anything from it, recovery clock will be
that of the last record of the previous WAL. If the previous WAL happens
to have a gap at the end, the next WAL will be created between the last
WAL (empty one) and the next to last (with a gap at the end), breaking
the file order in the WAL directory. That said, we must promote recovery
clock even if we don't recover anything from a WAL.
---
 src/box/recovery.cc                 | 12 ++++++++++
 test/xlog/panic_on_lsn_gap.result   | 47 +++++++++++++++++++++++++++++++++++++
 test/xlog/panic_on_lsn_gap.test.lua | 10 ++++++++
 3 files changed, 69 insertions(+)

diff --git a/src/box/recovery.cc b/src/box/recovery.cc
index 8ac89cc2..70eb7d74 100644
--- a/src/box/recovery.cc
+++ b/src/box/recovery.cc
@@ -183,6 +183,17 @@ recovery_open_log(struct recovery *r, const struct vclock *vclock)
 		 */
 		goto gap_error;
 	}
+out:
+	/*
+	 * We must promote recovery clock even if we don't recover
+	 * anything from the next WAL. Otherwise if the last WAL
+	 * in the directory is corrupted or empty and the previous
+	 * one has an LSN gap at the end (due to a write error),
+	 * we will create the next WAL between two existing ones,
+	 * thus breaking the file order.
+	 */
+	if (vclock_compare(&r->vclock, vclock) < 0)
+		vclock_copy(&r->vclock, vclock);
 	return;
 
 gap_error:
@@ -192,6 +203,7 @@ gap_error:
 	/* Ignore missing WALs if force_recovery is set. */
 	e->log();
 	say_warn("ignoring a gap in LSN");
+	goto out;
 }
 
 void
diff --git a/test/xlog/panic_on_lsn_gap.result b/test/xlog/panic_on_lsn_gap.result
index c93fcdd6..d5064ce6 100644
--- a/test/xlog/panic_on_lsn_gap.result
+++ b/test/xlog/panic_on_lsn_gap.result
@@ -309,11 +309,58 @@ box.space._schema:replace{'key', 'test 5'} -- creates new WAL
 ---
 - ['key', 'test 5']
 ...
+box.error.injection.set("ERRINJ_WAL_WRITE_DISK", true)
+---
+- ok
+...
+box.space._schema:replace{'key', 'test 6'} -- fails, makes gap
+---
+- error: Failed to write to disk
+...
+box.snapshot() -- fails, rotates WAL
+---
+- error: Error injection 'xlog write injection'
+...
+box.space._schema:replace{'key', 'test 6'} -- fails, creates empty WAL
+---
+- error: Failed to write to disk
+...
+name = string.match(arg[0], "([^,]+)%.lua")
+---
+...
+require('fio').glob(name .. "/*.xlog")
+---
+- - panic/00000000000000000000.xlog
+  - panic/00000000000000000001.xlog
+  - panic/00000000000000000012.xlog
+  - panic/00000000000000000022.xlog
+  - panic/00000000000000000025.xlog
+  - panic/00000000000000000027.xlog
+  - panic/00000000000000000029.xlog
+...
 test_run:cmd("restart server panic")
 box.space._schema:select{'key'}
 ---
 - - ['key', 'test 5']
 ...
+-- Check that we don't create a WAL in the gap between the last two.
+box.space._schema:replace{'key', 'test 6'}
+---
+- ['key', 'test 6']
+...
+name = string.match(arg[0], "([^,]+)%.lua")
+---
+...
+require('fio').glob(name .. "/*.xlog")
+---
+- - panic/00000000000000000000.xlog
+  - panic/00000000000000000001.xlog
+  - panic/00000000000000000012.xlog
+  - panic/00000000000000000022.xlog
+  - panic/00000000000000000025.xlog
+  - panic/00000000000000000027.xlog
+  - panic/00000000000000000029.xlog
+...
 test_run:cmd('switch default')
 ---
 - true
diff --git a/test/xlog/panic_on_lsn_gap.test.lua b/test/xlog/panic_on_lsn_gap.test.lua
index b1ede320..d72552d0 100644
--- a/test/xlog/panic_on_lsn_gap.test.lua
+++ b/test/xlog/panic_on_lsn_gap.test.lua
@@ -119,8 +119,18 @@ box.space._schema:replace{'key', 'test 5'} -- fails, makes gap
 box.snapshot() -- fails, rotates WAL
 box.error.injection.set("ERRINJ_WAL_WRITE_DISK", false)
 box.space._schema:replace{'key', 'test 5'} -- creates new WAL
+box.error.injection.set("ERRINJ_WAL_WRITE_DISK", true)
+box.space._schema:replace{'key', 'test 6'} -- fails, makes gap
+box.snapshot() -- fails, rotates WAL
+box.space._schema:replace{'key', 'test 6'} -- fails, creates empty WAL
+name = string.match(arg[0], "([^,]+)%.lua")
+require('fio').glob(name .. "/*.xlog")
 test_run:cmd("restart server panic")
 box.space._schema:select{'key'}
+-- Check that we don't create a WAL in the gap between the last two.
+box.space._schema:replace{'key', 'test 6'}
+name = string.match(arg[0], "([^,]+)%.lua")
+require('fio').glob(name .. "/*.xlog")
 test_run:cmd('switch default')
 test_run:cmd("stop server panic")
 test_run:cmd("cleanup server panic")
-- 
2.11.0

[PATCH v2 5/6] wal: create empty xlog on shutdown

[Vladimir Davydov]

In order to determine whether we need to rebootstrap the instance on
startup, we need to know its vclock. To find it out, we are planning to
scan the last xlog file before proceeding to local recovery, but this
means in case rebootstrap is not required we will scan the last xlog
twice, which is sub-optimal. To speed up this procedure, let's create a
new empty xlog before shutting down the server and reopen it after
restart.

Needed for #461
---
 src/box/recovery.cc                   | 23 ---------
 src/box/wal.c                         | 94 ++++++++++++++++++++++++++++++++++-
 test/replication/gc.result            | 23 +--------
 test/replication/gc.test.lua          |  8 +--
 test/replication/hot_standby.result   | 12 ++---
 test/replication/hot_standby.test.lua |  4 +-
 test/xlog-py/dup_key.result           | 20 ++------
 test/xlog-py/dup_key.test.py          | 29 ++++-------
 test/xlog/errinj.result               |  1 +
 test/xlog/panic_on_lsn_gap.result     | 64 ++++++++++++------------
 test/xlog/panic_on_lsn_gap.test.lua   | 10 ++--
 test/xlog/panic_on_wal_error.result   | 23 +--------
 test/xlog/panic_on_wal_error.test.lua |  9 +---
 13 files changed, 160 insertions(+), 160 deletions(-)

diff --git a/src/box/recovery.cc b/src/box/recovery.cc
index 70eb7d74..722b86c5 100644
--- a/src/box/recovery.cc
+++ b/src/box/recovery.cc
@@ -352,29 +352,6 @@ void
 recovery_finalize(struct recovery *r)
 {
 	recovery_close_log(r);
-
-	/*
-	 * Check if next xlog exists. If it's true this xlog is
-	 * corrupted and we should rename it (to avoid getting
-	 * problem on the next xlog write with the same name).
-	 * Possible reasons are:
-	 *  - last xlog has corrupted rows
-	 *  - last xlog has corrupted header
-	 *  - last xlog has zero size
-	 */
-	char *name = xdir_format_filename(&r->wal_dir,
-					  vclock_sum(&r->vclock),
-					  NONE);
-	if (access(name, F_OK) == 0) {
-		say_info("rename corrupted xlog %s", name);
-		char to[PATH_MAX];
-		snprintf(to, sizeof(to), "%s.corrupted", name);
-		if (rename(name, to) != 0) {
-			tnt_raise(SystemError,
-				  "%s: can't rename corrupted xlog",
-				  name);
-		}
-	}
 }
 
 
diff --git a/src/box/wal.c b/src/box/wal.c
index f6b0fa66..19c9138e 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -34,6 +34,8 @@
 #include "fiber.h"
 #include "fio.h"
 #include "errinj.h"
+#include "error.h"
+#include "exception.h"
 
 #include "xlog.h"
 #include "xrow.h"
@@ -310,6 +312,75 @@ wal_thread_start()
 	cpipe_set_max_input(&wal_thread.wal_pipe, IOV_MAX);
 }
 
+static int
+wal_open_f(struct cbus_call_msg *msg)
+{
+	(void)msg;
+	struct wal_writer *writer = &wal_writer_singleton;
+	const char *path = xdir_format_filename(&writer->wal_dir,
+				vclock_sum(&writer->vclock), NONE);
+	assert(!xlog_is_open(&writer->current_wal));
+	return xlog_open(&writer->current_wal, path);
+}
+
+/**
+ * Try to open the current WAL file for appending if it exists.
+ */
+static int
+wal_open(struct wal_writer *writer)
+{
+	const char *path = xdir_format_filename(&writer->wal_dir,
+				vclock_sum(&writer->vclock), NONE);
+	if (access(path, F_OK) != 0) {
+		if (errno == ENOENT) {
+			/* No WAL, nothing to do. */
+			return 0;
+		}
+		diag_set(SystemError, "failed to access %s", path);
+		return -1;
+	}
+
+	/*
+	 * The WAL file exists, try to open it.
+	 *
+	 * Note, an xlog object cannot be opened and used in
+	 * different threads (because it uses slab arena), so
+	 * we have to call xlog_open() on behalf of the WAL
+	 * thread.
+	 */
+	struct cbus_call_msg msg;
+	if (cbus_call(&wal_thread.wal_pipe, &wal_thread.tx_pipe, &msg,
+		      wal_open_f, NULL, TIMEOUT_INFINITY) == 0) {
+		/*
+		 * Success: we can now append to
+		 * the existing WAL file.
+		 */
+		return 0;
+	}
+	struct error *e = diag_last_error(diag_get());
+	if (!type_assignable(&type_XlogError, e->type)) {
+		/*
+		 * Out of memory or system error.
+		 * Nothing we can do.
+		 */
+		return -1;
+	}
+	diag_log();
+
+	/*
+	 * Looks like the WAL file is corrupted.
+	 * Rename it so that we can proceed.
+	 */
+	say_warn("renaming corrupted %s", path);
+	char new_path[PATH_MAX];
+	snprintf(new_path, sizeof(new_path), "%s.corrupted", path);
+	if (rename(path, new_path) != 0) {
+		diag_set(SystemError, "failed to rename %s", path);
+		return -1;
+	}
+	return 0;
+}
+
 /**
  * Initialize WAL writer.
  *
@@ -332,6 +403,9 @@ wal_init(enum wal_mode wal_mode, const char *wal_dirname,
 	if (xdir_scan(&writer->wal_dir))
 		return -1;
 
+	if (wal_open(writer) != 0)
+		return -1;
+
 	journal_set(&writer->base);
 	return 0;
 }
@@ -382,8 +456,7 @@ wal_checkpoint_f(struct cmsg *data)
 
 		xlog_close(&writer->current_wal, false);
 		/*
-		 * Avoid creating an empty xlog if this is the
-		 * last snapshot before shutdown.
+		 * The next WAL will be created on the first write.
 		 */
 	}
 	vclock_copy(msg->vclock, &writer->vclock);
@@ -703,6 +776,23 @@ wal_thread_f(va_list ap)
 
 	struct wal_writer *writer = &wal_writer_singleton;
 
+	/*
+	 * Create a new empty WAL on shutdown so that we don't
+	 * have to rescan the last WAL to find the instance vclock.
+	 * Don't create a WAL if the last one is empty.
+	 */
+	if (writer->wal_mode != WAL_NONE &&
+	    (!xlog_is_open(&writer->current_wal) ||
+	     vclock_compare(&writer->vclock,
+			    &writer->current_wal.meta.vclock) > 0)) {
+		struct xlog l;
+		if (xdir_create_xlog(&writer->wal_dir, &l,
+				     &writer->vclock) == 0)
+			xlog_close(&l, false);
+		else
+			diag_log();
+	}
+
 	if (xlog_is_open(&writer->current_wal))
 		xlog_close(&writer->current_wal, false);
 
diff --git a/test/replication/gc.result b/test/replication/gc.result
index 7d6644ae..47d91e85 100644
--- a/test/replication/gc.result
+++ b/test/replication/gc.result
@@ -209,35 +209,16 @@ test_run:cmd("switch replica")
 ---
 - true
 ...
--- Unblock the replica and make it fail to apply a row.
-box.info.replication[1].upstream.message == nil
----
-- true
-...
-box.error.injection.set("ERRINJ_WAL_WRITE", true)
----
-- ok
-...
+-- Unblock the replica and break replication.
 box.error.injection.set("ERRINJ_WAL_DELAY", false)
 ---
 - ok
 ...
-while box.info.replication[1].upstream.message == nil do fiber.sleep(0.01) end
----
-...
-box.info.replication[1].upstream.message
+box.cfg{replication = {}}
 ---
-- Failed to write to disk
-...
-test_run:cmd("switch default")
----
-- true
 ...
 -- Restart the replica to reestablish replication.
 test_run:cmd("restart server replica")
----
-- true
-...
 -- Wait for the replica to catch up.
 test_run:cmd("switch replica")
 ---
diff --git a/test/replication/gc.test.lua b/test/replication/gc.test.lua
index 3a680075..c45b1152 100644
--- a/test/replication/gc.test.lua
+++ b/test/replication/gc.test.lua
@@ -107,13 +107,9 @@ fiber.sleep(0.1) -- wait for master to relay data
 -- needed by the replica.
 #box.info.gc().checkpoints == 2 or box.info.gc()
 test_run:cmd("switch replica")
--- Unblock the replica and make it fail to apply a row.
-box.info.replication[1].upstream.message == nil
-box.error.injection.set("ERRINJ_WAL_WRITE", true)
+-- Unblock the replica and break replication.
 box.error.injection.set("ERRINJ_WAL_DELAY", false)
-while box.info.replication[1].upstream.message == nil do fiber.sleep(0.01) end
-box.info.replication[1].upstream.message
-test_run:cmd("switch default")
+box.cfg{replication = {}}
 -- Restart the replica to reestablish replication.
 test_run:cmd("restart server replica")
 -- Wait for the replica to catch up.
diff --git a/test/replication/hot_standby.result b/test/replication/hot_standby.result
index 66ede5b7..24be0a94 100644
--- a/test/replication/hot_standby.result
+++ b/test/replication/hot_standby.result
@@ -284,27 +284,27 @@ _select(11, 20)
   - [19, 'the tuple 19']
   - [20, 'the tuple 20']
 ...
-test_run:cmd("deploy server default")
+test_run:cmd("stop server hot_standby")
 ---
 - true
 ...
-test_run:cmd("start server default")
+test_run:cmd("cleanup server hot_standby")
 ---
 - true
 ...
-test_run:cmd("switch default")
+test_run:cmd("deploy server default")
 ---
 - true
 ...
-test_run:cmd("stop server hot_standby")
+test_run:cmd("start server default")
 ---
 - true
 ...
-test_run:cmd("stop server replica")
+test_run:cmd("switch default")
 ---
 - true
 ...
-test_run:cmd("cleanup server hot_standby")
+test_run:cmd("stop server replica")
 ---
 - true
 ...
diff --git a/test/replication/hot_standby.test.lua b/test/replication/hot_standby.test.lua
index 8a7c837e..adb3fb6f 100644
--- a/test/replication/hot_standby.test.lua
+++ b/test/replication/hot_standby.test.lua
@@ -109,10 +109,10 @@ test_run:cmd("switch replica")
 _wait_lsn(10)
 _select(11, 20)
 
+test_run:cmd("stop server hot_standby")
+test_run:cmd("cleanup server hot_standby")
 test_run:cmd("deploy server default")
 test_run:cmd("start server default")
 test_run:cmd("switch default")
-test_run:cmd("stop server hot_standby")
 test_run:cmd("stop server replica")
-test_run:cmd("cleanup server hot_standby")
 test_run:cmd("cleanup server replica")
diff --git a/test/xlog-py/dup_key.result b/test/xlog-py/dup_key.result
index 53ae7322..f387e8e8 100644
--- a/test/xlog-py/dup_key.result
+++ b/test/xlog-py/dup_key.result
@@ -4,6 +4,10 @@ space = box.schema.space.create('test')
 index = box.space.test:create_index('primary')
 ---
 ...
+box.snapshot()
+---
+- ok
+...
 box.space.test:insert{1, 'first tuple'}
 ---
 - [1, 'first tuple']
@@ -13,20 +17,6 @@ box.space.test:insert{2, 'second tuple'}
 - [2, 'second tuple']
 ...
 .xlog exists
-space = box.schema.space.create('test')
----
-...
-index = box.space.test:create_index('primary')
----
-...
-box.space.test:insert{1, 'first tuple'}
----
-- [1, 'first tuple']
-...
-box.space.test:delete{1}
----
-- [1, 'first tuple']
-...
 box.space.test:insert{1, 'third tuple'}
 ---
 - [1, 'third tuple']
@@ -35,7 +25,7 @@ box.space.test:insert{2, 'fourth tuple'}
 ---
 - [2, 'fourth tuple']
 ...
-.xlog exists
+.xlog does not exist
 check log line for 'Duplicate key'
 
 'Duplicate key' exists in server log
diff --git a/test/xlog-py/dup_key.test.py b/test/xlog-py/dup_key.test.py
index 058d9e3f..1c033da4 100644
--- a/test/xlog-py/dup_key.test.py
+++ b/test/xlog-py/dup_key.test.py
@@ -8,6 +8,11 @@ import yaml
 
 server.stop()
 server.deploy()
+
+server.admin("space = box.schema.space.create('test')")
+server.admin("index = box.space.test:create_index('primary')")
+server.admin("box.snapshot()")
+
 lsn = int(yaml.load(server.admin("box.info.lsn", silent=True))[0])
 filename = str(lsn).zfill(20) + ".xlog"
 vardir = os.path.join(server.vardir, server.name)
@@ -15,40 +20,26 @@ wal_old = os.path.join(vardir, "old_" + filename)
 wal = os.path.join(vardir, filename)
 
 # Create wal#1
-server.admin("space = box.schema.space.create('test')")
-server.admin("index = box.space.test:create_index('primary')")
 server.admin("box.space.test:insert{1, 'first tuple'}")
 server.admin("box.space.test:insert{2, 'second tuple'}")
 server.stop()
 
-# Save wal #1
+# Save wal#1
 if os.access(wal, os.F_OK):
     print ".xlog exists"
     os.rename(wal, wal_old)
 
-lsn += 4
-
-# Create another wal#1
-server.start()
-server.admin("space = box.schema.space.create('test')")
-server.admin("index = box.space.test:create_index('primary')")
-server.admin("box.space.test:insert{1, 'first tuple'}")
-server.admin("box.space.test:delete{1}")
-server.stop()
-
-# Create wal#2
+# Write wal#2
 server.start()
 server.admin("box.space.test:insert{1, 'third tuple'}")
 server.admin("box.space.test:insert{2, 'fourth tuple'}")
 server.stop()
 
-if os.access(wal, os.F_OK):
-    print ".xlog exists"
-    # Replace wal#1 with saved copy
-    os.unlink(wal)
+# Restore wal#1
+if not os.access(wal, os.F_OK):
+    print ".xlog does not exist"
     os.rename(wal_old, wal)
 
-
 server.start()
 line = 'Duplicate key'
 print "check log line for '%s'" % line
diff --git a/test/xlog/errinj.result b/test/xlog/errinj.result
index 76cbe754..262677f1 100644
--- a/test/xlog/errinj.result
+++ b/test/xlog/errinj.result
@@ -43,6 +43,7 @@ require('fio').glob(name .. "/*.xlog")
 ---
 - - xlog/00000000000000000000.xlog
   - xlog/00000000000000000001.xlog
+  - xlog/00000000000000000002.xlog
 ...
 test_run:cmd('restart server default with cleanup=1')
 -- gh-881 iproto request with wal IO error
diff --git a/test/xlog/panic_on_lsn_gap.result b/test/xlog/panic_on_lsn_gap.result
index d5064ce6..4dd1291f 100644
--- a/test/xlog/panic_on_lsn_gap.result
+++ b/test/xlog/panic_on_lsn_gap.result
@@ -83,8 +83,9 @@ t
   - Failed to write to disk
 ...
 --
--- Before restart: oops, our LSN is 11,
--- even though we didn't insert anything.
+-- Before restart: our LSN is 1, because
+-- LSN is promoted in tx only on successful
+-- WAL write.
 --
 name = string.match(arg[0], "([^,]+)%.lua")
 ---
@@ -99,13 +100,12 @@ require('fio').glob(name .. "/*.xlog")
 ...
 test_run:cmd("restart server panic")
 --
--- after restart: our LSN is the LSN of the
--- last *written* row, all the failed
--- rows are gone from lsn counter.
+-- After restart: our LSN is the LSN of the
+-- last empty WAL created on shutdown, i.e. 11.
 --
 box.info.vclock
 ---
-- {1: 1}
+- {1: 11}
 ...
 box.space._schema:select{'key'}
 ---
@@ -153,7 +153,7 @@ t
 ...
 box.info.vclock
 ---
-- {1: 1}
+- {1: 11}
 ...
 box.error.injection.set("ERRINJ_WAL_WRITE", false)
 ---
@@ -176,12 +176,12 @@ s:replace{'key', 'test 2'}
 --
 box.info.vclock
 ---
-- {1: 12}
+- {1: 22}
 ...
 test_run:cmd("restart server panic")
 box.info.vclock
 ---
-- {1: 12}
+- {1: 22}
 ...
 box.space._schema:select{'key'}
 ---
@@ -194,7 +194,8 @@ name = string.match(arg[0], "([^,]+)%.lua")
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
-  - panic/00000000000000000001.xlog
+  - panic/00000000000000000011.xlog
+  - panic/00000000000000000022.xlog
 ...
 -- now insert 10 rows - so that the next
 -- row will need to switch the WAL
@@ -216,8 +217,8 @@ test_run:cmd("setopt delimiter ''");
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
-  - panic/00000000000000000001.xlog
-  - panic/00000000000000000012.xlog
+  - panic/00000000000000000011.xlog
+  - panic/00000000000000000022.xlog
 ...
 box.error.injection.set("ERRINJ_WAL_WRITE", true)
 ---
@@ -229,14 +230,14 @@ box.space._schema:replace{"key", 'test 3'}
 ...
 box.info.vclock
 ---
-- {1: 22}
+- {1: 32}
 ...
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
-  - panic/00000000000000000001.xlog
-  - panic/00000000000000000012.xlog
+  - panic/00000000000000000011.xlog
   - panic/00000000000000000022.xlog
+  - panic/00000000000000000032.xlog
 ...
 -- and the next one (just to be sure
 box.space._schema:replace{"key", 'test 3'}
@@ -245,14 +246,14 @@ box.space._schema:replace{"key", 'test 3'}
 ...
 box.info.vclock
 ---
-- {1: 22}
+- {1: 32}
 ...
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
-  - panic/00000000000000000001.xlog
-  - panic/00000000000000000012.xlog
+  - panic/00000000000000000011.xlog
   - panic/00000000000000000022.xlog
+  - panic/00000000000000000032.xlog
 ...
 box.error.injection.set("ERRINJ_WAL_WRITE", false)
 ---
@@ -265,14 +266,14 @@ box.space._schema:replace{"key", 'test 4'}
 ...
 box.info.vclock
 ---
-- {1: 25}
+- {1: 35}
 ...
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
-  - panic/00000000000000000001.xlog
-  - panic/00000000000000000012.xlog
+  - panic/00000000000000000011.xlog
   - panic/00000000000000000022.xlog
+  - panic/00000000000000000032.xlog
 ...
 -- restart is ok
 test_run:cmd("restart server panic")
@@ -331,12 +332,12 @@ name = string.match(arg[0], "([^,]+)%.lua")
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
-  - panic/00000000000000000001.xlog
-  - panic/00000000000000000012.xlog
+  - panic/00000000000000000011.xlog
   - panic/00000000000000000022.xlog
-  - panic/00000000000000000025.xlog
-  - panic/00000000000000000027.xlog
-  - panic/00000000000000000029.xlog
+  - panic/00000000000000000032.xlog
+  - panic/00000000000000000035.xlog
+  - panic/00000000000000000037.xlog
+  - panic/00000000000000000039.xlog
 ...
 test_run:cmd("restart server panic")
 box.space._schema:select{'key'}
@@ -354,12 +355,13 @@ name = string.match(arg[0], "([^,]+)%.lua")
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
-  - panic/00000000000000000001.xlog
-  - panic/00000000000000000012.xlog
+  - panic/00000000000000000011.xlog
   - panic/00000000000000000022.xlog
-  - panic/00000000000000000025.xlog
-  - panic/00000000000000000027.xlog
-  - panic/00000000000000000029.xlog
+  - panic/00000000000000000032.xlog
+  - panic/00000000000000000035.xlog
+  - panic/00000000000000000037.xlog
+  - panic/00000000000000000039.xlog
+  - panic/00000000000000000040.xlog
 ...
 test_run:cmd('switch default')
 ---
diff --git a/test/xlog/panic_on_lsn_gap.test.lua b/test/xlog/panic_on_lsn_gap.test.lua
index d72552d0..6221261a 100644
--- a/test/xlog/panic_on_lsn_gap.test.lua
+++ b/test/xlog/panic_on_lsn_gap.test.lua
@@ -34,17 +34,17 @@ end;
 test_run:cmd("setopt delimiter ''");
 t
 --
--- Before restart: oops, our LSN is 11,
--- even though we didn't insert anything.
+-- Before restart: our LSN is 1, because
+-- LSN is promoted in tx only on successful
+-- WAL write.
 --
 name = string.match(arg[0], "([^,]+)%.lua")
 box.info.vclock
 require('fio').glob(name .. "/*.xlog")
 test_run:cmd("restart server panic")
 --
--- after restart: our LSN is the LSN of the
--- last *written* row, all the failed
--- rows are gone from lsn counter.
+-- After restart: our LSN is the LSN of the
+-- last empty WAL created on shutdown, i.e. 11.
 --
 box.info.vclock
 box.space._schema:select{'key'}
diff --git a/test/xlog/panic_on_wal_error.result b/test/xlog/panic_on_wal_error.result
index 267b5340..345534ba 100644
--- a/test/xlog/panic_on_wal_error.result
+++ b/test/xlog/panic_on_wal_error.result
@@ -5,28 +5,7 @@ env = require('test_run')
 test_run = env.new()
 ---
 ...
-fio = require('fio')
----
-...
-glob = fio.pathjoin(box.cfg.wal_dir, '*.xlog')
----
-...
-for _, file in pairs(fio.glob(glob)) do fio.unlink(file) end
----
-...
-glob = fio.pathjoin(box.cfg.vinyl_dir, '*.vylog')
----
-...
-for _, file in pairs(fio.glob(glob)) do fio.unlink(file) end
----
-...
-glob = fio.pathjoin(box.cfg.memtx_dir, '*.snap')
----
-...
-for _, file in pairs(fio.glob(glob)) do fio.unlink(file) end
----
-...
-test_run:cmd("restart server default")
+test_run:cmd("restart server default with cleanup=True")
 box.schema.user.grant('guest', 'replication')
 ---
 ...
diff --git a/test/xlog/panic_on_wal_error.test.lua b/test/xlog/panic_on_wal_error.test.lua
index 4f598e33..29410cb2 100644
--- a/test/xlog/panic_on_wal_error.test.lua
+++ b/test/xlog/panic_on_wal_error.test.lua
@@ -2,14 +2,7 @@
 env = require('test_run')
 test_run = env.new()
 
-fio = require('fio')
-glob = fio.pathjoin(box.cfg.wal_dir, '*.xlog')
-for _, file in pairs(fio.glob(glob)) do fio.unlink(file) end
-glob = fio.pathjoin(box.cfg.vinyl_dir, '*.vylog')
-for _, file in pairs(fio.glob(glob)) do fio.unlink(file) end
-glob = fio.pathjoin(box.cfg.memtx_dir, '*.snap')
-for _, file in pairs(fio.glob(glob)) do fio.unlink(file) end
-test_run:cmd("restart server default")
+test_run:cmd("restart server default with cleanup=True")
 box.schema.user.grant('guest', 'replication')
 _ = box.schema.space.create('test')
 _ = box.space.test:create_index('pk')
-- 
2.11.0

[PATCH v2 6/6] error: move XlogGapError to box/error.h

[Vladimir Davydov]

All box exceptions belong to box/error.h. Let's move XlogGapError there
as well. This will facilitate conversion of recovery.cc to C when we
finally get to it. While we are at it, let's also move BuildXlogError
function declaration from diag.h to box/error.h, closer to its
definition.
---
 src/box/CMakeLists.txt |  2 +-
 src/box/error.cc       | 36 ++++++++++++++++++++++++++++++++----
 src/box/error.h        | 16 ++++++++++++++++
 src/box/recovery.cc    | 21 ---------------------
 src/diag.h             |  2 --
 5 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 6b1ae3e8..f4899091 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -21,7 +21,7 @@ set_property(DIRECTORY PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${lua_sources})
 
 include_directories(${ZSTD_INCLUDE_DIRS})
 
-add_library(box_error STATIC error.cc errcode.c)
+add_library(box_error STATIC error.cc errcode.c vclock.c)
 target_link_libraries(box_error core stat)
 
 add_library(vclock STATIC vclock.c)
diff --git a/src/box/error.cc b/src/box/error.cc
index 6b14dff0..c4f782a4 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -31,8 +31,11 @@
 #include "error.h"
 #include <stdio.h>
 
-#include <fiber.h>
-#include <rmean.h>
+#include "fiber.h"
+#include "rmean.h"
+#include "trigger.h"
+#include "vclock.h"
+#include "schema.h"
 
 /* {{{ public API */
 
@@ -176,8 +179,33 @@ BuildXlogError(const char *file, unsigned line, const char *format, ...)
 	}
 }
 
-#include "schema.h"
-#include "trigger.h"
+const struct type_info type_XlogGapError =
+	make_type("XlogGapError", &type_XlogError);
+
+XlogGapError::XlogGapError(const char *file, unsigned line,
+			   const struct vclock *from, const struct vclock *to)
+		: XlogError(&type_XlogGapError, file, line)
+{
+	char *s_from = vclock_to_string(from);
+	char *s_to = vclock_to_string(to);
+	snprintf(errmsg, sizeof(errmsg),
+		 "Missing .xlog file between LSN %lld %s and %lld %s",
+		 (long long) vclock_sum(from), s_from ? s_from : "",
+		 (long long) vclock_sum(to), s_to ? s_to : "");
+	free(s_from);
+	free(s_to);
+}
+
+struct error *
+BuildXlogGapError(const char *file, unsigned line,
+		  const struct vclock *from, const struct vclock *to)
+{
+	try {
+		return new XlogGapError(file, line, from, to);
+	} catch (OutOfMemory *e) {
+		return e;
+	}
+}
 
 struct rlist on_access_denied = RLIST_HEAD_INITIALIZER(on_access_denied);
 
diff --git a/src/box/error.h b/src/box/error.h
index c791e6c6..b8c7cf73 100644
--- a/src/box/error.h
+++ b/src/box/error.h
@@ -36,6 +36,8 @@
 extern "C" {
 #endif /* defined(__cplusplus) */
 
+struct vclock;
+
 struct error *
 BuildClientError(const char *file, unsigned line, uint32_t errcode, ...);
 
@@ -44,6 +46,12 @@ BuildAccessDeniedError(const char *file, unsigned int line,
 		       const char *access_type, const char *object_type,
 		       const char *object_name, const char *user_name);
 
+struct error *
+BuildXlogError(const char *file, unsigned line, const char *format, ...);
+
+struct error *
+BuildXlogGapError(const char *file, unsigned line,
+		  const struct vclock *from, const struct vclock *to);
 
 /** \cond public */
 
@@ -250,6 +258,14 @@ struct XlogError: public Exception
 	virtual void raise() { throw this; }
 };
 
+struct XlogGapError: public XlogError
+{
+	XlogGapError(const char *file, unsigned line,
+		     const struct vclock *from, const struct vclock *to);
+
+	virtual void raise() { throw this; }
+};
+
 #endif /* defined(__cplusplus) */
 
 #endif /* TARANTOOL_BOX_ERROR_H_INCLUDED */
diff --git a/src/box/recovery.cc b/src/box/recovery.cc
index 722b86c5..c007b1fa 100644
--- a/src/box/recovery.cc
+++ b/src/box/recovery.cc
@@ -74,27 +74,6 @@
  * IRR -> RR        # recovery_follow_local()
  */
 
-const struct type_info type_XlogGapError =
-	make_type("XlogGapError", &type_XlogError);
-
-struct XlogGapError: public XlogError {
-	/** Used by BuildXlogGapError() */
-	XlogGapError(const char *file, unsigned line,
-		     const struct vclock *from, const struct vclock *to)
-		:XlogError(&type_XlogGapError, file, line)
-	{
-		char *s_from = vclock_to_string(from);
-		char *s_to = vclock_to_string(to);
-		snprintf(errmsg, sizeof(errmsg),
-			 "Missing .xlog file between LSN %lld %s and %lld %s",
-			 (long long) vclock_sum(from), s_from ? s_from : "",
-			 (long long) vclock_sum(to), s_to ? s_to : "");
-		free(s_from);
-		free(s_to);
-	}
-	virtual void raise() { throw this; }
-};
-
 /* {{{ Initial recovery */
 
 /**
diff --git a/src/diag.h b/src/diag.h
index 0ccf86d0..a0b71f04 100644
--- a/src/diag.h
+++ b/src/diag.h
@@ -248,8 +248,6 @@ BuildIllegalParams(const char *file, unsigned line, const char *format, ...);
 struct error *
 BuildSystemError(const char *file, unsigned line, const char *format, ...);
 struct error *
-BuildXlogError(const char *file, unsigned line, const char *format, ...);
-struct error *
 BuildCollationError(const char *file, unsigned line, const char *format, ...);
 
 struct index_def;
-- 
2.11.0

Re: [PATCH v2 1/6] xlog: store prev vclock in xlog header

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/06/29 19:49]:
> This patch adds a new key to xlog header, PrevVclock, which contains the
> vclock of the previous xlog file in the xlog directory. It is set by
> xdir_create_xlog() to the last vclock in xdir::index. The new key is
> only present in XLOG files (it doesn't make sense for SNAP or VYLOG
> anyway). It will be used to make the check for xlog gaps more thorough.

I don't like has_prev_vclock. 

I don't know what to do with it yet. Ideally, I would make it xdir
property - and set it on for xlog dir.

I'm not sure it's feasible, looking.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH v2 1/6] xlog: store prev vclock in xlog header

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/06/29 19:49]:
> This patch adds a new key to xlog header, PrevVclock, which contains the
> vclock of the previous xlog file in the xlog directory. It is set by
> xdir_create_xlog() to the last vclock in xdir::index. The new key is
> only present in XLOG files (it doesn't make sense for SNAP or VYLOG
> anyway). It will be used to make the check for xlog gaps more thorough.

What about the following diff:

---
diff --git a/src/box/xlog.c b/src/box/xlog.c
index 59459b25d..a60f2ffa8 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -126,12 +126,10 @@ xlog_meta_format(const struct xlog_meta *meta, char *buf, int size)
 		VCLOCK_KEY ": %s\n",
 		meta->filetype, v13, PACKAGE_VERSION, instance_uuid, vstr);
 	free(vstr);
-	if (meta->has_prev_vclock) {
-		vstr = vclock_to_string(&meta->prev_vclock);
-		SNPRINT(total, snprintf, buf, size,
-			PREV_VCLOCK_KEY ": %s\n", vstr);
-		free(vstr);
-	}
+	vstr = vclock_to_string(&meta->prev_vclock);
+	SNPRINT(total, snprintf, buf, size,
+		PREV_VCLOCK_KEY ": %s\n", vstr);
+	free(vstr);
 	SNPRINT(total, snprintf, buf, size, "\n");
 	assert(total > 0);
 	return total;
@@ -263,7 +261,6 @@ xlog_meta_parse(struct xlog_meta *meta, const char **data,
 			 */
 			if (parse_vclock(val, val_end, &meta->prev_vclock) != 0)
 				return -1;
-			meta->has_prev_vclock = true;
 		} else if (memcmp(key, VERSION_KEY, key_end - key) == 0) {
 			/* Ignore Version: for now */
 		} else {
@@ -926,12 +923,10 @@ xdir_create_xlog(struct xdir *dir, struct xlog *xlog,
 	 * For WAL dir: store vclock of the previous xlog file
 	 * to check for gaps on recovery.
 	 */
-	if (dir->type == XLOG && !vclockset_empty(&dir->index)) {
+	if (!vclockset_empty(&dir->index)) {
 		vclock_copy(&meta.prev_vclock, vclockset_last(&dir->index));
-		meta.has_prev_vclock = true;
 	} else {
 		vclock_create(&meta.prev_vclock);
-		meta.has_prev_vclock = false;
 	}
 
 	if (xlog_create(xlog, filename, dir->open_wflags, &meta) != 0)
diff --git a/src/box/xlog.h b/src/box/xlog.h
index 259b5210b..08eb0952c 100644
--- a/src/box/xlog.h
+++ b/src/box/xlog.h
@@ -246,8 +246,6 @@ struct xlog_meta {
 	 * directory for missing WALs.
 	 */
 	struct vclock prev_vclock;
-	/** Set if @prev_vclock is present. */
-	bool has_prev_vclock;
 };
 
 /* }}} */
 ---

 Would it work?

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH v2 1/6] xlog: store prev vclock in xlog header

[Vladimir Davydov]

On Thu, Jul 05, 2018 at 09:52:41AM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/06/29 19:49]:
> > This patch adds a new key to xlog header, PrevVclock, which contains the
> > vclock of the previous xlog file in the xlog directory. It is set by
> > xdir_create_xlog() to the last vclock in xdir::index. The new key is
> > only present in XLOG files (it doesn't make sense for SNAP or VYLOG
> > anyway). It will be used to make the check for xlog gaps more thorough.
> 
> What about the following diff:
> 
> ---
> diff --git a/src/box/xlog.c b/src/box/xlog.c
> index 59459b25d..a60f2ffa8 100644
> --- a/src/box/xlog.c
> +++ b/src/box/xlog.c
> @@ -126,12 +126,10 @@ xlog_meta_format(const struct xlog_meta *meta, char *buf, int size)
>  		VCLOCK_KEY ": %s\n",
>  		meta->filetype, v13, PACKAGE_VERSION, instance_uuid, vstr);
>  	free(vstr);
> -	if (meta->has_prev_vclock) {
> -		vstr = vclock_to_string(&meta->prev_vclock);
> -		SNPRINT(total, snprintf, buf, size,
> -			PREV_VCLOCK_KEY ": %s\n", vstr);
> -		free(vstr);
> -	}
> +	vstr = vclock_to_string(&meta->prev_vclock);
> +	SNPRINT(total, snprintf, buf, size,
> +		PREV_VCLOCK_KEY ": %s\n", vstr);
> +	free(vstr);
>  	SNPRINT(total, snprintf, buf, size, "\n");
>  	assert(total > 0);
>  	return total;
> @@ -263,7 +261,6 @@ xlog_meta_parse(struct xlog_meta *meta, const char **data,
>  			 */
>  			if (parse_vclock(val, val_end, &meta->prev_vclock) != 0)
>  				return -1;
> -			meta->has_prev_vclock = true;

We need to know if PrevVclock is available so that we can skip the check
when recovering from an xlog generated by a previous version.

>  		} else if (memcmp(key, VERSION_KEY, key_end - key) == 0) {
>  			/* Ignore Version: for now */
>  		} else {
> @@ -926,12 +923,10 @@ xdir_create_xlog(struct xdir *dir, struct xlog *xlog,
>  	 * For WAL dir: store vclock of the previous xlog file
>  	 * to check for gaps on recovery.
>  	 */
> -	if (dir->type == XLOG && !vclockset_empty(&dir->index)) {
> +	if (!vclockset_empty(&dir->index)) {
>  		vclock_copy(&meta.prev_vclock, vclockset_last(&dir->index));
> -		meta.has_prev_vclock = true;
>  	} else {
>  		vclock_create(&meta.prev_vclock);
> -		meta.has_prev_vclock = false;
>  	}

Storing vclock in snap and vylog files doesn't make any sense IMHO.
I'd prefer to check dir->type explicitly.

If you dislike the extra variable, we can probably introduce a helper to
check if vclock is set, something like

	static inline bool
	vclock_is_set(const struct vclock *vclock)
	{
		return vclock->signature >= 0;
	}

Re: [PATCH v2 1/6] xlog: store prev vclock in xlog header

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/07/05 14:01]:
> On Thu, Jul 05, 2018 at 09:52:41AM +0300, Konstantin Osipov wrote:
> > * Vladimir Davydov <vdavydov.dev@gmail.com> [18/06/29 19:49]:
> > > This patch adds a new key to xlog header, PrevVclock, which contains the
> > > vclock of the previous xlog file in the xlog directory. It is set by
> > > xdir_create_xlog() to the last vclock in xdir::index. The new key is
> > > only present in XLOG files (it doesn't make sense for SNAP or VYLOG
> > > anyway). It will be used to make the check for xlog gaps more thorough.
> > 
> > What about the following diff:
> > 
> > ---
> > diff --git a/src/box/xlog.c b/src/box/xlog.c
> > index 59459b25d..a60f2ffa8 100644
> > --- a/src/box/xlog.c
> > +++ b/src/box/xlog.c
> > @@ -126,12 +126,10 @@ xlog_meta_format(const struct xlog_meta *meta, char *buf, int size)
> >  		VCLOCK_KEY ": %s\n",
> >  		meta->filetype, v13, PACKAGE_VERSION, instance_uuid, vstr);
> >  	free(vstr);
> > -	if (meta->has_prev_vclock) {
> > -		vstr = vclock_to_string(&meta->prev_vclock);
> > -		SNPRINT(total, snprintf, buf, size,
> > -			PREV_VCLOCK_KEY ": %s\n", vstr);
> > -		free(vstr);
> > -	}
> > +	vstr = vclock_to_string(&meta->prev_vclock);
> > +	SNPRINT(total, snprintf, buf, size,
> > +		PREV_VCLOCK_KEY ": %s\n", vstr);
> > +	free(vstr);
> >  	SNPRINT(total, snprintf, buf, size, "\n");
> >  	assert(total > 0);
> >  	return total;
> > @@ -263,7 +261,6 @@ xlog_meta_parse(struct xlog_meta *meta, const char **data,
> >  			 */
> >  			if (parse_vclock(val, val_end, &meta->prev_vclock) != 0)
> >  				return -1;
> > -			meta->has_prev_vclock = true;
> 
> We need to know if PrevVclock is available so that we can skip the check
> when recovering from an xlog generated by a previous version.

In theory we could use Version key for this.

> 
> >  		} else if (memcmp(key, VERSION_KEY, key_end - key) == 0) {
> >  			/* Ignore Version: for now */
> >  		} else {
> > @@ -926,12 +923,10 @@ xdir_create_xlog(struct xdir *dir, struct xlog *xlog,
> >  	 * For WAL dir: store vclock of the previous xlog file
> >  	 * to check for gaps on recovery.
> >  	 */
> > -	if (dir->type == XLOG && !vclockset_empty(&dir->index)) {
> > +	if (!vclockset_empty(&dir->index)) {
> >  		vclock_copy(&meta.prev_vclock, vclockset_last(&dir->index));
> > -		meta.has_prev_vclock = true;
> >  	} else {
> >  		vclock_create(&meta.prev_vclock);
> > -		meta.has_prev_vclock = false;
> >  	}
> 
> Storing vclock in snap and vylog files doesn't make any sense IMHO.
> I'd prefer to check dir->type explicitly.
> 
> If you dislike the extra variable, we can probably introduce a helper to
> check if vclock is set, something like
> 
> 	static inline bool
> 	vclock_is_set(const struct vclock *vclock)
> 	{
> 		return vclock->signature >= 0;
> 	}

This is better IMHO.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

[PATCH] xlog: get rid of xlog_meta::has_prev_vclock

[Vladimir Davydov]

Introduce vclock_is_set() helper and use it on xlog_meta::prev_vclock
instead.

Follow-up ac90b498d1b0 ("xlog: store prev vclock in xlog header").
---
https://github.com/tarantool/tarantool/commits/dv/wal-make-new-xlog-on-shutdown-follow-up

 src/box/recovery.cc |  2 +-
 src/box/vclock.h    | 21 +++++++++++++++
 src/box/vy_run.c    | 14 +++++-----
 src/box/xlog.c      | 78 +++++++++++++++++++++++++++++++----------------------
 src/box/xlog.h      | 14 ++++++++--
 5 files changed, 86 insertions(+), 43 deletions(-)

diff --git a/src/box/recovery.cc b/src/box/recovery.cc
index 2cab8308..f0b85a53 100644
--- a/src/box/recovery.cc
+++ b/src/box/recovery.cc
@@ -154,7 +154,7 @@ recovery_open_log(struct recovery *r, const struct vclock *vclock)
 	}
 
 	if (state != XLOG_CURSOR_NEW &&
-	    r->cursor.meta.has_prev_vclock &&
+	    vclock_is_set(&r->cursor.meta.prev_vclock) &&
 	    vclock_compare(&r->cursor.meta.prev_vclock, &meta.vclock) != 0) {
 		/*
 		 * WALs are missing between the last scanned WAL
diff --git a/src/box/vclock.h b/src/box/vclock.h
index 3cd60102..be835360 100644
--- a/src/box/vclock.h
+++ b/src/box/vclock.h
@@ -118,6 +118,27 @@ vclock_create(struct vclock *vclock)
 	memset(vclock, 0, sizeof(*vclock));
 }
 
+/**
+ * Reset a vclock object. After this function is called,
+ * vclock_is_set() will return false.
+ */
+static inline void
+vclock_clear(struct vclock *vclock)
+{
+	memset(vclock, 0, sizeof(*vclock));
+	vclock->signature = -1;
+}
+
+/**
+ * Returns false if the vclock was cleared with vclock_clear(),
+ * true otherwise.
+ */
+static inline bool
+vclock_is_set(const struct vclock *vclock)
+{
+	return vclock->signature >= 0;
+}
+
 static inline int64_t
 vclock_get(const struct vclock *vclock, uint32_t replica_id)
 {
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index dc837c2b..eae3e74d 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -1960,10 +1960,9 @@ vy_run_write_index(struct vy_run *run, const char *dirpath,
 	say_info("writing `%s'", path);
 
 	struct xlog index_xlog;
-	struct xlog_meta meta = {
-		.filetype = XLOG_META_TYPE_INDEX,
-		.instance_uuid = INSTANCE_UUID,
-	};
+	struct xlog_meta meta;
+	xlog_meta_create(&meta, XLOG_META_TYPE_INDEX, &INSTANCE_UUID,
+			 NULL, NULL);
 	if (xlog_create(&index_xlog, path, 0, &meta) < 0)
 		return -1;
 
@@ -2057,10 +2056,9 @@ vy_run_writer_create_xlog(struct vy_run_writer *writer)
 			    writer->space_id, writer->iid, writer->run->id,
 			    VY_FILE_RUN);
 	say_info("writing `%s'", path);
-	const struct xlog_meta meta = {
-		.filetype = XLOG_META_TYPE_RUN,
-		.instance_uuid = INSTANCE_UUID,
-	};
+	struct xlog_meta meta;
+	xlog_meta_create(&meta, XLOG_META_TYPE_RUN, &INSTANCE_UUID,
+			 NULL, NULL);
 	if (xlog_create(&writer->data_xlog, path, 0, &meta) != 0)
 		return -1;
 	writer->data_xlog.rate_limit = writer->run->env->snap_io_rate_limit;
diff --git a/src/box/xlog.c b/src/box/xlog.c
index 59459b25..5ed11fc8 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -98,6 +98,24 @@ enum {
 static const char v13[] = "0.13";
 static const char v12[] = "0.12";
 
+void
+xlog_meta_create(struct xlog_meta *meta, const char *filetype,
+		 const struct tt_uuid *instance_uuid,
+		 const struct vclock *vclock,
+		 const struct vclock *prev_vclock)
+{
+	snprintf(meta->filetype, sizeof(meta->filetype), "%s", filetype);
+	meta->instance_uuid = *instance_uuid;
+	if (vclock != NULL)
+		vclock_copy(&meta->vclock, vclock);
+	else
+		vclock_clear(&meta->vclock);
+	if (prev_vclock != NULL)
+		vclock_copy(&meta->prev_vclock, prev_vclock);
+	else
+		vclock_clear(&meta->prev_vclock);
+}
+
 /**
  * Format xlog metadata into @a buf of size @a size
  *
@@ -113,21 +131,26 @@ static const char v12[] = "0.12";
 static int
 xlog_meta_format(const struct xlog_meta *meta, char *buf, int size)
 {
-	char *vstr = vclock_to_string(&meta->vclock);
-	if (vstr == NULL)
-		return -1;
-	char *instance_uuid = tt_uuid_str(&meta->instance_uuid);
 	int total = 0;
 	SNPRINT(total, snprintf, buf, size,
 		"%s\n"
 		"%s\n"
 		VERSION_KEY ": %s\n"
-		INSTANCE_UUID_KEY ": %s\n"
-		VCLOCK_KEY ": %s\n",
-		meta->filetype, v13, PACKAGE_VERSION, instance_uuid, vstr);
-	free(vstr);
-	if (meta->has_prev_vclock) {
-		vstr = vclock_to_string(&meta->prev_vclock);
+		INSTANCE_UUID_KEY ": %s\n",
+		meta->filetype, v13, PACKAGE_VERSION,
+		tt_uuid_str(&meta->instance_uuid));
+	if (vclock_is_set(&meta->vclock)) {
+		char *vstr = vclock_to_string(&meta->vclock);
+		if (vstr == NULL)
+			return -1;
+		SNPRINT(total, snprintf, buf, size,
+			VCLOCK_KEY ": %s\n", vstr);
+		free(vstr);
+	}
+	if (vclock_is_set(&meta->prev_vclock)) {
+		char *vstr = vclock_to_string(&meta->prev_vclock);
+		if (vstr == NULL)
+			return -1;
 		SNPRINT(total, snprintf, buf, size,
 			PREV_VCLOCK_KEY ": %s\n", vstr);
 		free(vstr);
@@ -214,6 +237,9 @@ xlog_meta_parse(struct xlog_meta *meta, const char **data,
 		return -1;
 	}
 
+	vclock_clear(&meta->vclock);
+	vclock_clear(&meta->prev_vclock);
+
 	/*
 	 * Parse "key: value" pairs
 	 */
@@ -263,7 +289,6 @@ xlog_meta_parse(struct xlog_meta *meta, const char **data,
 			 */
 			if (parse_vclock(val, val_end, &meta->prev_vclock) != 0)
 				return -1;
-			meta->has_prev_vclock = true;
 		} else if (memcmp(key, VERSION_KEY, key_end - key) == 0) {
 			/* Ignore Version: for now */
 		} else {
@@ -748,7 +773,8 @@ xlog_create(struct xlog *xlog, const char *name, int flags,
 	int meta_len;
 
 	/*
-	 * Check that the file without .inprogress suffix doesn't exist.
+	 * Check whether a file with this name already exists.
+	 * We don't overwrite existing files.
 	 */
 	if (access(name, F_OK) == 0) {
 		errno = EEXIST;
@@ -905,35 +931,23 @@ int
 xdir_create_xlog(struct xdir *dir, struct xlog *xlog,
 		 const struct vclock *vclock)
 {
-	char *filename;
 	int64_t signature = vclock_sum(vclock);
-	struct xlog_meta meta;
 	assert(signature >= 0);
 	assert(!tt_uuid_is_nil(dir->instance_uuid));
 
 	/*
-	* Check whether a file with this name already exists.
-	* We don't overwrite existing files.
-	*/
-	filename = xdir_format_filename(dir, signature, NONE);
-
-	/* Setup inherited values */
-	snprintf(meta.filetype, sizeof(meta.filetype), "%s", dir->filetype);
-	meta.instance_uuid = *dir->instance_uuid;
-	vclock_copy(&meta.vclock, vclock);
-
-	/*
 	 * For WAL dir: store vclock of the previous xlog file
 	 * to check for gaps on recovery.
 	 */
-	if (dir->type == XLOG && !vclockset_empty(&dir->index)) {
-		vclock_copy(&meta.prev_vclock, vclockset_last(&dir->index));
-		meta.has_prev_vclock = true;
-	} else {
-		vclock_create(&meta.prev_vclock);
-		meta.has_prev_vclock = false;
-	}
+	const struct vclock *prev_vclock = NULL;
+	if (dir->type == XLOG && !vclockset_empty(&dir->index))
+		prev_vclock = vclockset_last(&dir->index);
+
+	struct xlog_meta meta;
+	xlog_meta_create(&meta, dir->filetype, dir->instance_uuid,
+			 vclock, prev_vclock);
 
+	char *filename = xdir_format_filename(dir, signature, NONE);
 	if (xlog_create(xlog, filename, dir->open_wflags, &meta) != 0)
 		return -1;
 
diff --git a/src/box/xlog.h b/src/box/xlog.h
index d27e38e6..476105c2 100644
--- a/src/box/xlog.h
+++ b/src/box/xlog.h
@@ -246,10 +246,20 @@ struct xlog_meta {
 	 * directory for missing WALs.
 	 */
 	struct vclock prev_vclock;
-	/** Set if @prev_vclock is present. */
-	bool has_prev_vclock;
 };
 
+/**
+ * Initialize xlog meta struct.
+ *
+ * @vclock and @prev_vclock are optional: if the value is NULL,
+ * the key won't be written to the xlog header.
+ */
+void
+xlog_meta_create(struct xlog_meta *meta, const char *filetype,
+		 const struct tt_uuid *instance_uuid,
+		 const struct vclock *vclock,
+		 const struct vclock *prev_vclock);
+
 /* }}} */
 
 /**
-- 
2.11.0