Hi Shinya
Thanks for the updated v10 patch. The refactoring around GetXidHorizonBlocker()
and the prioritization of xid owner over xmin holder are very clean.
However, I would like to advocate strongly for keeping the distinction between
active and idle-in-transaction sessions (which was present in earlier versions
but dropped in v10).
From a production operations and DBA perspective, long-running idle-in-transaction
sessions (e.g. applications using ORMs or connection pools that execute a SELECT
with autocommit disabled, and then failing to commit/rollback before returning the
connection) are among the most common root causes of table bloat and horizon freeze.
While normal OLTP sessions switch rapidly between active and idle, leaked
transactions that hold back autovacuum are NOT rapidly changing—they typically
remain stagnant in "idle in transaction" for hours or days.
Reporting merely:
"removable cutoff was held back by: transaction holding snapshot (pid = %d)"
leaves DBAs without critical actionable information once the session has
disconnected:
1. If it was active, the action is SQL/index optimization or offloading heavy
queries to a standby.
2. If it was idle in transaction, the action is addressing application-side
bugs (uncommitted transaction leaks) or configuring
idle_in_transaction_session_timeout.
To avoid the layering concern of querying pgstat from procarray.c, could we
simply check proc->wait_event_info directly from PGPROC?
A backend waiting on WAIT_EVENT_CLIENT_READ while holding an open transaction/snapshot
is effectively idle-in-transaction. We could add a simple boolean flag (e.g. `is_idle`)
to XidHorizonBlocker, avoiding enum explosion while preserving this vital diagnostic clue
in the log.
What do you think?
Thanks
