From: Vladimir Davydov <vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org Subject: [PATCH 1/2] vinyl: drop useless vy_read_view->is_aborted flag Date: Fri, 22 Mar 2019 21:12:49 +0300 [thread overview] Message-ID: <6a374b13b7dc52ce14d8d23433efd46f3e2161cf.1553277782.git.vdavydov.dev@gmail.com> (raw) It is supposed to abort an iterator that read a statement reverted after a failed WAL write. However, we will abort such an iterator anyway - see vy_tx_abort_readers(). So let's drop the useless flag. It turned out that we never tested such a case (try grepping "read view is aborted" error). So let's also add a functional test for it. --- https://github.com/tarantool/tarantool/issues/4070 https://github.com/tarantool/tarantool/commits/dv/gh-4070-vy-fail-aborted-transactions-early src/box/vinyl.c | 2 +- src/box/vy_read_view.h | 6 ----- src/box/vy_tx.c | 6 ----- test/vinyl/errinj_tx.result | 60 +++++++++++++++++++++++++++++++++++++++++++ test/vinyl/errinj_tx.test.lua | 28 ++++++++++++++++++++ 5 files changed, 89 insertions(+), 13 deletions(-) diff --git a/src/box/vinyl.c b/src/box/vinyl.c index c2b7eb78..bf27f5fb 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -3716,7 +3716,7 @@ vinyl_iterator_check_tx(struct vinyl_iterator *it) diag_set(ClientError, ER_CURSOR_NO_TRANSACTION); return -1; } - if (it->tx->state == VINYL_TX_ABORT || it->tx->read_view->is_aborted) { + if (it->tx->state == VINYL_TX_ABORT) { /* Transaction read view was aborted. */ diag_set(ClientError, ER_READ_VIEW_ABORTED); return -1; diff --git a/src/box/vy_read_view.h b/src/box/vy_read_view.h index c3b14699..a9b37dcc 100644 --- a/src/box/vy_read_view.h +++ b/src/box/vy_read_view.h @@ -63,12 +63,6 @@ struct vy_read_view { * count it as it is missing from read_views list. */ int refs; - /** - * Is set to true when the read view which includes - * a prepared but not committed transaction, is - * compromised by a cascading rollback. - */ - bool is_aborted; }; #if defined(__cplusplus) diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c index c4445505..ab0bf071 100644 --- a/src/box/vy_tx.c +++ b/src/box/vy_tx.c @@ -97,7 +97,6 @@ vy_global_read_view_create(struct vy_read_view *rv, int64_t lsn) */ rv->vlsn = lsn; rv->refs = 0; - rv->is_aborted = false; } struct tx_manager * @@ -185,7 +184,6 @@ tx_manager_read_view(struct tx_manager *xm) "mempool", "read view"); return NULL; } - rv->is_aborted = false; if (xm->last_prepared_tx != NULL) { rv->vlsn = MAX_LSN + xm->last_prepared_tx->psn; xm->last_prepared_tx->read_view = rv; @@ -815,10 +813,6 @@ vy_tx_rollback_after_prepare(struct vy_tx *tx) vy_mem_unpin(v->mem); } - /* Abort read views of dependent transactions. */ - if (tx->read_view != &xm->global_read_view) - tx->read_view->is_aborted = true; - struct write_set_iterator it; write_set_ifirst(&tx->write_set, &it); while ((v = write_set_inext(&it)) != NULL) { diff --git a/test/vinyl/errinj_tx.result b/test/vinyl/errinj_tx.result index 29ec47c8..e2e4916e 100644 --- a/test/vinyl/errinj_tx.result +++ b/test/vinyl/errinj_tx.result @@ -433,6 +433,66 @@ last_read space:drop() --- ... +-- +-- Check that dependent transaction is aborted on WAL write. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +--- +... +_ = s:create_index('pk') +--- +... +s:replace{10} +--- +- [10] +... +-- Insert a tuple into the memory level and stall on WAL write. +ch = fiber.channel(1) +--- +... +errinj.set('ERRINJ_WAL_DELAY', true) +--- +- ok +... +_ = fiber.create(function() pcall(s.replace, s, {1}) ch:put(true) end) +--- +... +-- Read the tuple from another transaction. +itr = create_iterator(s) +--- +... +itr.next() +--- +- [1] +... +-- Resume the first transaction and let it fail on WAL write. +errinj.set('ERRINJ_WAL_WRITE', true) +--- +- ok +... +errinj.set('ERRINJ_WAL_DELAY', false) +--- +- ok +... +ch:get() +--- +- true +... +errinj.set('ERRINJ_WAL_WRITE', false) +--- +- ok +... +-- Must fail. +itr.next() +--- +- error: The read view is aborted +... +itr = nil +--- +... +s:drop() +--- +... -- Collect all iterators to make sure no read views are left behind, -- as they might disrupt the following test run. collectgarbage() diff --git a/test/vinyl/errinj_tx.test.lua b/test/vinyl/errinj_tx.test.lua index b06831b7..d17c1abb 100644 --- a/test/vinyl/errinj_tx.test.lua +++ b/test/vinyl/errinj_tx.test.lua @@ -193,6 +193,34 @@ last_read space:drop() +-- +-- Check that dependent transaction is aborted on WAL write. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +_ = s:create_index('pk') +s:replace{10} + +-- Insert a tuple into the memory level and stall on WAL write. +ch = fiber.channel(1) +errinj.set('ERRINJ_WAL_DELAY', true) +_ = fiber.create(function() pcall(s.replace, s, {1}) ch:put(true) end) + +-- Read the tuple from another transaction. +itr = create_iterator(s) +itr.next() + +-- Resume the first transaction and let it fail on WAL write. +errinj.set('ERRINJ_WAL_WRITE', true) +errinj.set('ERRINJ_WAL_DELAY', false) +ch:get() +errinj.set('ERRINJ_WAL_WRITE', false) + +-- Must fail. +itr.next() + +itr = nil +s:drop() + -- Collect all iterators to make sure no read views are left behind, -- as they might disrupt the following test run. collectgarbage() -- 2.11.0
next reply other threads:[~2019-03-22 18:12 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-22 18:12 Vladimir Davydov [this message] 2019-03-22 18:12 ` [PATCH 2/2] vinyl: fail aborted transactions early Vladimir Davydov 2019-03-28 14:05 ` [tarantool-patches] " Konstantin Osipov 2019-03-28 14:43 ` Vladimir Davydov 2019-03-28 12:50 ` [tarantool-patches] Re: [PATCH 1/2] vinyl: drop useless vy_read_view->is_aborted flag Konstantin Osipov
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=6a374b13b7dc52ce14d8d23433efd46f3e2161cf.1553277782.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH 1/2] vinyl: drop useless vy_read_view->is_aborted flag' \ /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