From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 2/3] wal: rollback vclock on write failure Date: Fri, 15 Jun 2018 18:48:21 +0300 Message-Id: In-Reply-To: References: <20180614125920.GD959@chai> In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: 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