[PATCH v2] tx: exclude sysview engine from transaction control

[Vladimir Davydov <vdavydov.dev@gmail.com>]

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

Changes in v2:
 - Rebase on top of the latest 1.10

 src/box/engine.h              | 12 ++++++++++++
 src/box/sysview.c             |  1 +
 src/box/txn.c                 |  9 +++++----
 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, 74 insertions(+), 14 deletions(-)

diff --git a/src/box/engine.h b/src/box/engine.h
index e10eaec4..5b96c744 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.c b/src/box/sysview.c
index fb4ecf2d..a636c686 100644
--- a/src/box/sysview.c
+++ b/src/box/sysview.c
@@ -557,5 +557,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 fbce612a..8947ac35 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -116,16 +116,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;
 		}
 		txn_stmt_unref_tuples(stmt);
+		stmt->space = NULL;
+		stmt->row = NULL;
 	}
 }
 
@@ -158,6 +157,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