[PATCH 2/3] wal: rollback vclock on write failure

[Vladimir Davydov <vdavydov.dev@gmail.com>]

In order to determine whether we need to rebootstrap the instance on
startup, we need to know its vclock. To find it out, we scan the last
xlog file before proceeding to local recovery, but this means in case
rebootstrap is not required we scan the last xlog twice, which is
sub-optimal. To avoid double scan, we can create a new empty xlog before
shutting down the server and reopen it after restart. However, since we
promote WAL writer vclock even if xlog write fails, there will be an LSN
gap between the last xlog and the one created on shutdown in case we
failed to write last few records. To avoid that, let's rollback WAL
writer vclock if write fails. BTW this will make it consistent with
replicaset vclock - see commit 3c4bac715960a ("Follow vclock only for
success wal writes").
---
 src/box/wal.c                       |  8 +++++++-
 test/xlog/panic_on_lsn_gap.result   | 33 +++++++++++++--------------------
 test/xlog/panic_on_lsn_gap.test.lua | 15 ++++-----------
 3 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/src/box/wal.c b/src/box/wal.c
index f6b0fa66..1c6d2422 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -637,14 +637,18 @@ wal_write_to_disk(struct cmsg *msg)
 	 */
 	struct journal_entry *entry;
 	struct stailq_entry *last_committed = NULL;
+	struct vclock last_committed_vclock;
+	vclock_copy(&last_committed_vclock, &writer->vclock);
 	stailq_foreach_entry(entry, &wal_msg->commit, fifo) {
 		wal_assign_lsn(writer, entry->rows, entry->rows + entry->n_rows);
 		entry->res = vclock_sum(&writer->vclock);
 		int rc = xlog_write_entry(l, entry);
 		if (rc < 0)
 			goto done;
-		if (rc > 0)
+		if (rc > 0) {
 			last_committed = &entry->fifo;
+			vclock_copy(&last_committed_vclock, &writer->vclock);
+		}
 		/* rc == 0: the write is buffered in xlog_tx */
 	}
 	if (xlog_flush(l) < 0)
@@ -670,6 +674,8 @@ done:
 	stailq_cut_tail(&wal_msg->commit, last_committed, &rollback);
 
 	if (!stailq_empty(&rollback)) {
+		/* Reset WAL writer vclock. */
+		vclock_copy(&writer->vclock, &last_committed_vclock);
 		/* Update status of the successfully committed requests. */
 		stailq_foreach_entry(entry, &rollback, fifo)
 			entry->res = -1;
diff --git a/test/xlog/panic_on_lsn_gap.result b/test/xlog/panic_on_lsn_gap.result
index 313850a6..731eec4e 100644
--- a/test/xlog/panic_on_lsn_gap.result
+++ b/test/xlog/panic_on_lsn_gap.result
@@ -31,10 +31,6 @@ box.info.vclock
 s = box.space._schema
 ---
 ...
--- we need to have at least one record in the
--- xlog otherwise the server believes that there
--- is an lsn gap during recovery.
---
 s:replace{"key", 'test 1'}
 ---
 - ['key', 'test 1']
@@ -83,8 +79,8 @@ 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
+-- we didn't insert anything.
 --
 name = string.match(arg[0], "([^,]+)%.lua")
 ---
@@ -100,8 +96,7 @@ 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.
+-- last written row, i.e. 1 again.
 --
 box.info.vclock
 ---
@@ -161,9 +156,7 @@ box.error.injection.set("ERRINJ_WAL_WRITE", false)
 ...
 --
 -- Write a good row after a series of failed
--- rows. There is a gap in LSN, correct,
--- but it's *inside* a single WAL, so doesn't
--- affect WAL search in recover_remaining_wals()
+-- rows. There is no gap in LSN.
 --
 s:replace{'key', 'test 2'}
 ---
@@ -176,12 +169,12 @@ s:replace{'key', 'test 2'}
 --
 box.info.vclock
 ---
-- {1: 12}
+- {1: 2}
 ...
 test_run:cmd("restart server panic")
 box.info.vclock
 ---
-- {1: 12}
+- {1: 2}
 ...
 box.space._schema:select{'key'}
 ---
@@ -217,7 +210,7 @@ require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
   - panic/00000000000000000001.xlog
-  - panic/00000000000000000012.xlog
+  - panic/00000000000000000002.xlog
 ...
 box.error.injection.set("ERRINJ_WAL_WRITE", true)
 ---
@@ -229,14 +222,14 @@ box.space._schema:replace{"key", 'test 3'}
 ...
 box.info.vclock
 ---
-- {1: 22}
+- {1: 12}
 ...
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
   - panic/00000000000000000001.xlog
+  - panic/00000000000000000002.xlog
   - panic/00000000000000000012.xlog
-  - panic/00000000000000000022.xlog
 ...
 -- and the next one (just to be sure
 box.space._schema:replace{"key", 'test 3'}
@@ -245,14 +238,14 @@ box.space._schema:replace{"key", 'test 3'}
 ...
 box.info.vclock
 ---
-- {1: 22}
+- {1: 12}
 ...
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
   - panic/00000000000000000001.xlog
+  - panic/00000000000000000002.xlog
   - panic/00000000000000000012.xlog
-  - panic/00000000000000000022.xlog
 ...
 box.error.injection.set("ERRINJ_WAL_WRITE", false)
 ---
@@ -265,14 +258,14 @@ box.space._schema:replace{"key", 'test 4'}
 ...
 box.info.vclock
 ---
-- {1: 25}
+- {1: 13}
 ...
 require('fio').glob(name .. "/*.xlog")
 ---
 - - panic/00000000000000000000.xlog
   - panic/00000000000000000001.xlog
+  - panic/00000000000000000002.xlog
   - panic/00000000000000000012.xlog
-  - panic/00000000000000000022.xlog
 ...
 -- restart is ok
 test_run:cmd("restart server panic")
diff --git a/test/xlog/panic_on_lsn_gap.test.lua b/test/xlog/panic_on_lsn_gap.test.lua
index 248a3e63..7f16d68e 100644
--- a/test/xlog/panic_on_lsn_gap.test.lua
+++ b/test/xlog/panic_on_lsn_gap.test.lua
@@ -13,10 +13,6 @@ test_run:cmd("start server panic")
 test_run:cmd("switch panic")
 box.info.vclock
 s = box.space._schema
--- we need to have at least one record in the
--- xlog otherwise the server believes that there
--- is an lsn gap during recovery.
---
 s:replace{"key", 'test 1'}
 box.info.vclock
 box.error.injection.set("ERRINJ_WAL_WRITE", true)
@@ -34,8 +30,8 @@ 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
+-- we didn't insert anything.
 --
 name = string.match(arg[0], "([^,]+)%.lua")
 box.info.vclock
@@ -43,8 +39,7 @@ 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.
+-- last written row, i.e. 1 again.
 --
 box.info.vclock
 box.space._schema:select{'key'}
@@ -65,9 +60,7 @@ box.info.vclock
 box.error.injection.set("ERRINJ_WAL_WRITE", false)
 --
 -- Write a good row after a series of failed
--- rows. There is a gap in LSN, correct,
--- but it's *inside* a single WAL, so doesn't
--- affect WAL search in recover_remaining_wals()
+-- rows. There is no gap in LSN.
 --
 s:replace{'key', 'test 2'}
 --
-- 
2.11.0