Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH] tx: exclude sysview engine from transaction control
@ 2018-07-18 11:27 Vladimir Davydov
  2018-07-21 12:57 ` [PATCH v2] " Vladimir Davydov
  2018-07-23 19:44 ` [PATCH] " Konstantin Osipov
  0 siblings, 2 replies; 3+ messages in thread
From: Vladimir Davydov @ 2018-07-18 11:27 UTC (permalink / raw)
  To: kostja; +Cc: tarantool-patches

Sysview is a special engine that is used for filtering out objects that
a user can't access due to lack of privileges. Since it's treated as a
separate engine by the transaction manager, we can't query sysview
spaces from a memtx/vinyl transaction. In particular, if called from a
transaction space:format() will return

  error: A multi-statement transaction can not use multiple storage engines

which is inconvenient.

To fix this, let's mark sysview engine with a new ENGINE_BYPASS_TX flag
and make the transaction manager skip binding a transaction to an engine
in case this flag is set.

Closes #3528
---
https://github.com/tarantool/tarantool/issues/3528
https://github.com/tarantool/tarantool/commits/dv/gh-3528-exclude-sysview-from-tx

 src/box/engine.h              | 12 ++++++++++++
 src/box/sysview_engine.c      |  1 +
 src/box/txn.c                 |  6 ++++--
 src/box/txn.h                 |  1 -
 test/box/on_replace.result    | 12 ++++++------
 test/box/transaction.result   | 37 ++++++++++++++++++++++++++++++++++---
 test/box/transaction.test.lua | 16 ++++++++++++++++
 7 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/src/box/engine.h b/src/box/engine.h
index 825f059e..6d0df44e 100644
--- a/src/box/engine.h
+++ b/src/box/engine.h
@@ -190,6 +190,16 @@ struct engine_vtab {
 	int (*check_space_def)(struct space_def *);
 };
 
+enum {
+	/**
+	 * If set, the engine will not participate in transaction
+	 * control. In particular, this means that any operations
+	 * done on this engine's spaces can mix in other engine's
+	 * transactions w/o throwing ER_CROSS_ENGINE_TRANSACTION.
+	 */
+	ENGINE_BYPASS_TX = 1 << 0,
+};
+
 struct engine {
 	/** Virtual function table. */
 	const struct engine_vtab *vtab;
@@ -197,6 +207,8 @@ struct engine {
 	const char *name;
 	/** Engine id. */
 	uint32_t id;
+	/** Engine flags. */
+	uint32_t flags;
 	/** Used for search for engine by name. */
 	struct rlist link;
 };
diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c
index 762bf93e..3e345d87 100644
--- a/src/box/sysview_engine.c
+++ b/src/box/sysview_engine.c
@@ -402,5 +402,6 @@ sysview_engine_new(void)
 
 	sysview->base.vtab = &sysview_engine_vtab;
 	sysview->base.name = "sysview";
+	sysview->base.flags = ENGINE_BYPASS_TX;
 	return sysview;
 }
diff --git a/src/box/txn.c b/src/box/txn.c
index cb301015..534309c1 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -107,15 +107,15 @@ txn_rollback_to_svp(struct txn *txn, struct stailq_entry *svp)
 	stailq_cut_tail(&txn->stmts, svp, &rollback);
 	stailq_reverse(&rollback);
 	stailq_foreach_entry(stmt, &rollback, next) {
-		if (stmt->space != NULL) {
+		if (txn->engine != NULL && stmt->space != NULL) {
 			engine_rollback_statement(txn->engine, txn, stmt);
-			stmt->space = NULL;
 		}
 		if (stmt->row != NULL) {
 			assert(txn->n_rows > 0);
 			txn->n_rows--;
 			stmt->row = NULL;
 		}
+		stmt->space = NULL;
 	}
 }
 
