Tarantool development patches archive
 help / color / mirror / Atom feed
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

             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