[YSQL] YBCAbortTransaction elog(FATAL) during pg_terminate_backend of DDL session

[jasonyb]

Jira Link: DB-21258

Description

Summary

When a PostgreSQL backend running a DDL transaction is terminated via
pg_terminate_backend, the backend's attempt to abort the DDL transaction
fails with an elog(FATAL), producing a backtrace in the PG logs (on all build
types, due to yb_log_min_backtraces defaulting to FATAL). On debug and
fastdebug builds, this also triggers a TRAP assertion failure
!IsTransactionOrTransactionBlock() in pgstat_report_stat.

Root Cause

The SIGTERM signal handler die()
calls YBCInterruptPgGate(),
which signals the interrupter thread to shut down the RPC messenger via
Interrupter::AsyncHandler():

void AsyncHandler(ev::async& async, int events) {
    messenger_.Shutdown();
    pg_client_.Interrupt();
    loop_.break_loop();
}

The signal is sent via async_.send()
which is non-blocking — the actual messenger shutdown happens asynchronously on
the interrupter thread. There is technically a race between the signal handler
returning and the messenger shutdown completing. However, in practice the
messenger is shut down well before the abort RPC is attempted because the code
path from signal handler to RPC is long (see below).

Two FATALs are involved. To avoid ambiguity, they are labeled throughout:

• FATAL-1: ereport(FATAL, "terminating connection due to administrator command")
  at postgres.c:3723
• FATAL-2: elog(FATAL, "Failed to abort DDL transaction: %s", ...)
  at pg_yb_utils.c:1552

The chain of events

1. ProcessInterrupts() fires FATAL-1. In PostgreSQL, FATAL does not longjmp
   to the error handler like ERROR does (ERROR longjmps via PG_RE_THROW()).
   Instead, FATAL calls proc_exit(1) directly.

2. proc_exit runs before_shmem_exit callbacks in LIFO order. One of these is
   ShutdownPostgres
   → AbortOutOfAnyTransaction()
   → AbortTransaction() → YBCAbortTransaction().

3. YBCAbortTransaction() calls YBCPgClearSeparateDdlTxnMode()
   → FinishTransaction(Commit::kFalse), which attempts the abort RPC via
   PgClient::FinishTransaction.
   By this point the messenger is effectively always shut down, so the RPC fails.

4. The RPC failure triggers FATAL-2 at pg_yb_utils.c:1552.

Why FATAL-2 exists

FATAL-2 exists to prevent infinite retry loops in PostgresMain() error
handling: if abort fails and we return an ERROR, PG tries to abort again, which
fails again, looping forever. However, at this point we are already inside
proc_exit (triggered by FATAL-1), not in the PostgresMain loop, so that
infinite-loop concern does not apply here.

Why FATAL-2 during proc_exit does not itself cause an infinite loop

PG explicitly protects against this. Both shmem_exit and proc_exit_prepare
use a decrementing-index pattern:
each callback's index is decremented before calling it, so if the callback
triggers another FATAL → nested proc_exit → nested shmem_exit, the
already-called callbacks are skipped. The code comments explicitly note:
"if a callback calls ereport(ERROR) or ereport(FATAL) then it won't be invoked
again ... So, an infinite loop should not be possible."

TRAP assertion (debug/fastdebug only)

The TRAP assertion and FATAL-2 happen in the same process, sequentially —
FATAL-2 fires first, then the TRAP fires during the shutdown triggered by
FATAL-2.

FATAL-2 interrupts AbortTransaction() partway through. At this point,
s->state has been set to TRANS_ABORT
but CleanupTransaction()
(which resets blockState to TBLOCK_DEFAULT) never runs.

FATAL-2 triggers a nested proc_exit(1), which continues running remaining
before_shmem_exit callbacks. Since ShutdownPostgres was registered later
(LIFO), pgstat_shutdown_hook
(registered earlier, so runs later) is still pending:
pgstat_report_stat(true)
→ Assert(!IsTransactionOrTransactionBlock()).
Since blockState was never reset, this assertion fails.