@@ -148,6 +148,8 @@ txn_begin(bool is_autocommit)
 int
 txn_begin_in_engine(struct engine *engine, struct txn *txn)
 {
+	if (engine->flags & ENGINE_BYPASS_TX)
+		return 0;
 	if (txn->engine == NULL) {
 		txn->engine = engine;
 		return engine_begin(engine, txn);
diff --git a/src/box/txn.h b/src/box/txn.h
index 70efb8c1..9a1f175a 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -249,7 +249,6 @@ txn_commit_ro_stmt(struct txn *txn)
 {
 	assert(txn == in_txn());
 	if (txn) {
-		assert(txn->engine);
 		/* nothing to do */
 	} else {
 		fiber_gc();
diff --git a/test/box/on_replace.result b/test/box/on_replace.result
index fcdb4379..f2de06f9 100644
--- a/test/box/on_replace.result
+++ b/test/box/on_replace.result
@@ -471,21 +471,21 @@ t = s:on_replace(function () s:create_index('sec') end, t)
 ...
 s:replace({2, 3})
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: DDL does not support multi-statement transactions
 ...
 t = s:on_replace(function () box.schema.user.create('newu') end, t)
 ---
 ...
 s:replace({3, 4})
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: Space _user does not support multi-statement transactions
 ...
 t = s:on_replace(function () box.schema.role.create('newr') end, t)
 ---
 ...
 s:replace({4, 5})
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: Space _user does not support multi-statement transactions
 ...
 t = s:on_replace(function () box.space._user:delete{box.schema.GUEST_ID} end, t)
 ---
@@ -506,21 +506,21 @@ t = s:on_replace(function () s:drop() end, t)
 ...
 s:replace({5, 6})
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: DDL does not support multi-statement transactions
 ...
 t = s:on_replace(function () box.schema.func.create('newf') end, t)
 ---
 ...
 s:replace({6, 7})
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: Space _func does not support multi-statement transactions
 ...
 t = s:on_replace(function () box.schema.user.grant('guest', 'read,write', 'space', 'test_on_repl_ddl') end, t)
 ---
 ...
 s:replace({7, 8})
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: Space _priv does not support multi-statement transactions
 ...
 t = s:on_replace(function () s:rename('newname') end, t)
 ---
diff --git a/test/box/transaction.result b/test/box/transaction.result
index 841dcf77..e0240842 100644
--- a/test/box/transaction.result
+++ b/test/box/transaction.result
@@ -69,21 +69,21 @@ box.rollback();
 ...
 box.begin() box.schema.func.create('test');
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: Space _func does not support multi-statement transactions
 ...
 box.rollback();
 ---
 ...
 box.begin() box.schema.user.create('test');
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: Space _user does not support multi-statement transactions
 ...
 box.rollback();
 ---
 ...
 box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv');
 ---
-- error: A multi-statement transaction can not use multiple storage engines
+- error: Space _priv does not support multi-statement transactions
 ...
 box.rollback();
 ---
@@ -498,3 +498,34 @@ space:select{}
 space:drop()
 ---
 ...
+--
+-- gh-3528: sysview engine shouldn't participate in transaction control
+--
+space = box.schema.space.create('test', {id = 9000})
+---
+...
+index = space:create_index('primary')
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+box.begin()
+space:auto_increment{box.space._vspace:get{space.id}}
+space:auto_increment{box.space._vindex:get{space.id, index.id}}
+box.commit();
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+space:select()
+---
+- - [1, [9000, 1, 'test', 'memtx', 0, {}, []]]
+  - [2, [9000, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]]
+...
+space:drop()
+---
+...
diff --git a/test/box/transaction.test.lua b/test/box/transaction.test.lua
index 14d1a690..e1d258e6 100644
--- a/test/box/transaction.test.lua
+++ b/test/box/transaction.test.lua
@@ -232,3 +232,19 @@ test_run:cmd("setopt delimiter ''");
 space:select{}
 
 space:drop()
+
+--
+-- gh-3528: sysview engine shouldn't participate in transaction control
+--
+space = box.schema.space.create('test', {id = 9000})
+index = space:create_index('primary')
+
+test_run:cmd("setopt delimiter ';'")
+box.begin()
+space:auto_increment{box.space._vspace:get{space.id}}
+space:auto_increment{box.space._vindex:get{space.id, index.id}}
+box.commit();
+test_run:cmd("setopt delimiter ''");
+
+space:select()
+space:drop()
-- 
2.11.0

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-07-23 19:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-18 11:27 [PATCH] tx: exclude sysview engine from transaction control Vladimir Davydov
2018-07-21 12:57 ` [PATCH v2] " Vladimir Davydov
2018-07-23 19:44 ` [PATCH] " Konstantin Osipov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox