diff --git a/changelogs/unreleased/change-transaction-behaviour-after-rollback-on-yield.md b/changelogs/unreleased/change-transaction-behaviour-after-rollback-on-yield.md new file mode 100644 index 0000000000000000000000000000000000000000..862dc707cf9cc8fdd7d4d3afa075097b7744e746 --- /dev/null +++ b/changelogs/unreleased/change-transaction-behaviour-after-rollback-on-yield.md @@ -0,0 +1,8 @@ +## core/feature + +* Previously, if a yield occurs for a transaction that does not + support it, we roll back all its statements, but still process + its new statements (they will roll back with each yield). Also, + the transaction will be rolled back when a commit is attempted. + Now we stop processing any new statements right after first yield, + if transaction doesn't support it. diff --git a/src/box/memtx_tx.c b/src/box/memtx_tx.c index 0642d21376f6eba5d6571bd91b7c7a29c828c149..980d35f465d775624be92520b62badf6da9f1c4d 100644 --- a/src/box/memtx_tx.c +++ b/src/box/memtx_tx.c @@ -291,6 +291,7 @@ memtx_tx_abort_all_for_ddl(struct txn *ddl_owner) to_be_aborted->status != TXN_IN_READ_VIEW) continue; to_be_aborted->status = TXN_CONFLICTED; + txn_set_flags(to_be_aborted, TXN_IS_CONFLICTED); say_warn("Transaction committing DDL (id=%lld) has aborted " "another TX (id=%lld)", (long long) ddl_owner->id, (long long) to_be_aborted->id); @@ -360,6 +361,7 @@ memtx_tx_handle_conflict(struct txn *breaker, struct txn *victim) } else { /* Mark as conflicted. */ victim->status = TXN_CONFLICTED; + txn_set_flags(victim, TXN_IS_CONFLICTED); } } diff --git a/src/box/txn.c b/src/box/txn.c index 3785545d93ea7deb1bcd0b4f48948bb8c861d138..9203d2cff80382b684a8e512045e86695ee7b3e3 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -53,6 +53,27 @@ txn_on_stop(struct trigger *trigger, void *event); static int txn_on_yield(struct trigger *trigger, void *event); +static inline enum box_error_code +txn_flags_to_error_code(struct txn *txn) +{ + if (txn_has_flag(txn, TXN_IS_CONFLICTED)) + return ER_TRANSACTION_CONFLICT; + else if (txn_has_flag(txn, TXN_IS_ABORTED_BY_YIELD)) + return ER_TRANSACTION_YIELD; + return ER_UNKNOWN; +} + +static inline int +txn_check_can_continue(struct txn *txn) +{ + enum txn_flag flags = TXN_IS_CONFLICTED | TXN_IS_ABORTED_BY_YIELD; + if (txn_has_any_of_flags(txn, flags)) { + diag_set(ClientError, txn_flags_to_error_code(txn)); + return -1; + } + return 0; +} + static int txn_add_redo(struct txn *txn, struct txn_stmt *stmt, struct request *request) { @@ -349,13 +370,8 @@ txn_begin_stmt(struct txn *txn, struct space *space, uint16_t type) return -1; } - /* - * A conflict have happened; there is no reason to continue the TX. - */ - if (txn->status == TXN_CONFLICTED) { - diag_set(ClientError, ER_TRANSACTION_CONFLICT); + if (txn_check_can_continue(txn) != 0) return -1; - } struct txn_stmt *stmt = txn_stmt_new(&txn->region); if (stmt == NULL) @@ -703,12 +719,8 @@ txn_prepare(struct txn *txn) { txn->psn = ++txn_last_psn; - if (txn_has_flag(txn, TXN_IS_ABORTED_BY_YIELD)) { - assert(!txn_has_flag(txn, TXN_CAN_YIELD)); - diag_set(ClientError, ER_TRANSACTION_YIELD); - diag_log(); + if (txn_check_can_continue(txn) != 0) return -1; - } /* * If transaction has been started in SQL, deferred * foreign key constraints must not be violated. @@ -723,8 +735,7 @@ txn_prepare(struct txn *txn) * Somebody else has written some value that we have read. * The RW transaction is not possible. */ - if (txn->status == TXN_CONFLICTED || - (txn->status == TXN_IN_READ_VIEW && !stailq_empty(&txn->stmts))) { + if (txn->status == TXN_IN_READ_VIEW && !stailq_empty(&txn->stmts)) { diag_set(ClientError, ER_TRANSACTION_CONFLICT); return -1; } @@ -1251,9 +1262,11 @@ txn_on_yield(struct trigger *trigger, void *event) (void) event; struct txn *txn = in_txn(); assert(txn != NULL); - if (!txn_has_flag(txn, TXN_CAN_YIELD)) { + enum txn_flag flags = TXN_CAN_YIELD | TXN_IS_ABORTED_BY_YIELD; + if (!txn_has_any_of_flags(txn, flags)) { txn_rollback_to_svp(txn, NULL); txn_set_flags(txn, TXN_IS_ABORTED_BY_YIELD); + say_warn("Transaction has been aborted by a fiber yield"); } return 0; } diff --git a/src/box/txn.h b/src/box/txn.h index 588acf66079c189a2cc648eb82821437856778fd..0a13d341fddd2edd844f6f5368f57e48a3469635 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -91,6 +91,11 @@ enum txn_flag { * example, when applier receives snapshot from master. */ TXN_FORCE_ASYNC = 0x40, + /** + * Transaction was aborted when other transaction was + * committed due to conflict. + */ + TXN_IS_CONFLICTED = 0x80, }; enum { @@ -437,6 +442,12 @@ txn_has_flag(struct txn *txn, enum txn_flag flag) return (txn->flags & flag) != 0; } +static inline bool +txn_has_any_of_flags(struct txn *txn, enum txn_flag flags) +{ + return (txn->flags & flags) != 0; +} + static inline void txn_set_flags(struct txn *txn, unsigned int flags) {