From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergepetrenko@tarantool.org Subject: [Tarantool-patches] [PATCH 13/13] txn: stop TXN_SIGNATURE_ABORT override Date: Fri, 11 Jun 2021 23:56:09 +0200 [thread overview] Message-ID: <6ef0d84375576f94283e2bf1fea56a55daf9523e.1623448465.git.v.shpilevoy@tarantool.org> (raw) In-Reply-To: <cover.1623448465.git.v.shpilevoy@tarantool.org> When txn_commit/try_async() failed before going to WAL thread, they installed TXN_SIGNATURE_ABORT signature meaning that the caller and the rollback triggers must look at the global diag. But they called txn_rollback() before doing return and calling the triggers, which overrode the signature with TXN_SIGNATURE_ROLLBACK leading to the original error loss. The patch makes TXN_SIGNATURE_ROLLBACK installed only when a real rollback happens (via box_txn_rollback()). This makes the original commit errors like a conflict in the transaction manager and OOM not lost. Besides, ERRINJ_TXN_COMMIT_ASYNC does not need its own diag_log() anymore. Because since this commit the applier logs the correct error instead of ER_WAL_IO/ER_TXN_ROLLBACK. Closes #6027 --- .../unreleased/gh-6027-applier-lost-error.md | 7 ++ src/box/txn.c | 10 ++- .../gh-6027-applier-error-show.result | 82 +++++++++++++++++++ .../gh-6027-applier-error-show.test.lua | 31 +++++++ test/replication/suite.cfg | 1 + test/replication/suite.ini | 2 +- 6 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/gh-6027-applier-lost-error.md create mode 100644 test/replication/gh-6027-applier-error-show.result create mode 100644 test/replication/gh-6027-applier-error-show.test.lua diff --git a/changelogs/unreleased/gh-6027-applier-lost-error.md b/changelogs/unreleased/gh-6027-applier-lost-error.md new file mode 100644 index 000000000..9c765b8e2 --- /dev/null +++ b/changelogs/unreleased/gh-6027-applier-lost-error.md @@ -0,0 +1,7 @@ +## bugfix/replication + +* When an error happened during appliance of a transaction received from a + remote instance via replication, it was always reported as "Failed to write + to disk" regardless of what really happened. Now the correct error is shown. + For example, "Out of memory", or "Transaction has been aborted by conflict", + and so on (gh-6027). diff --git a/src/box/txn.c b/src/box/txn.c index 5cae7b41d..c2734c237 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -805,7 +805,6 @@ txn_commit_try_async(struct txn *txn) * Log it for the testing sake: we grep * output to mark this event. */ - diag_log(); goto rollback; }); @@ -983,11 +982,11 @@ void txn_rollback(struct txn *txn) { assert(txn == in_txn()); + assert(txn->signature != TXN_SIGNATURE_UNKNOWN); txn->status = TXN_ABORTED; trigger_clear(&txn->fiber_on_stop); if (!txn_has_flag(txn, TXN_CAN_YIELD)) trigger_clear(&txn->fiber_on_yield); - txn->signature = TXN_SIGNATURE_ROLLBACK; txn_complete_fail(txn); fiber_set_txn(fiber(), NULL); } @@ -1086,6 +1085,8 @@ box_txn_rollback(void) diag_set(ClientError, ER_ROLLBACK_IN_SUB_STMT); return -1; } + assert(txn->signature == TXN_SIGNATURE_UNKNOWN); + txn->signature = TXN_SIGNATURE_ROLLBACK; txn_rollback(txn); /* doesn't throw */ fiber_gc(); return 0; @@ -1221,7 +1222,10 @@ txn_on_stop(struct trigger *trigger, void *event) { (void) trigger; (void) event; - txn_rollback(in_txn()); /* doesn't yield or fail */ + struct txn *txn = in_txn(); + assert(txn->signature == TXN_SIGNATURE_UNKNOWN); + txn->signature = TXN_SIGNATURE_ROLLBACK; + txn_rollback(txn); fiber_gc(); return 0; } diff --git a/test/replication/gh-6027-applier-error-show.result b/test/replication/gh-6027-applier-error-show.result new file mode 100644 index 000000000..c3a01ab50 --- /dev/null +++ b/test/replication/gh-6027-applier-error-show.result @@ -0,0 +1,82 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... + +-- +-- gh-6027: on attempt to a commit transaction its original error was lost. +-- + +box.schema.user.grant('guest', 'super') + | --- + | ... +s = box.schema.create_space('test') + | --- + | ... +_ = s:create_index('pk') + | --- + | ... + +test_run:cmd('create server replica with rpl_master=default, '.. \ + 'script="replication/replica.lua"') + | --- + | - true + | ... +test_run:cmd('start server replica') + | --- + | - true + | ... + +test_run:switch('replica') + | --- + | - true + | ... +box.error.injection.set('ERRINJ_TXN_COMMIT_ASYNC', true) + | --- + | - ok + | ... + +test_run:switch('default') + | --- + | - true + | ... +_ = s:replace{1} + | --- + | ... + +test_run:switch('replica') + | --- + | - true + | ... +test_run:wait_upstream(1, {status = 'stopped'}) + | --- + | - true + | ... +-- Should be something about error injection. +box.info.replication[1].upstream.message + | --- + | - Error injection 'txn commit async injection' + | ... + +test_run:switch('default') + | --- + | - true + | ... +test_run:cmd('stop server replica') + | --- + | - true + | ... +test_run:cmd('delete server replica') + | --- + | - true + | ... +box.error.injection.set('ERRINJ_TXN_COMMIT_ASYNC', false) + | --- + | - ok + | ... +s:drop() + | --- + | ... +box.schema.user.revoke('guest', 'super') + | --- + | ... diff --git a/test/replication/gh-6027-applier-error-show.test.lua b/test/replication/gh-6027-applier-error-show.test.lua new file mode 100644 index 000000000..8e17cdfa9 --- /dev/null +++ b/test/replication/gh-6027-applier-error-show.test.lua @@ -0,0 +1,31 @@ +test_run = require('test_run').new() + +-- +-- gh-6027: on attempt to a commit transaction its original error was lost. +-- + +box.schema.user.grant('guest', 'super') +s = box.schema.create_space('test') +_ = s:create_index('pk') + +test_run:cmd('create server replica with rpl_master=default, '.. \ + 'script="replication/replica.lua"') +test_run:cmd('start server replica') + +test_run:switch('replica') +box.error.injection.set('ERRINJ_TXN_COMMIT_ASYNC', true) + +test_run:switch('default') +_ = s:replace{1} + +test_run:switch('replica') +test_run:wait_upstream(1, {status = 'stopped'}) +-- Should be something about error injection. +box.info.replication[1].upstream.message + +test_run:switch('default') +test_run:cmd('stop server replica') +test_run:cmd('delete server replica') +box.error.injection.set('ERRINJ_TXN_COMMIT_ASYNC', false) +s:drop() +box.schema.user.revoke('guest', 'super') diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg index 9eccf9e19..3a0a8649f 100644 --- a/test/replication/suite.cfg +++ b/test/replication/suite.cfg @@ -45,6 +45,7 @@ "gh-5536-wal-limit.test.lua": {}, "gh-5566-final-join-synchro.test.lua": {}, "gh-5613-bootstrap-prefer-booted.test.lua": {}, + "gh-6027-applier-error-show.test.lua": {}, "gh-6032-promote-wal-write.test.lua": {}, "gh-6057-qsync-confirm-async-no-wal.test.lua": {}, "gh-6094-rs-uuid-mismatch.test.lua": {}, diff --git a/test/replication/suite.ini b/test/replication/suite.ini index 80e968d56..ccf3559df 100644 --- a/test/replication/suite.ini +++ b/test/replication/suite.ini @@ -3,7 +3,7 @@ core = tarantool script = master.lua description = tarantool/box, replication disabled = consistent.test.lua -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 +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-6027-applier-error-show.test.lua gh-6032-promote-wal-write.test.lua gh-6057-qsync-confirm-async-no-wal.test.lua config = suite.cfg lua_libs = lua/fast_replica.lua lua/rlimit.lua use_unix_sockets = True -- 2.24.3 (Apple Git-128)
next prev parent reply other threads:[~2021-06-11 21:59 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-11 21:56 [Tarantool-patches] [PATCH 00/13] Applier rollback reason Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 01/13] error: introduce ER_CASCADE_ROLLBACK Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 10/13] txn: install proper diag errors on txn fail Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 11/13] wal: introduce JOURNAL_ENTRY_ERR_CASCADE Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 12/13] txn: introduce TXN_SIGNATURE_ABORT Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` Vladislav Shpilevoy via Tarantool-patches [this message] 2021-06-15 13:44 ` [Tarantool-patches] [PATCH 13/13] txn: stop TXN_SIGNATURE_ABORT override Serge Petrenko via Tarantool-patches 2021-06-15 19:34 ` Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 02/13] test: remove replica-applier-rollback.lua Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 03/13] journal: make journal_write() set diag on error Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 04/13] wal: refactor wal_write_to_disk() Vladislav Shpilevoy via Tarantool-patches 2021-06-15 20:46 ` Cyrill Gorcunov via Tarantool-patches 2021-06-16 6:22 ` Vladislav Shpilevoy via Tarantool-patches 2021-06-16 8:02 ` Cyrill Gorcunov via Tarantool-patches 2021-06-16 23:32 ` Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 05/13] diag: introduce diag_set_detailed() Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 06/13] wal: encapsulate ER_WAL_IO Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 07/13] txn: change limbo rollback check in the trigger Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 08/13] journal: introduce proper error codes Vladislav Shpilevoy via Tarantool-patches 2021-06-11 21:56 ` [Tarantool-patches] [PATCH 09/13] txn: assert after WAL write that txn is not done Vladislav Shpilevoy via Tarantool-patches 2021-06-15 13:43 ` [Tarantool-patches] [PATCH 00/13] Applier rollback reason Serge Petrenko via Tarantool-patches 2021-06-16 23:32 ` Vladislav Shpilevoy via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=6ef0d84375576f94283e2bf1fea56a55daf9523e.1623448465.git.v.shpilevoy@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=gorcunov@gmail.com \ --cc=sergepetrenko@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 13/13] txn: stop TXN_SIGNATURE_ABORT override' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox