[Tarantool-patches] [PATCH 1/1] qsync: handle async txns right during CONFIRM
Serge Petrenko
sergepetrenko at tarantool.org
Fri May 28 12:01:17 MSK 2021
28.05.2021 00:28, Vladislav Shpilevoy пишет:
> 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 #6057
Hi! Thanks for working on this and for the fast fix!
Please find one comment below.
> ---
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-6057-confirm-async-no-wal
> Issue: https://github.com/tarantool/tarantool/issues/6057
>
> .../gh-6057-qsync-confirm-async-no-wal.md | 5 +
> src/box/txn.c | 14 +-
> src/box/txn_limbo.c | 21 +++
> .../gh-6057-qsync-confirm-async-no-wal.result | 163 ++++++++++++++++++
> ...h-6057-qsync-confirm-async-no-wal.test.lua | 88 ++++++++++
> test/replication/suite.cfg | 1 +
> test/replication/suite.ini | 2 +-
> 7 files changed, 289 insertions(+), 5 deletions(-)
> create mode 100644 changelogs/unreleased/gh-6057-qsync-confirm-async-no-wal.md
> create mode 100644 test/replication/gh-6057-qsync-confirm-async-no-wal.result
> create mode 100644 test/replication/gh-6057-qsync-confirm-async-no-wal.test.lua
>
> diff --git a/changelogs/unreleased/gh-6057-qsync-confirm-async-no-wal.md b/changelogs/unreleased/gh-6057-qsync-confirm-async-no-wal.md
> new file mode 100644
> index 000000000..1005389d8
> --- /dev/null
> +++ b/changelogs/unreleased/gh-6057-qsync-confirm-async-no-wal.md
> @@ -0,0 +1,5 @@
> +## bugfix/replication
> +
> +* Fixed a possible crash when a synchronous transaction was followed by an
> + asynchronous transaction right when its confirmation was being written
> + (gh-6057).
> diff --git a/src/box/txn.c b/src/box/txn.c
> index 1d42c9113..3d4d5c397 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -880,8 +880,14 @@ txn_commit(struct txn *txn)
> if (req == NULL)
> goto rollback;
>
> - bool is_sync = txn_has_flag(txn, TXN_WAIT_SYNC);
> - if (is_sync) {
> + /*
> + * Do not cash the flag value in a variable. The flag might be deleted
> + * during WAL write. This can happen for async transactions created
> + * during CONFIRM write, whose all blocking sync transactions get
> + * confirmed. They they turn the async transaction into just a plain
> + * txn not waiting for anything.
> + */
> + if (txn_has_flag(txn, TXN_WAIT_SYNC)) {
> /*
> * Remote rows, if any, come before local rows, so
> * check for originating instance id here.
> @@ -900,13 +906,13 @@ txn_commit(struct txn *txn)
>
> fiber_set_txn(fiber(), NULL);
> if (journal_write(req) != 0 || req->res < 0) {
> - if (is_sync)
> + if (txn_has_flag(txn, TXN_WAIT_SYNC))
> txn_limbo_abort(&txn_limbo, limbo_entry);
> diag_set(ClientError, ER_WAL_IO);
> diag_log();
> goto rollback;
> }
> - if (is_sync) {
> + if (txn_has_flag(txn, TXN_WAIT_SYNC)) {
> if (txn_has_flag(txn, TXN_WAIT_ACK)) {
> int64_t lsn = req->rows[req->n_rows - 1]->lsn;
> /*
> diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
> index f287369a2..05f0bf30a 100644
> --- a/src/box/txn_limbo.c
> +++ b/src/box/txn_limbo.c
> @@ -389,6 +389,27 @@ txn_limbo_read_confirm(struct txn_limbo *limbo, int64_t lsn)
> */
> if (e->lsn == -1)
> break;
> + } else if (e->txn->signature < 0) {
> + /*
> + * A transaction might be covered by the CONFIRM even if
> + * it is not written to WAL yet when it is an async
> + * transaction. It could be created just when the
> + * CONFIRM was being written to WAL.
> + */
> + assert(e->txn->status == TXN_PREPARED);
> + /*
> + * Let it complete normally as a plain transaction.
> + */
> + txn_clear_flags(e->txn, TXN_WAIT_SYNC | TXN_WAIT_ACK);
AFAICS it's enough to clear WAIT_SYNC here.
Asynchronous transactions never have WAIT_ACK set, do they?
> + txn_limbo_remove(limbo, e);
> + /*
> + * The limbo entry now should not be used by the owner
> + * transaction since it just became a plain one. Nullify
> + * the txn to get a crash on any usage attempt instead
> + * of potential undefined behaviour.
> + */
> + e->txn = NULL;
> + continue;
> }
> e->is_commit = true;
> txn_limbo_remove(limbo, e);
--
Serge Petrenko
More information about the Tarantool-patches
mailing list