From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 066BB6EC55; Tue, 15 Jun 2021 16:44:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 066BB6EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1623764650; bh=O3InyCv1aIQkm2vlfCXhsPEMnBtizrEvxSOaGYvlDmk=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=RlMwd4jj4bFtyk6tUJwbQkJnGR0du2hjEIH+m3i1wyaSFjMnN2TcG976pSwscDFct N2d+c77QbHXuwoqae3H5vw29ZmLD8IXAYCCbH8qLmcmSFczsRF5JsievzMOIXfdAOI ijUDfQTo1ClQLvyv1PPwM4U0+zZBg668L/Gx1RXA= Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [94.100.177.102]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 154106EC55 for ; Tue, 15 Jun 2021 16:44:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 154106EC55 Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1lt9MO-0005Tp-7Y; Tue, 15 Jun 2021 16:44:08 +0300 To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org, gorcunov@gmail.com References: <6ef0d84375576f94283e2bf1fea56a55daf9523e.1623448465.git.v.shpilevoy@tarantool.org> Message-ID: Date: Tue, 15 Jun 2021 16:44:07 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <6ef0d84375576f94283e2bf1fea56a55daf9523e.1623448465.git.v.shpilevoy@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D5B0DA836B685C54907A7AE9C1BA82BC67C1327DFB87C6A6182A05F5380850408A1E87374367890AED44F91E22D80EF8E12F891200F35DF5B490003E1ACB388C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7D52C0B46024FCC4BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637095DDD168D16E65E8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8DACD8BB9ACD409FD8C4A517236B7FE17117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE140C956E756FBB7AA9D86F9317F2E7ACD8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE386A7C529F68B8E5C9735652A29929C6CC4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637571EAFDAACF43475EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A24209795067102C07E8F7B195E1C978317D4FA9AC0045A223DEE0BF9F2B90C42B X-C1DE0DAB: 0D63561A33F958A5F66ADB5B566A41358CF7EF17A74688F865C2E607CA77D0AAD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75FBC5FED0552DA851410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34BC3EEE75EF3BACCFBA6BE71E37FDE48464CC417BA11B9101F0DF5ECD25DBD745D260E697718A7A4B1D7E09C32AA3244CEB87C5FA164C75E772500A1663AACC329CA7333006C390A0FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj6OL1iHTyIM2xlAsyqXszzA== X-Mailru-Sender: 583F1D7ACE8F49BD9DF7A8DAE6E2B08ACDD0910B4FBD09DCD6BC5371F114576C7E500A85F8B933ED424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 13/13] txn: stop TXN_SIGNATURE_ABORT override X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 12.06.2021 00:56, Vladislav Shpilevoy пишет: > 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. > */ The comment became rather misleading now. We log the error in applier, not here. I would drop it altogether, but up to you. > - 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 -- Serge Petrenko