• On release builds: Assert is compiled out, so pgstat_report_stat
  proceeds normally. The process exits via the remaining shutdown callbacks.
• On debug/fastdebug builds: Assert fires → abort() → immediate process
  termination with a core dump, skipping remaining callbacks.

This TRAP is a direct consequence of FATAL-2. Fixing FATAL-2 (so that
AbortTransaction completes normally and CleanupTransaction runs) would
eliminate the TRAP. Conversely, suppressing the TRAP assertion alone (e.g.
adding a proc_exit_inprogress guard in pgstat_report_stat) would not fix
FATAL-2 — it would only hide one symptom.

Symptoms

• FATAL: Failed to abort DDL transaction: Shutdown connection with backtrace
  in PG backend logs (all build types: release, debug, fastdebug — backtraces
  are emitted for FATAL and above due to yb_log_min_backtraces defaulting to
  FATAL)
• TRAP assertion !IsTransactionOrTransactionBlock() in pgstat.c:586
  (debug and fastdebug only; Assert is compiled out in release)

What this issue is fixing

FATAL-1 ("terminating connection") is the normal, expected behavior of
pg_terminate_backend and is not a bug.

This issue is about eliminating FATAL-2 ("Failed to abort DDL transaction")
and its consequence, the TRAP assertion on debug/fastdebug builds.

Proposed Fix

Apply at pg_yb_utils.c lines 1551-1552:
downgrade FATAL-2 to WARNING when the abort fails during shutdown. The abort
RPC is still attempted (in case the messenger hasn't shut down yet), but a
failure during shutdown no longer triggers a nested FATAL.

Note: ProcDiePending is cleared by proc_exit
when it starts, so proc_exit_inprogress should be used (or both, to also
cover any paths where YBCAbortTransaction is called before proc_exit).

if (unlikely(status)) {
    if (ProcDiePending || proc_exit_inprogress)
        elog(WARNING, "Best-effort DDL transaction abort failed during shutdown: %s",
             YBCMessageAsCString(status));
    else
        elog(FATAL, "Failed to abort DDL transaction: %s",
             YBCMessageAsCString(status));
}

This is safe because:

1. We are already inside proc_exit (triggered by FATAL-1), so the
   PostgresMain infinite retry loop that motivates FATAL-2 cannot occur.
2. A nested FATAL during proc_exit does not itself cause an infinite loop
   (PG's decrementing-index pattern prevents this), but it does produce
   unnecessary backtrace noise and, on debug/fastdebug, triggers the TRAP.
3. The abort RPC is still attempted, so in the rare case where the messenger
   hasn't shut down yet, the abort can succeed and the transaction is properly
   cleaned up immediately rather than waiting for heartbeat expiry.
4. The transaction coordinator will expire the PENDING transaction via heartbeat
   timeout (transaction_heartbeat_usec × transaction_max_missed_heartbeat_periods,
   default ~15s in release) if the RPC does fail.
5. The FATAL-2 guard for the non-shutdown case is preserved, maintaining the
   existing protection against infinite error-recovery loops during normal
   operation.

How to Reproduce

1. Start a long-running DDL operation, such as CREATE INDEX CONCURRENTLY on a
   large table. The operation needs to be long-lived enough that it is still in
   progress when step 2 is executed.
2. From another session, find the PID of the session running the DDL (e.g. via
   SELECT pid FROM pg_stat_activity WHERE query LIKE 'CREATE INDEX%'), then call
   SELECT pg_terminate_backend(<that pid>).
3. Observe FATAL-2 in the terminated backend's logs (all build types).
4. On debug/fastdebug builds, also observe the TRAP assertion.

Related

• PR [#26918] YSQL: Stop index backfill when the CREATE INDEX session is terminated #31378 / [YSQL] Cancel distributed index backfill when the PG backend goes away. #31263: Tests introduced by this PR (PgIndexBackfillCancellationTest)
  use pg_terminate_backend to kill a DDL backend and reliably surface this issue.
  The failing test logs serve as evidence of this bug.

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

• I confirm this issue does not contain any sensitive information.