From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH] tx: exclude sysview engine from transaction control Date: Wed, 18 Jul 2018 14:27:27 +0300 Message-Id: <1e2b7faf141e6b18fdcdf2ea756b28d5d91722fb.1531913079.git.vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: 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