Skip to content

Commit 2a0a56c

Browse files
Gerold103Gerold103
committed
qsync: handle async txns right during CONFIRM
It is possible that a new async transaction is added to the limbo when there is an in-progress CONFIRM WAL write for all the pending sync transactions. Then when CONFIRM WAL write is done, it might see that the limbo now in the first place contains an async transaction not yet written to WAL. A suspicious situation - on one hand the async transaction does not have any blocking sync txns before it and can be considered complete, on the other hand its WAL write is not done and it is not complete. Before this patch it resulted into a crash - limbo didn't consider the situation possible at all. Now when CONFIRM covers a not yet written async transactions, they are removed from the limbo and are turned to plain transactions. When their WAL write is done, they see they no more have TXN_WAIT_SYNC flag and don't even need to interact with the limbo. It is important to remove them from the limbo right when the CONFIRM is done. Because otherwise their limbo entry may be not removed at all when it is done on a replica. On a replica the limbo entries are removed only by CONFIRM/ROLLBACK/PROMOTE. If there would be an async transaction in the first position in the limbo queue, it wouldn't be deleted until next sync transaction appears. This replica case is not possible now though. Because all synchro entries on the applier are written in a blocking way. Nonetheless if it ever becomes non-blocking, the code should handle it ok. Closes tarantool#6057
1 parent 8494d84 commit 2a0a56c

7 files changed

+295
-6
lines changed

‎changelogs/unreleased/gh-6057-qsync-confirm-async-no-wal.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
## bugfix/replication
2+
3+
* Fixed a possible crash when a synchronous transaction was followed by an
4+
asynchronous transaction right when its confirmation was being written
5+
(gh-6057).

