* [Tarantool-patches] [PATCH 1/3] txn_limbo: handle ROLLBACK during CONFIRM
2020-07-30 22:37 [Tarantool-patches] [PATCH 0/3] Qsync tricky crashes Vladislav Shpilevoy
@ 2020-07-30 22:37 ` Vladislav Shpilevoy
2020-07-30 22:37 ` [Tarantool-patches] [PATCH 2/3] txn_limbo: handle CONFIRM during ROLLBACK Vladislav Shpilevoy
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Vladislav Shpilevoy @ 2020-07-30 22:37 UTC (permalink / raw)
To: tarantool-patches, gorcunov
Limbo could try to ROLLBACK LSN whose CONFIRM is in progress. This
is how it could happen:
- A synchronous transaction is created, written to WAL;
- The fiber sleeps in the limbo waiting for CONFIRM or timeout;
- Replica receives the transaction, sends ACK;
- Master receives ACK, starts writing CONFIRM;
- The first fiber times out and tries to write ROLLBACK for the
LSN, whose CONFIRM is in progress right now.
The patch adds more checks to the 'timed out' code path to see if
it isn't too late to write ROLLBACK. If CONFIRM is in progress,
the fiber will wait for its finish.
Part of #5185
---
src/box/txn_limbo.c | 31 +++++++---
test/replication/qsync_errinj.result | 85 ++++++++++++++++++++++++++
test/replication/qsync_errinj.test.lua | 40 ++++++++++++
3 files changed, 147 insertions(+), 9 deletions(-)
diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index 27a7302c0..1602c5d16 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -200,25 +200,33 @@ txn_limbo_wait_complete(struct txn_limbo *limbo, struct txn_limbo_entry *entry)
if (txn_limbo_entry_is_complete(entry))
goto complete;
if (rc != 0)
- goto do_rollback;
+ break;
}
-do_rollback:
assert(!txn_limbo_is_empty(limbo));
if (txn_limbo_first_entry(limbo) != entry) {
- assert(limbo->is_in_rollback);
/*
* If this is not a first entry in the limbo, it
* is definitely not a first timed out entry. And
* since it managed to time out too, it means
* there is currently another fiber writing
- * rollback. Wait when it will finish and wake us
- * up.
+ * rollback, or waiting for confirmation WAL
+ * write. Wait when it will finish and wake us up.
*/
- do {
- fiber_yield();
- } while (!txn_limbo_entry_is_complete(entry));
- goto complete;
+ goto wait;
+ }
+
+ /* First in the queue is always a synchronous transaction. */
+ assert(entry->lsn > 0);
+ if (entry->lsn <= limbo->confirmed_lsn) {
+ /*
+ * Yes, the wait timed out, but there is an on-going CONFIRM WAL
+ * write in another fiber covering this LSN. Can't rollback it
+ * already. All what can be done is waiting. The CONFIRM writer
+ * will wakeup all the confirmed txns when WAL write will be
+ * finished.
+ */
+ goto wait;
}
txn_limbo_write_rollback(limbo, entry->lsn);
@@ -238,6 +246,11 @@ do_rollback:
diag_set(ClientError, ER_SYNC_QUORUM_TIMEOUT);
return -1;
+wait:
+ do {
+ fiber_yield();
+ } while (!txn_limbo_entry_is_complete(entry));
+
complete:
assert(txn_limbo_entry_is_complete(entry));
/*
diff --git a/test/replication/qsync_errinj.result b/test/replication/qsync_errinj.result
index 1c6e357f7..4b8977810 100644
--- a/test/replication/qsync_errinj.result
+++ b/test/replication/qsync_errinj.result
@@ -283,6 +283,91 @@ box.space.sync:select{}
| - - [2]
| ...
+--
+-- See if synchro timeout during CONFIRM WAL write won't try to rollback the
+-- confirmed transaction.
+--
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+box.space.sync:truncate()
+ | ---
+ | ...
+-- Write something to flush the master's state to the replica.
+_ = box.space.sync:insert({1})
+ | ---
+ | ...
+_ = box.space.sync:delete({1})
+ | ---
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+-- Set a trap for CONFIRM write so as the current txn won't hang, but following
+-- CONFIRM will.
+box.error.injection.set('ERRINJ_WAL_DELAY_COUNTDOWN', 1)
+ | ---
+ | - ok
+ | ...
+ok, err = nil
+ | ---
+ | ...
+f = fiber.create(function() \
+ ok, err = pcall(box.space.sync.replace, box.space.sync, {1}) \
+end)
+ | ---
+ | ...
+
+-- Wait when got ACK from replica and started writing CONFIRM.
+test_run:wait_cond(function() \
+ return box.error.injection.get("ERRINJ_WAL_DELAY") \
+end)
+ | ---
+ | - true
+ | ...
+
+-- Let the pending synchro transaction go. It should timeout, notice the
+-- on-going confirmation, and go to sleep instead of doing rollback.
+box.cfg{replication_synchro_timeout = 0.001}
+ | ---
+ | ...
+fiber.yield()
+ | ---
+ | ...
+
+-- Let the confirmation finish.
+box.error.injection.set("ERRINJ_WAL_DELAY", false)
+ | ---
+ | - ok
+ | ...
+
+-- Now the transaction sees the CONFIRM is ok, and it is also finished.
+test_run:wait_cond(function() return f:status() == 'dead' end)
+ | ---
+ | - true
+ | ...
+ok, err
+ | ---
+ | - true
+ | - [1]
+ | ...
+box.space.sync:select{}
+ | ---
+ | - - [1]
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+box.space.sync:select{}
+ | ---
+ | - - [1]
+ | ...
+
test_run:cmd('switch default')
| ---
| - true
diff --git a/test/replication/qsync_errinj.test.lua b/test/replication/qsync_errinj.test.lua
index da554c775..817b08a25 100644
--- a/test/replication/qsync_errinj.test.lua
+++ b/test/replication/qsync_errinj.test.lua
@@ -105,6 +105,46 @@ box.space.sync:replace{2}
test_run:switch('replica')
box.space.sync:select{}
+--
+-- See if synchro timeout during CONFIRM WAL write won't try to rollback the
+-- confirmed transaction.
+--
+test_run:switch('default')
+box.space.sync:truncate()
+-- Write something to flush the master's state to the replica.
+_ = box.space.sync:insert({1})
+_ = box.space.sync:delete({1})
+
+test_run:switch('default')
+-- Set a trap for CONFIRM write so as the current txn won't hang, but following
+-- CONFIRM will.
+box.error.injection.set('ERRINJ_WAL_DELAY_COUNTDOWN', 1)
+ok, err = nil
+f = fiber.create(function() \
+ ok, err = pcall(box.space.sync.replace, box.space.sync, {1}) \
+end)
+
+-- Wait when got ACK from replica and started writing CONFIRM.
+test_run:wait_cond(function() \
+ return box.error.injection.get("ERRINJ_WAL_DELAY") \
+end)
+
+-- Let the pending synchro transaction go. It should timeout, notice the
+-- on-going confirmation, and go to sleep instead of doing rollback.
+box.cfg{replication_synchro_timeout = 0.001}
+fiber.yield()
+
+-- Let the confirmation finish.
+box.error.injection.set("ERRINJ_WAL_DELAY", false)
+
+-- Now the transaction sees the CONFIRM is ok, and it is also finished.
+test_run:wait_cond(function() return f:status() == 'dead' end)
+ok, err
+box.space.sync:select{}
+
+test_run:switch('replica')
+box.space.sync:select{}
+
test_run:cmd('switch default')
box.cfg{ \
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH 2/3] txn_limbo: handle CONFIRM during ROLLBACK
2020-07-30 22:37 [Tarantool-patches] [PATCH 0/3] Qsync tricky crashes Vladislav Shpilevoy
2020-07-30 22:37 ` [Tarantool-patches] [PATCH 1/3] txn_limbo: handle ROLLBACK during CONFIRM Vladislav Shpilevoy
@ 2020-07-30 22:37 ` Vladislav Shpilevoy
2020-07-30 22:37 ` [Tarantool-patches] [PATCH 3/3] txn_limbo: handle duplicate ACKs Vladislav Shpilevoy
2020-07-31 13:59 ` [Tarantool-patches] [PATCH 0/3] Qsync tricky crashes Cyrill Gorcunov
3 siblings, 0 replies; 6+ messages in thread
From: Vladislav Shpilevoy @ 2020-07-30 22:37 UTC (permalink / raw)
To: tarantool-patches, gorcunov
Limbo could try to CONFIRM LSN whose ROLLBACK is in progress. This
is how it could happen:
- A synchronous transaction is created, written to WAL;
- The fiber sleeps in the limbo waiting for CONFIRM or timeout;
- Timeout happens. ROLLBACK for this and all next LSNs is sent to
WAL;
- Replica receives the transaction, sends ACK;
- Master receives ACK, starts writing CONFIRM for the LSN, whose
ROLLBACK is in progress right now.
Another case - attempt to lower synchro quorum during ROLLBACK
write. It also could try to write CONFIRM.
The patch skips CONFIRM if there is a ROLLBACK in progress. Not
even necessary to check LSNs. Because ROLLBACK always reverts the
entire limbo queue, so it will cancel all pending transactions
with all LSNs, and new commits are rolled back even before they
try to go to WAL. CONFIRM can't help here with anything already.
Part of #5185
---
src/box/txn_limbo.c | 15 ++-
test/replication/qsync_errinj.result | 147 +++++++++++++++++++++++++
test/replication/qsync_errinj.test.lua | 65 +++++++++++
3 files changed, 226 insertions(+), 1 deletion(-)
diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index 1602c5d16..e052b9003 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -336,6 +336,7 @@ static void
txn_limbo_write_confirm(struct txn_limbo *limbo, int64_t lsn)
{
assert(lsn > limbo->confirmed_lsn);
+ assert(!limbo->is_in_rollback);
limbo->confirmed_lsn = lsn;
txn_limbo_write_confirm_rollback(limbo, lsn, true);
}
@@ -438,6 +439,18 @@ txn_limbo_ack(struct txn_limbo *limbo, uint32_t replica_id, int64_t lsn)
{
if (rlist_empty(&limbo->queue))
return;
+ /*
+ * If limbo is currently writing a rollback, it means the the whole
+ * queue will be rolled back. Because rollback is written only for
+ * timeout. Timeout always happens first for the oldest entry, i.e.
+ * first entry in the queue. The rollback will clear all the newer
+ * entries. So in total the whole queue is dead already. Would be
+ * strange to write CONFIRM for rolled back LSNs. Even though
+ * probably it wouldn't break anything. Would be just 2 conflicting
+ * decisions for the same LSNs.
+ */
+ if (limbo->is_in_rollback)
+ return;
assert(limbo->instance_id != REPLICA_ID_NIL);
int64_t prev_lsn = vclock_get(&limbo->vclock, replica_id);
vclock_follow(&limbo->vclock, replica_id, lsn);
@@ -601,7 +614,7 @@ txn_limbo_on_parameters_change(struct txn_limbo *limbo)
assert(confirm_lsn > 0);
}
}
- if (confirm_lsn > limbo->confirmed_lsn) {
+ if (confirm_lsn > limbo->confirmed_lsn && !limbo->is_in_rollback) {
txn_limbo_write_confirm(limbo, confirm_lsn);
txn_limbo_read_confirm(limbo, confirm_lsn);
}
diff --git a/test/replication/qsync_errinj.result b/test/replication/qsync_errinj.result
index 4b8977810..635bcf939 100644
--- a/test/replication/qsync_errinj.result
+++ b/test/replication/qsync_errinj.result
@@ -368,6 +368,153 @@ box.space.sync:select{}
| - - [1]
| ...
+--
+-- See what happens when the quorum is collected during writing ROLLBACK.
+-- CONFIRM for the same LSN should not be written.
+--
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+box.cfg{replication_synchro_timeout = 1000, replication_synchro_quorum = 2}
+ | ---
+ | ...
+box.space.sync:truncate()
+ | ---
+ | ...
+-- Write something to flush the master's state to the replica.
+_ = box.space.sync:insert({1})
+ | ---
+ | ...
+_ = box.space.sync:delete({1})
+ | ---
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+-- Block WAL write to block ACK sending.
+box.error.injection.set("ERRINJ_WAL_DELAY", true)
+ | ---
+ | - ok
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+-- Set a trap for ROLLBACK write so as the txn itself won't hang, but ROLLBACK
+-- will.
+box.error.injection.set('ERRINJ_WAL_DELAY_COUNTDOWN', 1)
+ | ---
+ | - ok
+ | ...
+box.cfg{replication_synchro_timeout = 0.001}
+ | ---
+ | ...
+lsn = box.info.lsn
+ | ---
+ | ...
+ok, err = nil
+ | ---
+ | ...
+f = fiber.create(function() \
+ ok, err = pcall(box.space.sync.replace, box.space.sync, {1}) \
+end)
+ | ---
+ | ...
+-- Wait ROLLBACK WAL write start.
+test_run:wait_cond(function() \
+ return box.error.injection.get("ERRINJ_WAL_DELAY") \
+end)
+ | ---
+ | - true
+ | ...
+-- The transaction is written to WAL. ROLLBACK is not yet.
+lsn = lsn + 1
+ | ---
+ | ...
+assert(box.info.lsn == lsn)
+ | ---
+ | - true
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+-- Let ACKs go. Master will receive ACK, but shouldn't try to CONFIRM. Because
+-- ROLLBACK for the same LSN is in progress right now already.
+box.error.injection.set("ERRINJ_WAL_DELAY", false)
+ | ---
+ | - ok
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+-- Wait ACK receipt.
+function wait_lsn_ack(id, lsn) \
+ local this_id = box.info.id \
+ test_run:wait_downstream(id, {status='follow'}) \
+ test_run:wait_cond(function() \
+ return box.info.replication[id].downstream.vclock[this_id] >= lsn \
+ end) \
+end
+ | ---
+ | ...
+replica_id = test_run:get_server_id('replica')
+ | ---
+ | ...
+wait_lsn_ack(replica_id, lsn)
+ | ---
+ | ...
+
+-- See if parameters change will try to write CONFIRM.
+box.cfg{replication_synchro_quorum = 1}
+ | ---
+ | ...
+box.cfg{replication_synchro_quorum = 2}
+ | ---
+ | ...
+
+-- Let ROLLBACK go and finish the test.
+box.error.injection.set("ERRINJ_WAL_DELAY", false)
+ | ---
+ | - ok
+ | ...
+test_run:wait_cond(function() return f:status() == 'dead' end)
+ | ---
+ | - true
+ | ...
+ok, err
+ | ---
+ | - false
+ | - Quorum collection for a synchronous transaction is timed out
+ | ...
+box.cfg{replication_synchro_timeout = 1000}
+ | ---
+ | ...
+box.space.sync:replace{2}
+ | ---
+ | - [2]
+ | ...
+box.space.sync:select{}
+ | ---
+ | - - [2]
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+box.space.sync:select{}
+ | ---
+ | - - [2]
+ | ...
+
test_run:cmd('switch default')
| ---
| - true
diff --git a/test/replication/qsync_errinj.test.lua b/test/replication/qsync_errinj.test.lua
index 817b08a25..6a9fd3e1a 100644
--- a/test/replication/qsync_errinj.test.lua
+++ b/test/replication/qsync_errinj.test.lua
@@ -145,6 +145,71 @@ box.space.sync:select{}
test_run:switch('replica')
box.space.sync:select{}
+--
+-- See what happens when the quorum is collected during writing ROLLBACK.
+-- CONFIRM for the same LSN should not be written.
+--
+test_run:switch('default')
+box.cfg{replication_synchro_timeout = 1000, replication_synchro_quorum = 2}
+box.space.sync:truncate()
+-- Write something to flush the master's state to the replica.
+_ = box.space.sync:insert({1})
+_ = box.space.sync:delete({1})
+
+test_run:switch('replica')
+-- Block WAL write to block ACK sending.
+box.error.injection.set("ERRINJ_WAL_DELAY", true)
+
+test_run:switch('default')
+-- Set a trap for ROLLBACK write so as the txn itself won't hang, but ROLLBACK
+-- will.
+box.error.injection.set('ERRINJ_WAL_DELAY_COUNTDOWN', 1)
+box.cfg{replication_synchro_timeout = 0.001}
+lsn = box.info.lsn
+ok, err = nil
+f = fiber.create(function() \
+ ok, err = pcall(box.space.sync.replace, box.space.sync, {1}) \
+end)
+-- Wait ROLLBACK WAL write start.
+test_run:wait_cond(function() \
+ return box.error.injection.get("ERRINJ_WAL_DELAY") \
+end)
+-- The transaction is written to WAL. ROLLBACK is not yet.
+lsn = lsn + 1
+assert(box.info.lsn == lsn)
+
+test_run:switch('replica')
+-- Let ACKs go. Master will receive ACK, but shouldn't try to CONFIRM. Because
+-- ROLLBACK for the same LSN is in progress right now already.
+box.error.injection.set("ERRINJ_WAL_DELAY", false)
+
+test_run:switch('default')
+-- Wait ACK receipt.
+function wait_lsn_ack(id, lsn) \
+ local this_id = box.info.id \
+ test_run:wait_downstream(id, {status='follow'}) \
+ test_run:wait_cond(function() \
+ return box.info.replication[id].downstream.vclock[this_id] >= lsn \
+ end) \
+end
+replica_id = test_run:get_server_id('replica')
+wait_lsn_ack(replica_id, lsn)
+
+-- See if parameters change will try to write CONFIRM.
+box.cfg{replication_synchro_quorum = 1}
+box.cfg{replication_synchro_quorum = 2}
+
+-- Let ROLLBACK go and finish the test.
+box.error.injection.set("ERRINJ_WAL_DELAY", false)
+test_run:wait_cond(function() return f:status() == 'dead' end)
+ok, err
+box.cfg{replication_synchro_timeout = 1000}
+box.space.sync:replace{2}
+box.space.sync:select{}
+
+test_run:switch('replica')
+box.space.sync:select{}
+
test_run:cmd('switch default')
box.cfg{ \
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH 3/3] txn_limbo: handle duplicate ACKs
2020-07-30 22:37 [Tarantool-patches] [PATCH 0/3] Qsync tricky crashes Vladislav Shpilevoy
2020-07-30 22:37 ` [Tarantool-patches] [PATCH 1/3] txn_limbo: handle ROLLBACK during CONFIRM Vladislav Shpilevoy
2020-07-30 22:37 ` [Tarantool-patches] [PATCH 2/3] txn_limbo: handle CONFIRM during ROLLBACK Vladislav Shpilevoy
@ 2020-07-30 22:37 ` Vladislav Shpilevoy
2020-07-31 13:59 ` [Tarantool-patches] [PATCH 0/3] Qsync tricky crashes Cyrill Gorcunov
3 siblings, 0 replies; 6+ messages in thread
From: Vladislav Shpilevoy @ 2020-07-30 22:37 UTC (permalink / raw)
To: tarantool-patches, gorcunov
Replica can send the same ACK multiple times. This is relatively
easy to achieve.
ACK is a message form the replica containing its vclock. It is
sent on each replica's vclock update. The update not necessarily
means that master's LSN was changed. Replica could write
something locally, with its own instance_id. Vclock is changed,
sent to the master, but from the limbo's point of view it looks
like duplicated ACK, because the received master's LSN didn't
change.
The patch makes limbo ignore duplicated ACKs.
Closes #5195
Part of #5219
---
src/box/txn_limbo.c | 9 +
.../gh-5195-qsync-replica-write.result | 156 ++++++++++++++++++
.../gh-5195-qsync-replica-write.test.lua | 68 ++++++++
3 files changed, 233 insertions(+)
create mode 100644 test/replication/gh-5195-qsync-replica-write.result
create mode 100644 test/replication/gh-5195-qsync-replica-write.test.lua
diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index e052b9003..0ab387392 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -453,6 +453,15 @@ txn_limbo_ack(struct txn_limbo *limbo, uint32_t replica_id, int64_t lsn)
return;
assert(limbo->instance_id != REPLICA_ID_NIL);
int64_t prev_lsn = vclock_get(&limbo->vclock, replica_id);
+ /*
+ * One of the reasons why can happen - the remote instance is not
+ * read-only and wrote something under its own insance_id. For qsync
+ * that most likely means that the remote instance decided to take over
+ * the limbo ownership, and the current node is going to become a
+ * replica very soon.
+ */
+ if (lsn == prev_lsn)
+ return;
vclock_follow(&limbo->vclock, replica_id, lsn);
struct txn_limbo_entry *e;
int64_t confirm_lsn = -1;
diff --git a/test/replication/gh-5195-qsync-replica-write.result b/test/replication/gh-5195-qsync-replica-write.result
new file mode 100644
index 000000000..3999e8f5e
--- /dev/null
+++ b/test/replication/gh-5195-qsync-replica-write.result
@@ -0,0 +1,156 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+fiber = require('fiber')
+ | ---
+ | ...
+engine = test_run:get_cfg('engine')
+ | ---
+ | ...
+old_synchro_quorum = box.cfg.replication_synchro_quorum
+ | ---
+ | ...
+old_synchro_timeout = box.cfg.replication_synchro_timeout
+ | ---
+ | ...
+
+box.schema.user.grant('guest', 'super')
+ | ---
+ | ...
+
+test_run:cmd('create server replica with rpl_master=default,\
+ script="replication/replica.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica with wait=True, wait_load=True')
+ | ---
+ | - true
+ | ...
+
+--
+-- gh-5195: WAL write on a read-write replica should not crash master. Limbo
+-- should be ready that ACKs can contain the same LSN from the same replica
+-- multiple times.
+--
+_ = box.schema.space.create('sync', {engine = engine, is_sync = true})
+ | ---
+ | ...
+_ = box.space.sync:create_index('pk')
+ | ---
+ | ...
+
+box.cfg{replication_synchro_timeout = 1000, replication_synchro_quorum = 3}
+ | ---
+ | ...
+lsn = box.info.lsn
+ | ---
+ | ...
+ok, err = nil
+ | ---
+ | ...
+f = fiber.create(function() \
+ ok, err = pcall(box.space.sync.insert, box.space.sync, {2}) \
+end)
+ | ---
+ | ...
+lsn = lsn + 1
+ | ---
+ | ...
+test_run:wait_cond(function() return box.info.lsn == lsn end)
+ | ---
+ | - true
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+test_run:wait_lsn('replica', 'default')
+ | ---
+ | ...
+-- Normal DML is blocked - the limbo is not empty and does not belong to the
+-- replica. But synchro queue cleanup also does a WAL write, and propagates LSN
+-- of the instance.
+box.cfg{replication_synchro_timeout = 0.001}
+ | ---
+ | ...
+box.ctl.clear_synchro_queue()
+ | ---
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+-- Wait second ACK receipt.
+replica_id = test_run:get_server_id('replica')
+ | ---
+ | ...
+replica_lsn = test_run:get_lsn('replica', replica_id)
+ | ---
+ | ...
+test_run:wait_downstream(replica_id, {status='follow'})
+ | ---
+ | - true
+ | ...
+test_run:wait_cond(function() \
+ local info = box.info.replication[replica_id] \
+ local lsn = info.downstream.vclock[replica_id] \
+ return lsn and lsn >= replica_lsn \
+end) \
+
+box.cfg{replication_synchro_quorum = 2}
+ | ---
+ | ...
+test_run:wait_cond(function() return f:status() == 'dead' end)
+ | ---
+ | - true
+ | ...
+ok, err
+ | ---
+ | - true
+ | - [2]
+ | ...
+box.space.sync:select{}
+ | ---
+ | - - [2]
+ | ...
+
+test_run:switch('replica')
+ | ---
+ | - true
+ | ...
+box.space.sync:select{}
+ | ---
+ | - - [2]
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('stop server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server replica')
+ | ---
+ | - true
+ | ...
+
+box.space.sync:drop()
+ | ---
+ | ...
+box.schema.user.revoke('guest', 'super')
+ | ---
+ | ...
+
+box.cfg{ \
+ replication_synchro_quorum = old_synchro_quorum, \
+ replication_synchro_timeout = old_synchro_timeout, \
+}
+ | ---
+ | ...
diff --git a/test/replication/gh-5195-qsync-replica-write.test.lua b/test/replication/gh-5195-qsync-replica-write.test.lua
new file mode 100644
index 000000000..2b9909e21
--- /dev/null
+++ b/test/replication/gh-5195-qsync-replica-write.test.lua
@@ -0,0 +1,68 @@
+test_run = require('test_run').new()
+fiber = require('fiber')
+engine = test_run:get_cfg('engine')
+old_synchro_quorum = box.cfg.replication_synchro_quorum
+old_synchro_timeout = box.cfg.replication_synchro_timeout
+
+box.schema.user.grant('guest', 'super')
+
+test_run:cmd('create server replica with rpl_master=default,\
+ script="replication/replica.lua"')
+test_run:cmd('start server replica with wait=True, wait_load=True')
+
+--
+-- gh-5195: WAL write on a read-write replica should not crash master. Limbo
+-- should be ready that ACKs can contain the same LSN from the same replica
+-- multiple times.
+--
+_ = box.schema.space.create('sync', {engine = engine, is_sync = true})
+_ = box.space.sync:create_index('pk')
+
+box.cfg{replication_synchro_timeout = 1000, replication_synchro_quorum = 3}
+lsn = box.info.lsn
+ok, err = nil
+f = fiber.create(function() \
+ ok, err = pcall(box.space.sync.insert, box.space.sync, {2}) \
+end)
+lsn = lsn + 1
+test_run:wait_cond(function() return box.info.lsn == lsn end)
+
+test_run:switch('replica')
+test_run:wait_lsn('replica', 'default')
+-- Normal DML is blocked - the limbo is not empty and does not belong to the
+-- replica. But synchro queue cleanup also does a WAL write, and propagates LSN
+-- of the instance.
+box.cfg{replication_synchro_timeout = 0.001}
+box.ctl.clear_synchro_queue()
+
+test_run:switch('default')
+-- Wait second ACK receipt.
+replica_id = test_run:get_server_id('replica')
+replica_lsn = test_run:get_lsn('replica', replica_id)
+test_run:wait_downstream(replica_id, {status='follow'})
+test_run:wait_cond(function() \
+ local info = box.info.replication[replica_id] \
+ local lsn = info.downstream.vclock[replica_id] \
+ return lsn and lsn >= replica_lsn \
+end) \
+
+box.cfg{replication_synchro_quorum = 2}
+test_run:wait_cond(function() return f:status() == 'dead' end)
+ok, err
+box.space.sync:select{}
+
+test_run:switch('replica')
+box.space.sync:select{}
+
+test_run:switch('default')
+
+test_run:cmd('stop server replica')
+test_run:cmd('delete server replica')
+
+box.space.sync:drop()
+box.schema.user.revoke('guest', 'super')
+
+box.cfg{ \
+ replication_synchro_quorum = old_synchro_quorum, \
+ replication_synchro_timeout = old_synchro_timeout, \
+}
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/3] Qsync tricky crashes
2020-07-30 22:37 [Tarantool-patches] [PATCH 0/3] Qsync tricky crashes Vladislav Shpilevoy
` (2 preceding siblings ...)
2020-07-30 22:37 ` [Tarantool-patches] [PATCH 3/3] txn_limbo: handle duplicate ACKs Vladislav Shpilevoy
@ 2020-07-31 13:59 ` Cyrill Gorcunov
2020-07-31 22:31 ` Vladislav Shpilevoy
3 siblings, 1 reply; 6+ messages in thread
From: Cyrill Gorcunov @ 2020-07-31 13:59 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
On Fri, Jul 31, 2020 at 12:37:42AM +0200, Vladislav Shpilevoy wrote:
> The patchset fixes 2 crashes in qsync. 5185 wasn't reproduced, but a similar
> test gave another crash, fixed here.
>
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-5185-5195-qsync-crashes
> Issue: https://github.com/tarantool/tarantool/issues/5185
> Issue: https://github.com/tarantool/tarantool/issues/5195
>
> @ChangeLog
> * Fixed a crash when replica cleared synchronous transaction queue, while it was not empty on master (gh-5195).
> * Fixed a crash when synchronous transaction rollback and confirm could be written simultaneously for the same LSN (gh-5185).
Ack. Lets give them a spin.
^ permalink raw reply [flat|nested] 6+ messages in thread