From 9c5d3ed7ca27f7573ff70ff6871a0e6befb37d12 Mon Sep 17 00:00:00 2001
From: mechanik20051988 <mechanik20051988@tarantool.org>
Date: Wed, 6 Oct 2021 17:22:07 +0300
Subject: [PATCH] txn: ignore changes after rollback on yield
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. This is done so that when we
implement timeout for transactions, the rollback of the transaction
at its expiration and at yield has the same behavior.
Part of #6177
---
...ction-behaviour-after-rollback-on-yield.md | 8 ++++
src/box/memtx_tx.c | 2 +
src/box/txn.c | 41 ++++++++++++-------
src/box/txn.h | 11 +++++
4 files changed, 48 insertions(+), 14 deletions(-)
create mode 100644 changelogs/unreleased/change-transaction-behaviour-after-rollback-on-yield.md
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 0000000000..862dc707cf
--- /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 0642d21376..980d35f465 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 3785545d93..9203d2cff8 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 588acf6607..0a13d341fd 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)
{
--
GitLab