‎src/box/txn.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -879,9 +879,14 @@ txn_commit(struct txn *txn)
879879
req = txn_journal_entry_new(txn);
880880
if (req == NULL)
881881
goto rollback;
882-
883-
bool is_sync = txn_has_flag(txn, TXN_WAIT_SYNC);
884-
if (is_sync) {
882+
/*
883+
* Do not cache the flag value in a variable. The flag might be deleted
884+
* during WAL write. This can happen for async transactions created
885+
* during CONFIRM write, whose all blocking sync transactions get
886+
* confirmed. Then they turn the async transaction into just a plain
887+
* txn not waiting for anything.
888+
*/
889+
if (txn_has_flag(txn, TXN_WAIT_SYNC)) {
885890
/*
886891
* Remote rows, if any, come before local rows, so
887892
* check for originating instance id here.
@@ -900,13 +905,13 @@ txn_commit(struct txn *txn)
900905

901906
fiber_set_txn(fiber(), NULL);
902907
if (journal_write(req) != 0 || req->res < 0) {
903-
if (is_sync)
908+
if (txn_has_flag(txn, TXN_WAIT_SYNC))
904909
txn_limbo_abort(&txn_limbo, limbo_entry);
905910
diag_set(ClientError, ER_WAL_IO);
906911
diag_log();
907912
goto rollback;
908913
}
909-
if (is_sync) {
914+
if (txn_has_flag(txn, TXN_WAIT_SYNC)) {
910915
if (txn_has_flag(txn, TXN_WAIT_ACK)) {
911916
int64_t lsn = req->rows[req->n_rows - 1]->lsn;
912917
/*

‎src/box/txn_limbo.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,33 @@ txn_limbo_read_confirm(struct txn_limbo *limbo, int64_t lsn)
389389
*/
390390
if (e->lsn == -1)
391391
break;
392+
} else if (e->txn->signature < 0) {
393+
/*
394+
* A transaction might be covered by the CONFIRM even if
395+
* it is not written to WAL yet when it is an async
396+
* transaction. It could be created just when the
397+
* CONFIRM was being written to WAL.
398+
*/
399+
assert(e->txn->status == TXN_PREPARED);
400+
/*
401+
* Let it complete normally as a plain transaction. It
402+
* is important to remove the limbo entry, because the
403+
* async transaction might be committed in a
404+
* non-blocking way and won't ever wait explicitly for
405+
* its completion. Therefore, won't be able to remove
406+
* the limbo entry on its own. This happens for txns
407+
* created in the applier.
408+
*/
409+
txn_clear_flags(e->txn, TXN_WAIT_SYNC);
410+
txn_limbo_remove(limbo, e);
411+
/*
412+
* The limbo entry now should not be used by the owner
413+
* transaction since it just became a plain one. Nullify
414+
* the txn to get a crash on any usage attempt instead
415+
* of potential undefined behaviour.
416+
*/
417+
e->txn = NULL;
418+
continue;
392419
}
393420
e->is_commit = true;
394421
txn_limbo_remove(limbo, e);

‎test/replication/gh-6057-qsync-confirm-async-no-wal.result

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
-- test-run result file version 2
2+
test_run = require('test_run').new()
3+
| ---
4+
| ...
5+
fiber = require('fiber')
6+
| ---
7+
| ...
8+
9+
--
10+
-- gh-6057: while CONFIRM is being written, there might appear async
11+
-- transactions not having an LSN/signature yet. When CONFIRM is done, it covers
12+
-- these transactions too since they don't need to wait for an ACK, but it
13+
-- should not try to complete them. Because their WAL write is not done and
14+
-- it might even fail later. It should simply turn them into plain transactions
15+
-- not depending on any synchronous ones.
16+
--
17+
18+
old_synchro_quorum = box.cfg.replication_synchro_quorum
19+
| ---
20+
| ...
21+
old_synchro_timeout = box.cfg.replication_synchro_timeout
22+
| ---
23+
| ...
24+
box.cfg{ \
25+
replication_synchro_quorum = 1, \
26+
replication_synchro_timeout = 1000000, \
27+
}
28+
| ---
29+
| ...
30+
s = box.schema.create_space('test', {is_sync = true})
31+
| ---
32+
| ...
33+
_ = s:create_index('pk')
34+
| ---
35+
| ...
36+
s2 = box.schema.create_space('test2')
37+
| ---
38+
| ...
39+
_ = s2:create_index('pk')
40+
| ---
41+
| ...
42+
43+
errinj = box.error.injection
44+
| ---
45+
| ...
46+
47+
function create_hanging_async_after_confirm(sync_key, async_key1, async_key2) \
48+
-- Let the transaction itself to WAL, but CONFIRM will be blocked. \
49+
errinj.set('ERRINJ_WAL_DELAY_COUNTDOWN', 1) \
50+
local lsn = box.info.lsn \
51+
local f = fiber.create(function() s:replace{sync_key} end) \
52+
test_run:wait_cond(function() return box.info.lsn == lsn + 1 end) \
53+
-- Wait for the CONFIRM block. WAL thread is in busy loop now. \
54+
test_run:wait_cond(function() return errinj.get('ERRINJ_WAL_DELAY') end) \
55+
\
56+
-- Create 2 async transactions to ensure multiple of them are handled fine. \
57+
-- But return only fiber of the second one. It is enough because if it is \
58+
-- finished, the first one is too. \
59+
fiber.new(function() s2:replace{async_key1} end) \
60+
local f2 = fiber.new(function() s2:replace{async_key2} end) \
61+
fiber.yield() \
62+
-- When WAL thread would finish CONFIRM, it should be blocked on the async \
63+
-- transaction so as it wouldn't be completed when CONFIRM is processed. \
64+
errinj.set('ERRINJ_WAL_DELAY_COUNTDOWN', 0) \
65+
-- Let CONFIRM go. \
66+
errinj.set('ERRINJ_WAL_DELAY', false) \
67+
-- And WAL thread should be blocked on the async txn. \
68+
test_run:wait_cond(function() return errinj.get('ERRINJ_WAL_DELAY') end) \
69+
-- Wait CONFIRM finish. \
70+
test_run:wait_cond(function() return f:status() == 'dead' end) \
71+
return f2 \
72+
end
73+
| ---
74+
| ...
75+
76+
async_f = create_hanging_async_after_confirm(1, 2, 3)
77+
| ---
78+
| ...
79+
-- Let the async transaction finish its WAL write.
80+
errinj.set('ERRINJ_WAL_DELAY', false)
81+
| ---
82+
| - ok
83+
| ...
84+
-- It should see that even though it is in the limbo, it does not have any
85+
-- synchronous transactions to wait for and can complete right away.
86+
test_run:wait_cond(function() return async_f:status() == 'dead' end)
87+
| ---
88+
| - true
89+
| ...
90+
91+
assert(s:get({1}) ~= nil)
92+
| ---
93+
| - true
94+
| ...
95+
assert(s2:get({2}) ~= nil)
96+
| ---
97+
| - true
98+
| ...
99+
assert(s2:get({3}) ~= nil)
100+
| ---
101+
| - true
102+
| ...
103+
104+
--
105+
-- Try all the same, but the async transaction fails its WAL write.
106+
--
107+
async_f = create_hanging_async_after_confirm(4, 5, 6)
108+
| ---
109+
| ...
110+
-- The async transaction will fail to go to WAL when WAL thread is unblocked.
111+
errinj.set('ERRINJ_WAL_ROTATE', true)
112+
| ---
113+
| - ok
114+
| ...
115+
errinj.set('ERRINJ_WAL_DELAY', false)
116+
| ---
117+
| - ok
118+
| ...
119+
test_run:wait_cond(function() return async_f:status() == 'dead' end)
120+
| ---
121+
| - true
122+
| ...
123+
errinj.set('ERRINJ_WAL_ROTATE', false)
124+
| ---
125+
| - ok
126+
| ...
127+
128+
assert(s:get({4}) ~= nil)
129+
| ---
130+
| - true
131+
| ...
132+
assert(s2:get({5}) == nil)
133+
| ---
134+
| - true
135+
| ...
136+
assert(s2:get({6}) == nil)
137+
| ---
138+
| - true
139+
| ...
140+
141+
-- Ensure nothing is broken, works fine.
142+
s:replace{7}
143+
| ---
144+
| - [7]
145+
| ...
146+
s2:replace{8}
147+
| ---
148+
| - [8]
149+
| ...
150+
151+
s:drop()
152+
| ---
153+
| ...
154+
s2:drop()
155+
| ---
156+
| ...
157+
158+
box.cfg{ \
159+
replication_synchro_quorum = old_synchro_quorum, \
160+
replication_synchro_timeout = old_synchro_timeout, \
161+
}
162+
| ---
163+
| ...

‎test/replication/gh-6057-qsync-confirm-async-no-wal.test.lua

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
test_run = require('test_run').new()
2+
fiber = require('fiber')
3+
4+
--
5+
-- gh-6057: while CONFIRM is being written, there might appear async
6+
-- transactions not having an LSN/signature yet. When CONFIRM is done, it covers
7+
-- these transactions too since they don't need to wait for an ACK, but it
8+
-- should not try to complete them. Because their WAL write is not done and
9+
-- it might even fail later. It should simply turn them into plain transactions
10+
-- not depending on any synchronous ones.
11+
--
12+
13+
old_synchro_quorum = box.cfg.replication_synchro_quorum
14+
old_synchro_timeout = box.cfg.replication_synchro_timeout
15+
box.cfg{ \
16+
replication_synchro_quorum = 1, \
17+
replication_synchro_timeout = 1000000, \
18+
}
19+
s = box.schema.create_space('test', {is_sync = true})
20+
_ = s:create_index('pk')
21+
s2 = box.schema.create_space('test2')
22+
_ = s2:create_index('pk')
23+
24+
errinj = box.error.injection
25+
26+
function create_hanging_async_after_confirm(sync_key, async_key1, async_key2) \
27+
-- Let the transaction itself to WAL, but CONFIRM will be blocked. \
28+
errinj.set('ERRINJ_WAL_DELAY_COUNTDOWN', 1) \
29+
local lsn = box.info.lsn \
30+
local f = fiber.create(function() s:replace{sync_key} end) \
31+
test_run:wait_cond(function() return box.info.lsn == lsn + 1 end) \
32+
-- Wait for the CONFIRM block. WAL thread is in busy loop now. \
33+
test_run:wait_cond(function() return errinj.get('ERRINJ_WAL_DELAY') end) \
34+
\
35+
-- Create 2 async transactions to ensure multiple of them are handled fine. \
36+
-- But return only fiber of the second one. It is enough because if it is \
37+
-- finished, the first one is too. \
38+
fiber.new(function() s2:replace{async_key1} end) \
39+
local f2 = fiber.new(function() s2:replace{async_key2} end) \
40+
fiber.yield() \
41+
-- When WAL thread would finish CONFIRM, it should be blocked on the async \
42+
-- transaction so as it wouldn't be completed when CONFIRM is processed. \
43+
errinj.set('ERRINJ_WAL_DELAY_COUNTDOWN', 0) \
44+
-- Let CONFIRM go. \
45+
errinj.set('ERRINJ_WAL_DELAY', false) \
46+
-- And WAL thread should be blocked on the async txn. \
47+
test_run:wait_cond(function() return errinj.get('ERRINJ_WAL_DELAY') end) \
48+
-- Wait CONFIRM finish. \
49+
test_run:wait_cond(function() return f:status() == 'dead' end) \
50+
return f2 \
51+
end
52+
53+
async_f = create_hanging_async_after_confirm(1, 2, 3)
54+
-- Let the async transaction finish its WAL write.
55+
errinj.set('ERRINJ_WAL_DELAY', false)
56+
-- It should see that even though it is in the limbo, it does not have any
57+
-- synchronous transactions to wait for and can complete right away.
58+
test_run:wait_cond(function() return async_f:status() == 'dead' end)
59+
60+
assert(s:get({1}) ~= nil)
61+
assert(s2:get({2}) ~= nil)
62+
assert(s2:get({3}) ~= nil)
63+
64+
--
65+
-- Try all the same, but the async transaction fails its WAL write.
66+
--
67+
async_f = create_hanging_async_after_confirm(4, 5, 6)
68+
-- The async transaction will fail to go to WAL when WAL thread is unblocked.
69+
errinj.set('ERRINJ_WAL_ROTATE', true)
70+
errinj.set('ERRINJ_WAL_DELAY', false)
71+
test_run:wait_cond(function() return async_f:status() == 'dead' end)
72+
errinj.set('ERRINJ_WAL_ROTATE', false)
73+
74+
assert(s:get({4}) ~= nil)
75+
assert(s2:get({5}) == nil)
76+
assert(s2:get({6}) == nil)
77+
78+
-- Ensure nothing is broken, works fine.
79+
s:replace{7}
80+
s2:replace{8}
81+
82+
s:drop()
83+
s2:drop()
84+
85+
box.cfg{ \
86+
replication_synchro_quorum = old_synchro_quorum, \
87+
replication_synchro_timeout = old_synchro_timeout, \
88+
}

‎test/replication/suite.cfg

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
"gh-5536-wal-limit.test.lua": {},
4747
"gh-5566-final-join-synchro.test.lua": {},
4848
"gh-6032-promote-wal-write.test.lua": {},
49+
"gh-6057-qsync-confirm-async-no-wal.test.lua": {},
4950
"*": {
5051
"memtx": {"engine": "memtx"},
5152
"vinyl": {"engine": "vinyl"}

‎test/replication/suite.ini

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ core = tarantool
33
script = master.lua
44
description = tarantool/box, replication
55
disabled = consistent.test.lua
6-
release_disabled = catch.test.lua errinj.test.lua gc.test.lua gc_no_space.test.lua before_replace.test.lua qsync_advanced.test.lua qsync_errinj.test.lua quorum.test.lua recover_missing_xlog.test.lua sync.test.lua long_row_timeout.test.lua gh-4739-vclock-assert.test.lua gh-4730-applier-rollback.test.lua gh-5140-qsync-casc-rollback.test.lua gh-5144-qsync-dup-confirm.test.lua gh-5167-qsync-rollback-snap.test.lua gh-5506-election-on-off.test.lua gh-5536-wal-limit.test.lua hang_on_synchro_fail.test.lua anon_register_gap.test.lua gh-5213-qsync-applier-order.test.lua gh-5213-qsync-applier-order-3.test.lua gh-6032-promote-wal-write.test.lua
6+
release_disabled = catch.test.lua errinj.test.lua gc.test.lua gc_no_space.test.lua before_replace.test.lua qsync_advanced.test.lua qsync_errinj.test.lua quorum.test.lua recover_missing_xlog.test.lua sync.test.lua long_row_timeout.test.lua gh-4739-vclock-assert.test.lua gh-4730-applier-rollback.test.lua gh-5140-qsync-casc-rollback.test.lua gh-5144-qsync-dup-confirm.test.lua gh-5167-qsync-rollback-snap.test.lua gh-5506-election-on-off.test.lua gh-5536-wal-limit.test.lua hang_on_synchro_fail.test.lua anon_register_gap.test.lua gh-5213-qsync-applier-order.test.lua gh-5213-qsync-applier-order-3.test.lua gh-6032-promote-wal-write.test.lua gh-6057-qsync-confirm-async-no-wal.test.lua
77
config = suite.cfg
88
lua_libs = lua/fast_replica.lua lua/rlimit.lua
99
use_unix_sockets = True

0 commit comments

Comments
 (0)