[Tarantool-patches] [PATCH] vinyl: restart read iterator in case of roll backed WAL
Nikita Pettik
korablev at tarantool.org
Tue May 19 15:50:40 MSK 2020
Disk scan in vinyl is known to yield during read. So it opens a window
for modifications of in-memory level. Imagine following scenario: right
before data selection tuple is inserted into space. It passes first
stage of commit procedure, i.e. it is prepared to be committed but still
is not yet reached WAL. Meanwhile we are starting to read the same key.
At this moment prepared statement is already inserted to in-memory tree
ergo visible to read iterator. So, read iterator fetches this statement
and proceeds to disk scan. In turn, disk scan yields and in this moment
WAL fails to write statement on disk, so it is roll-backed and removed
from in-memory tree. Read iterator must recognize this situation and
should not include roll-backed tuple into result set. To achieve this
let's patch vy_read_iterator_restore_mem() so that it returns return
code > 0 in case read iterator should be restarted.
Close #3395
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-3395-vy-read-prepared
Issue: https://github.com/tarantool/tarantool/issues/3395
src/box/vy_read_iterator.c | 18 +++-
.../gh-3395-read-prepared-uncommitted.result | 95 +++++++++++++++++++
...gh-3395-read-prepared-uncommitted.test.lua | 41 ++++++++
test/vinyl/suite.ini | 2 +-
4 files changed, 153 insertions(+), 3 deletions(-)
create mode 100644 test/vinyl/gh-3395-read-prepared-uncommitted.result
create mode 100644 test/vinyl/gh-3395-read-prepared-uncommitted.test.lua
diff --git a/src/box/vy_read_iterator.c b/src/box/vy_read_iterator.c
index dcc9a0873..d83d5f81d 100644
--- a/src/box/vy_read_iterator.c
+++ b/src/box/vy_read_iterator.c
@@ -382,6 +382,9 @@ vy_read_iterator_scan_disk(struct vy_read_iterator *itr, uint32_t disk_src,
* Restore the position of the active in-memory tree iterator
* after a yield caused by a disk read and update 'next'
* if necessary.
+ * @retval -1 In case of error (e.g. OOM);
+ * @retval 0 Successful execution;
+ * @retval 1 Restart of advance_iterator is required.
*/
static NODISCARD int
vy_read_iterator_restore_mem(struct vy_read_iterator *itr,
@@ -405,8 +408,14 @@ vy_read_iterator_restore_mem(struct vy_read_iterator *itr,
* Memory trees are append-only so if the
* source is not on top of the heap after
* restoration, it was not before.
+ * It may turn out that we read statement
+ * which was prepared but not committed to
+ * WAL. In such a rare case, we should restart
+ * iterator_advance() to get the right key.
*/
- assert(src->front_id < itr->front_id);
+ assert(src->front_id <= itr->front_id);
+ if (src->front_id == itr->front_id)
+ return 1;
return 0;
}
if (cmp < 0) {
@@ -529,8 +538,13 @@ rescan_disk:
* as it is owned exclusively by the current fiber so the only
* source to check is the active in-memory tree.
*/
- if (vy_read_iterator_restore_mem(itr, &next) != 0)
+ int rc = vy_read_iterator_restore_mem(itr, &next);
+ if (rc < 0)
return -1;
+ if (rc > 0) {
+ vy_read_iterator_restore(itr);
+ goto restart;
+ }
/*
* Scan the next range in case we transgressed the current
* range's boundaries.
diff --git a/test/vinyl/gh-3395-read-prepared-uncommitted.result b/test/vinyl/gh-3395-read-prepared-uncommitted.result
new file mode 100644
index 000000000..a4ec023fd
--- /dev/null
+++ b/test/vinyl/gh-3395-read-prepared-uncommitted.result
@@ -0,0 +1,95 @@
+-- test-run result file version 2
+-- Disk scan in vinyl is known to yield during read. So it opens
+-- a window for modifications of in-memory level. Let's test
+-- following case: right before data selection tuple is inserted
+-- into space. It passes first stage of commit procedure, i.e. it
+-- is prepared to be committed but still not yet reached WAL.
+-- Meanwhile we are starting read of the same key. At this moment
+-- prepared statement is already inserted to in-memory tree. So,
+-- read iterator fetches this statement and proceeds to disk scan.
+-- In turn, disk scan yields and in this moment WAL fails to write
+-- statement on disk, so it is roll-backed. Read iterator must
+-- recognize this situation and must not include roll-backed tuple
+-- into result set.
+--
+fiber = require('fiber')
+ | ---
+ | ...
+
+errinj = box.error.injection
+ | ---
+ | ...
+s = box.schema.create_space('test', {engine = 'vinyl'})
+ | ---
+ | ...
+pk = s:create_index('pk')
+ | ---
+ | ...
+sk = s:create_index('sk', {parts = {{2, 'unsigned'}}, unique = false})
+ | ---
+ | ...
+s:replace{3, 2}
+ | ---
+ | - [3, 2]
+ | ...
+s:replace{2, 2}
+ | ---
+ | - [2, 2]
+ | ...
+box.snapshot()
+ | ---
+ | - ok
+ | ...
+-- Tuple {1, 2} is not going to be committed, so it should not
+-- appear in the result set.
+--
+c = fiber.channel(1)
+ | ---
+ | ...
+function do_write() s:replace{1, 2} end
+ | ---
+ | ...
+errinj.set("ERRINJ_WAL_DELAY", true)
+ | ---
+ | - ok
+ | ...
+writer = fiber.create(do_write)
+ | ---
+ | ...
+
+function do_read() local ret = sk:select{2} c:put(ret) end
+ | ---
+ | ...
+errinj.set("ERRINJ_VY_READ_PAGE_DELAY", true)
+ | ---
+ | - ok
+ | ...
+f = fiber.create(do_read)
+ | ---
+ | ...
+errinj.set("ERRINJ_WAL_WRITE", true)
+ | ---
+ | - ok
+ | ...
+errinj.set("ERRINJ_WAL_DELAY", false)
+ | ---
+ | - ok
+ | ...
+errinj.set("ERRINJ_VY_READ_PAGE_DELAY", false)
+ | ---
+ | - ok
+ | ...
+
+c:get()
+ | ---
+ | - - [2, 2]
+ | - [3, 2]
+ | ...
+
+errinj.set("ERRINJ_WAL_WRITE", false)
+ | ---
+ | - ok
+ | ...
+s:drop()
+ | ---
+ | ...
diff --git a/test/vinyl/gh-3395-read-prepared-uncommitted.test.lua b/test/vinyl/gh-3395-read-prepared-uncommitted.test.lua
new file mode 100644
index 000000000..d75b36d1a
--- /dev/null
+++ b/test/vinyl/gh-3395-read-prepared-uncommitted.test.lua
@@ -0,0 +1,41 @@
+-- Disk scan in vinyl is known to yield during read. So it opens
+-- a window for modifications of in-memory level. Let's test
+-- following case: right before data selection tuple is inserted
+-- into space. It passes first stage of commit procedure, i.e. it
+-- is prepared to be committed but still not yet reached WAL.
+-- Meanwhile we are starting read of the same key. At this moment
+-- prepared statement is already inserted to in-memory tree. So,
+-- read iterator fetches this statement and proceeds to disk scan.
+-- In turn, disk scan yields and in this moment WAL fails to write
+-- statement on disk, so it is roll-backed. Read iterator must
+-- recognize this situation and must not include roll-backed tuple
+-- into result set.
+--
+fiber = require('fiber')
+
+errinj = box.error.injection
+s = box.schema.create_space('test', {engine = 'vinyl'})
+pk = s:create_index('pk')
+sk = s:create_index('sk', {parts = {{2, 'unsigned'}}, unique = false})
+s:replace{3, 2}
+s:replace{2, 2}
+box.snapshot()
+-- Tuple {1, 2} is not going to be committed, so it should not
+-- appear in the result set.
+--
+c = fiber.channel(1)
+function do_write() s:replace{1, 2} end
+errinj.set("ERRINJ_WAL_DELAY", true)
+writer = fiber.create(do_write)
+
+function do_read() local ret = sk:select{2} c:put(ret) end
+errinj.set("ERRINJ_VY_READ_PAGE_DELAY", true)
+f = fiber.create(do_read)
+errinj.set("ERRINJ_WAL_WRITE", true)
+errinj.set("ERRINJ_WAL_DELAY", false)
+errinj.set("ERRINJ_VY_READ_PAGE_DELAY", false)
+
+c:get()
+
+errinj.set("ERRINJ_WAL_WRITE", false)
+s:drop()
diff --git a/test/vinyl/suite.ini b/test/vinyl/suite.ini
index 8b7e87955..09829b975 100644
--- a/test/vinyl/suite.ini
+++ b/test/vinyl/suite.ini
@@ -2,7 +2,7 @@
core = tarantool
description = vinyl integration tests
script = vinyl.lua
-release_disabled = errinj.test.lua errinj_ddl.test.lua errinj_gc.test.lua errinj_stat.test.lua errinj_tx.test.lua errinj_vylog.test.lua partial_dump.test.lua quota_timeout.test.lua recovery_quota.test.lua replica_rejoin.test.lua gh-4864-stmt-alloc-fail-compact.test.lua gh-4805-open-run-err-recovery.test.lua
+release_disabled = errinj.test.lua errinj_ddl.test.lua errinj_gc.test.lua errinj_stat.test.lua errinj_tx.test.lua errinj_vylog.test.lua partial_dump.test.lua quota_timeout.test.lua recovery_quota.test.lua replica_rejoin.test.lua gh-4864-stmt-alloc-fail-compact.test.lua gh-4805-open-run-err-recovery.test.lua gh-3395-read-prepared-uncommitted.test.lua
config = suite.cfg
lua_libs = suite.lua stress.lua large.lua txn_proxy.lua ../box/lua/utils.lua
use_unix_sockets = True
--
2.17.1
More information about the Tarantool-patches
mailing list