Skip to content

Commit

Permalink
[yugabyte#18192] YSQL: Close backend on subtransaction abort failure.
Browse files Browse the repository at this point in the history
Summary:
D34725 introduced a best effort approach to abort transactions in order to prevent an error stack overflow in case of repeated failures.
This revision extends the same behavior to the abort of subtransactions: if a failure is detected in rolling back to a subtransaction, the backend
connection is terminated. This approach is preferred to handling and propagating the error further because of its simplicity.
This is helpful from an end user's perspective, as the previous approach produced a core-dump (as a result of a PANIC from stack overflow) which
raised a system alert and engaged Support teams for what is an innocuous error. This revision changes the core-dump to a FATAL log message.
Jira: DB-7215

Test Plan:
Manual testing.
Unit tests to follow in a separate revision.

Reviewers: pjain

Reviewed By: pjain

Subscribers: smishra, yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D35757
  • Loading branch information
karthik-ramanathan-3006 committed Jun 24, 2024
1 parent 0ac5e82 commit fe2b7ef
Showing 1 changed file with 25 additions and 1 deletion.
26 changes: 25 additions & 1 deletion src/postgres/src/backend/utils/misc/pg_yb_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,10 @@ YBCAbortTransaction()
* scenarios to avoid a recursive loop of aborting again and again as part
* of error handling in PostgresMain() because of the error faced during
* abort.
*
* Note - If you are changing the behavior to not terminate the backend,
* please consider its impact on sub-transaction abort failures
* (YBCRollbackToSubTransaction) as well.
*/
if (unlikely(status))
elog(FATAL, "Failed to abort DDL transaction: %s", YBCMessageAsCString(status));
Expand All @@ -990,7 +994,27 @@ YBCSetActiveSubTransaction(SubTransactionId id)
void
YBCRollbackToSubTransaction(SubTransactionId id)
{
HandleYBStatus(YBCPgRollbackToSubTransaction(id));
/*
* This function is invoked:
* - explicitly by user flows ("ROLLBACK TO <savepoint>" commands)
* - implicitly as a result of abort/commit flows
* - implicitly to implement exception handling in procedures and statement
* retries for YugabyteDB's read committed isolation level
* An error in rolling back to a subtransaction is likely due to issues in
* communicating with the tserver. Closing the backend connection
* here prevents Postgres from attempting transaction error recovery, which
* invokes the AbortCurrentTransaction flow (via the top level error handler
* in PostgresMain()), and likely ending up in a PANIC'ed state due to
* repeated failures caused by AbortSubTransaction not being reentrant.
* Closing the backend here is acceptable because alternate ways of
* handling this failure end up trying to abort the transaction which
* would anyway terminate the backend on failure. Revisit this approach in
* case the behavior of YBCAbortTransaction changes.
*/
YBCStatus status = YBCPgRollbackToSubTransaction(id);
if (unlikely(status))
elog(FATAL, "Failed to rollback to subtransaction %" PRId32 ": %s",
id, YBCMessageAsCString(status));
}

bool
Expand Down

0 comments on commit fe2b7ef

Please sign in to comment.