[PATCH] tx: exclude sysview engine from transaction control
Vladimir Davydov
vdavydov.dev at gmail.com
Wed Jul 18 14:27:27 MSK 2018
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
More information about the Tarantool-patches
mailing list