diff --git a/src/box/errcode.h b/src/box/errcode.h index a0759f8f4bde34d5050a1931565c42a7b9acd01e..7e3ea1ed148ea68dd0f460d9d0add25db275eea6 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -206,6 +206,7 @@ struct errcode_record { /*151 */_(ER_WRONG_COLLATION_OPTIONS, "Wrong collation options (field %u): %s") \ /*152 */_(ER_NULLABLE_PRIMARY, "Primary index of the space '%s' can not contain nullable parts") \ /*153 */_(ER_NULLABLE_MISMATCH, "Field %d is %s in space format, but %s in index parts") \ + /*154 */_(ER_TRANSACTION_YIELD, "Transaction has been aborted by a fiber yield") \ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c index 90b71702988c93ca2ea19c66562d551c0b228a6a..aa7ba770dc27fd0a26106d27e443a04bbc4eef44 100644 --- a/src/box/memtx_engine.c +++ b/src/box/memtx_engine.c @@ -68,12 +68,59 @@ struct mempool memtx_index_extent_pool; static int memtx_index_num_reserved_extents; static void *memtx_index_reserved_extents; +/* + * Memtx yield-in-transaction trigger: roll back the effects + * of the transaction and mark the transaction as aborted. + */ static void -txn_on_yield_or_stop(struct trigger *trigger, void *event) +txn_on_yield(struct trigger *trigger, void *event) { - (void)trigger; - (void)event; - txn_rollback(); /* doesn't throw */ + (void) trigger; + (void) event; + + struct txn *txn = in_txn(); + assert(txn && txn->engine_tx); + if (txn == NULL || txn->engine_tx == NULL) + return; + txn_abort(txn); /* doesn't yield or fail */ +} + +/** + * Initialize context for yield triggers. + * In case of a yield inside memtx multi-statement transaction + * we must, first of all, roll back the effects of the transaction + * so that concurrent transactions won't see dirty, uncommitted + * data. + * Second, we must abort the transaction, since it has been rolled + * back implicitly. The transaction can not be rolled back + * completely from within a yield trigger, since a yield trigger + * can't fail. Instead, we mark the transaction as aborted and + * raise an error on attempt to commit it. + * + * So much hassle to be user-friendly until we have a true + * interactive transaction support in memtx. + */ +void +memtx_init_txn(struct txn *txn) +{ + struct fiber *fiber = fiber(); + + trigger_create(&txn->fiber_on_yield, txn_on_yield, + NULL, NULL); + trigger_create(&txn->fiber_on_stop, txn_on_stop, + NULL, NULL); + /* + * Memtx doesn't allow yields between statements of + * a transaction. Set a trigger which would roll + * back the transaction if there is a yield. + */ + trigger_add(&fiber->on_yield, &txn->fiber_on_yield); + trigger_add(&fiber->on_stop, &txn->fiber_on_stop); + /* + * This serves as a marker that the triggers are + * initialized. + */ + txn->engine_tx = txn; } static int @@ -319,7 +366,7 @@ static int memtx_engine_prepare(struct engine *engine, struct txn *txn) { (void)engine; - if (txn->is_autocommit) + if (txn->engine_tx == 0) return 0; /* * These triggers are only used for memtx and only @@ -328,6 +375,11 @@ memtx_engine_prepare(struct engine *engine, struct txn *txn) */ trigger_clear(&txn->fiber_on_yield); trigger_clear(&txn->fiber_on_stop); + if (txn->is_aborted) { + diag_set(ClientError, ER_TRANSACTION_YIELD); + diag_log(); + return -1; + } return 0; } @@ -342,18 +394,7 @@ memtx_engine_begin(struct engine *engine, struct txn *txn) * to match with trigger_clear() in rollbackStatement(). */ if (txn->is_autocommit == false) { - - trigger_create(&txn->fiber_on_yield, txn_on_yield_or_stop, - NULL, NULL); - trigger_create(&txn->fiber_on_stop, txn_on_yield_or_stop, - NULL, NULL); - /* - * Memtx doesn't allow yields between statements of - * a transaction. Set a trigger which would roll - * back the transaction if there is a yield. - */ - trigger_add(&fiber()->on_yield, &txn->fiber_on_yield); - trigger_add(&fiber()->on_stop, &txn->fiber_on_stop); + memtx_init_txn(txn); } return 0; } @@ -363,6 +404,15 @@ memtx_engine_begin_statement(struct engine *engine, struct txn *txn) { (void)engine; (void)txn; + if (txn->engine_tx == NULL && + ! rlist_empty(&txn_last_stmt(txn)->space->on_replace)) { + /** + * A space on_replace trigger may initiate + * a yield. + */ + assert(txn->is_autocommit); + memtx_init_txn(txn); + } return 0; } @@ -414,7 +464,10 @@ memtx_engine_rollback_statement(struct engine *engine, struct txn *txn, static void memtx_engine_rollback(struct engine *engine, struct txn *txn) { - memtx_engine_prepare(engine, txn); + if (txn->engine_tx != NULL) { + trigger_clear(&txn->fiber_on_yield); + trigger_clear(&txn->fiber_on_stop); + } struct txn_stmt *stmt; stailq_reverse(&txn->stmts); stailq_foreach_entry(stmt, &txn->stmts, next) diff --git a/src/box/txn.c b/src/box/txn.c index e25c0e0e0eda7b9ff732d6a62f70e71ed2fddc0a..2934d21879e155cecc5b3f59a08b32b86f351b0b 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -131,6 +131,7 @@ txn_begin(bool is_autocommit) txn->n_rows = 0; txn->is_autocommit = is_autocommit; txn->has_triggers = false; + txn->is_aborted = false; txn->in_sub_stmt = 0; txn->id = ++txn_id; txn->signature = -1; @@ -356,6 +357,13 @@ txn_rollback() fiber_set_txn(fiber(), NULL); } +void +txn_abort(struct txn *txn) +{ + txn_rollback_to_svp(txn, NULL); + txn->is_aborted = true; +} + int txn_check_singlestatement(struct txn *txn, const char *where) { @@ -485,3 +493,12 @@ box_txn_rollback_to_savepoint(box_txn_savepoint_t *svp) txn_rollback_to_svp(txn, svp->stmt); return 0; } + +void +txn_on_stop(struct trigger *trigger, void *event) +{ + (void) trigger; + (void) event; + txn_rollback(); /* doesn't yield or fail */ +} + diff --git a/src/box/txn.h b/src/box/txn.h index 4db74dfca9d2ef3bf43775ce8d6cdbf40c4d5e23..06d7bed5fe40c25072201ca39b562e447fe82fb0 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -114,10 +114,15 @@ struct txn { * (statement end causes an automatic transaction commit). */ bool is_autocommit; + /** + * True if the transaction was aborted so should be + * rolled back at commit. + */ + bool is_aborted; /** True if on_commit and on_rollback lists are non-empty. */ bool has_triggers; /** The number of active nested statement-level transactions. */ - int in_sub_stmt; + int8_t in_sub_stmt; /** * First statement at each statement-level. * Needed to rollback sub statements. @@ -130,10 +135,15 @@ struct txn { /** Engine-specific transaction data */ void *engine_tx; /** - * Triggers on fiber yield and stop to abort transaction + * Triggers on fiber yield to abort transaction for * for in-memory engine. */ - struct trigger fiber_on_yield, fiber_on_stop; + struct trigger fiber_on_yield; + /** + * Trigger on fiber stop, to rollback transaction + * in case a fiber stops (all engines). + */ + struct trigger fiber_on_stop; /** Commit and rollback triggers */ struct rlist on_commit, on_rollback; }; @@ -166,6 +176,17 @@ txn_commit(struct txn *txn); void txn_rollback(); +/** + * Roll back the transaction but keep the object around. + * A special case for memtx transaction abort on yield. In this + * case we need to abort the transaction to avoid dirty reads but + * need to keep it around to ensure a new one is not implicitly + * started and committed by the user program. Later, at + * transaction commit we will raise an exception. + */ +void +txn_abort(struct txn *txn); + /** * Most txns don't have triggers, and txn objects * are created on every access to data, so txns @@ -278,6 +299,13 @@ txn_last_stmt(struct txn *txn) return stailq_last_entry(&txn->stmts, struct txn_stmt, next); } +/** + * Fiber-stop trigger: roll back the transaction. + */ +void +txn_on_stop(struct trigger *trigger, void *event); + + /** * FFI bindings: do not throw exceptions, do not accept extra * arguments diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 2dcb85923e9621d0707a297d2352451f4382df04..34799a290cb92735d7b01588ddaab3b2b25006c4 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -2385,14 +2385,6 @@ txn_stmt_unref_tuples(struct txn_stmt *stmt) stmt->new_tuple = NULL; } -static void -txn_on_stop(struct trigger *trigger, void *event) -{ - (void)trigger; - (void)event; - txn_rollback(); -} - static int vinyl_engine_begin(struct engine *engine, struct txn *txn) { diff --git a/test/app/fiber.result b/test/app/fiber.result index 38033dc53ccbde02c66e92fa0aea44e90ca8ce46..0253fe08fc448b784b396a7b19a43aa8dd63fff0 100644 --- a/test/app/fiber.result +++ b/test/app/fiber.result @@ -1044,7 +1044,7 @@ _ = box.space.test:create_index('pk') --- ... -- --- check that derived fiber does not see changes of the transaction +-- Check that derived fiber does not see changes of the transaction -- it must be rolled back before call -- l = nil @@ -1077,10 +1077,134 @@ f = nil l = nil --- ... -box.schema.space.drop('test') +box.space.test:drop() --- -- error: Illegal parameters, space_id should be a number ... +-- +-- Check that yield trigger is installed for a sub-statement +-- in autocommit mode +-- +_ = box.schema.space.create('test') +--- +... +_ = box.space.test:create_index('pk') +--- +... +test_run:cmd("setopt delimiter ';'") +--- +- true +... +l = nil +function yield() + f = fiber.create(function() l = box.space.test:get{1} end) + while f:status() ~= 'dead' do fiber.sleep(0.01) end +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +_ = box.space.test:on_replace(yield) +--- +... +box.space.test:insert{1} +--- +- error: Transaction has been aborted by a fiber yield +... +l +--- +- null +... +box.space.test:get{1} +--- +... +f = nil +--- +... +l = nil +--- +... +box.space.test:drop() +--- +... +-- +-- Check that rollback trigger is not left behind in the fiber in +-- case of user rollback. +-- +-- Begin a multi-statement transaction in memtx and roll it back. +-- Then begin a multi-statement transaction in vinyl and yield. +-- Observe vinyl transaction being implicitly rolled back by +-- yield. +-- +_ = box.schema.space.create('m') +--- +... +_ = box.space.m:create_index('pk') +--- +... +_ = box.schema.space.create('v', {engine='vinyl'}) +--- +... +_ = box.space.v:create_index('pk') +--- +... +l = nil +--- +... +l1 = nil +--- +... +test_run:cmd("setopt delimiter ';'") +--- +- true +... +function f() + box.begin() + box.space.m:insert{1} + box.rollback() + box.begin() + box.space.v:insert{1} + local f = fiber.create(function() l = box.space.v:get{1} end) + while f:status() ~= 'dead' do + fiber.sleep(0.01) + end + box.commit() + l1 = box.space.v:get{1} +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +f() +--- +... +l +--- +- null +... +l1 +--- +- [1] +... +box.space.m:drop() +--- +... +box.space.v:drop() +--- +... +f = nil +--- +... +l = nil +--- +... +l1 = nil +--- +... +-- cleanup test_run:cmd("clear filter") --- - true diff --git a/test/app/fiber.test.lua b/test/app/fiber.test.lua index d8ef913a69da5efd677ca5c38e5e42ad069fa2a3..c06a41779fba1728ac83c5d8cb430cd2a3ec6708 100644 --- a/test/app/fiber.test.lua +++ b/test/app/fiber.test.lua @@ -429,7 +429,7 @@ fiber.name(f) _ = box.schema.space.create('test') _ = box.space.test:create_index('pk') -- --- check that derived fiber does not see changes of the transaction +-- Check that derived fiber does not see changes of the transaction -- it must be rolled back before call -- l = nil @@ -443,6 +443,65 @@ while f:status() ~= 'dead' do fiber.sleep(0.01) end l f = nil l = nil -box.schema.space.drop('test') +box.space.test:drop() +-- +-- Check that yield trigger is installed for a sub-statement +-- in autocommit mode +-- +_ = box.schema.space.create('test') +_ = box.space.test:create_index('pk') +test_run:cmd("setopt delimiter ';'") +l = nil +function yield() + f = fiber.create(function() l = box.space.test:get{1} end) + while f:status() ~= 'dead' do fiber.sleep(0.01) end +end; +test_run:cmd("setopt delimiter ''"); +_ = box.space.test:on_replace(yield) +box.space.test:insert{1} +l +box.space.test:get{1} +f = nil +l = nil +box.space.test:drop() +-- +-- Check that rollback trigger is not left behind in the fiber in +-- case of user rollback. +-- +-- Begin a multi-statement transaction in memtx and roll it back. +-- Then begin a multi-statement transaction in vinyl and yield. +-- Observe vinyl transaction being implicitly rolled back by +-- yield. +-- +_ = box.schema.space.create('m') +_ = box.space.m:create_index('pk') +_ = box.schema.space.create('v', {engine='vinyl'}) +_ = box.space.v:create_index('pk') +l = nil +l1 = nil +test_run:cmd("setopt delimiter ';'") +function f() + box.begin() + box.space.m:insert{1} + box.rollback() + box.begin() + box.space.v:insert{1} + local f = fiber.create(function() l = box.space.v:get{1} end) + while f:status() ~= 'dead' do + fiber.sleep(0.01) + end + box.commit() + l1 = box.space.v:get{1} +end; +test_run:cmd("setopt delimiter ''"); +f() +l +l1 +box.space.m:drop() +box.space.v:drop() +f = nil +l = nil +l1 = nil +-- cleanup test_run:cmd("clear filter") diff --git a/test/box/ddl.result b/test/box/ddl.result index f6991840f7089d45c7d0ef6a4204ed4aa13861dc..3b2b12447dd760f4c309158a17cae78be94fd7a9 100644 --- a/test/box/ddl.result +++ b/test/box/ddl.result @@ -546,7 +546,7 @@ end); --- ... box.begin() -test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}}) + test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}}) box.commit(); --- - error: DDL does not support multi-statement transactions @@ -558,6 +558,11 @@ test_run:cmd("setopt delimiter ''"); _ = c:get() --- ... +-- Explicitly roll back the transaction in multi-statement, +-- which hasn't finished due to DDL error +box.rollback() +--- +... test_latch:drop() -- this is where everything stops --- ... diff --git a/test/box/ddl.test.lua b/test/box/ddl.test.lua index 6184ccb1a333e7f5c96feb73b90fb1ec7560e250..8b9884a1e238ea1828b74556105efda5e5d0a062 100644 --- a/test/box/ddl.test.lua +++ b/test/box/ddl.test.lua @@ -219,10 +219,11 @@ _ = fiber.create(function() end); box.begin() -test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}}) + test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}}) box.commit(); - test_run:cmd("setopt delimiter ''"); - _ = c:get() +-- Explicitly roll back the transaction in multi-statement, +-- which hasn't finished due to DDL error +box.rollback() test_latch:drop() -- this is where everything stops diff --git a/test/box/misc.result b/test/box/misc.result index cd3af7f9ea69ab841c541cefd9e6618bae3b3fcf..fe6781be7e8d80433608dd095f18e7e74c2af7bb 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -385,9 +385,10 @@ t; - 'box.error.CROSS_ENGINE_TRANSACTION : 81' - 'box.error.FORMAT_MISMATCH_INDEX_PART : 27' - 'box.error.FUNCTION_TX_ACTIVE : 30' + - 'box.error.injection : table: <address> - 'box.error.NO_SUCH_ENGINE : 57' - 'box.error.COMMIT_IN_SUB_STMT : 122' - - 'box.error.injection : table: <address> + - 'box.error.TRANSACTION_YIELD : 154' - 'box.error.NULLABLE_MISMATCH : 153' - 'box.error.LAST_DROP : 15' - 'box.error.NO_SUCH_ROLE : 82' diff --git a/test/box/transaction.result b/test/box/transaction.result index f60846e75ab11547043d8c964076ee027c337286..69e09e034a3ef7ea9c2f991781000144f8a1b30b 100644 --- a/test/box/transaction.result +++ b/test/box/transaction.result @@ -208,6 +208,7 @@ box.begin() s:insert{1, 'Must be rolled back'}; while s:get{1} ~= nil do fiber.sleep(0) end box.commit(); --- +- error: Transaction has been aborted by a fiber yield ... -- Test background fiber -- diff --git a/test/engine/iterator.result b/test/engine/iterator.result index a5fc9d735e05b78da24cbd1a728f2817e8865519..808713a8c8247caf2a3ee27804639ddc04263958 100644 --- a/test/engine/iterator.result +++ b/test/engine/iterator.result @@ -3947,17 +3947,19 @@ i2 = s:create_index('i2', { type = 'tree', parts = {2,'unsigned'}, unique = true _ = s:replace{2, 2} --- ... -box.begin() +inspector:cmd("setopt delimiter ';'"); --- +- true ... +box.begin() _ = s:replace{1, 1} +_ = pcall(s.upsert, s, {1, 1}, {{"+", 2, 1}}) +box.commit(); --- ... -_ = pcall(s.upsert, s, {1, 1}, {{"+", 2, 1}}) -- failed in unique secondary ---- -... -box.commit() +inspector:cmd("setopt delimiter ''"); --- +- true ... s:select{} --- diff --git a/test/engine/iterator.test.lua b/test/engine/iterator.test.lua index 905173ad9426fa5b7dabffe1a603c412001bafb2..fcf753f46438685bbbde4bbcb42f29ba55caa1d4 100644 --- a/test/engine/iterator.test.lua +++ b/test/engine/iterator.test.lua @@ -312,10 +312,12 @@ i2 = s:create_index('i2', { type = 'tree', parts = {2,'unsigned'}, unique = true _ = s:replace{2, 2} +inspector:cmd("setopt delimiter ';'"); box.begin() _ = s:replace{1, 1} -_ = pcall(s.upsert, s, {1, 1}, {{"+", 2, 1}}) -- failed in unique secondary -box.commit() +_ = pcall(s.upsert, s, {1, 1}, {{"+", 2, 1}}) +box.commit(); +inspector:cmd("setopt delimiter ''"); s:select{} s:drop{}