I have already posted the version addressing the previous review comments. On top of that, v7 includes several further fixes I found while re-examining the code myself, which are summarized below.
Correctness
-----------
- Scope the blocker scan to the relation's horizon kind.
GetXidHorizonBlocker() now takes the Relation and derives the
GlobalVisHorizonKind, so the ProcArray scan applies the same
per-database filter as ComputeXidHorizons() [1] (other-database
backends only hold back the shared horizon, plus
PROC_AFFECTS_ALL_HORIZONS), and a slot's catalog_xmin is only treated
as a blocker for the shared/catalog horizons. The temp-table horizon
(held only by our own backend) now returns no blocker. Previously a
backend in an unrelated database, or a logical slot's catalog_xmin
against a data-only horizon, could be reported.
- Compare against the slot's effective xmin, not data.xmin. The
horizon is aggregated from effective_xmin/effective_catalog_xmin [2],
so the scan now reads those. With data.xmin a blocker was missed
whenever the two diverge. For example, while a logical slot that
exports a snapshot is being created, effective_xmin is set but
data.xmin is left invalid. Reading effective_xmin is also safe for
physical slots, where it is set together with data.xmin and retained
after disconnect [3].
- Cover all replication slots and skip invalidated ones. The scan
iterates the full slot array (including repack slots) and skips
data.invalidated != RS_INVAL_NONE, matching
ReplicationSlotsComputeRequiredXmin() [2]. The worst-case result array
was resized accordingly.
- Classify active vs. idle-in-transaction from backend state, not
wait_event_info. A PGPROC cannot tell an idle-in-transaction session
from one blocked on client I/O (both wait on WAIT_EVENT_CLIENT_READ),
so e.g. an in-progress COPY FROM STDIN was logged as "idle in
transaction". The active/idle split is now resolved after the scan
locks are released, from PgBackendStatus.st_state, the same source as
pg_stat_activity.state [4].
Cleanup
-------
- Removed the unreachable XHB_NONE enum value (and its vacuumlazy.c
switch case).
- Removed the unused storage/lock.h include from procarray.h
(VirtualTransactionId comes via storage/standby.h).
- Report the slot blocker's xid as the horizon directly, rather than
recomputing it from the slot's effective xmin or catalog_xmin.
Test (014_log_vacuum_blockers.pl)
---------------------------------
- wal_receiver_status_interval = 100ms -> 1s. The GUC's unit is
seconds, so 100ms rounded down to 0, which disables periodic
hot-standby feedback. The feedback cases previously passed only by
chance, via the one-shot message sent at walreceiver startup.
- Made the hot-standby-feedback cases deterministic: wait for the
standby reader's snapshot to be reflected in the feedback (walsender
backend_xmin, or the slot's xmin) before the DELETE, rather than
polling for merely non-null.
- Disconnected-physical-slot case: issue a fresh INSERT/DELETE after
the standby disconnects, since terminating the reader may have let a
final feedback advance the frozen slot xmin past the earlier DELETE.
- Switched wait_for_catchup(..., 'replay', lsn('flush')) to
wait_for_replay_catchup().
[1]
https://github.com/postgres/postgres/blob/dbaa4dc3c8dd77a0e1c977024d4ae150e00456b3/src/backend/storage/ipc/procarray.c#L1796-L1799
[2]
https://github.com/postgres/postgres/blob/dbaa4dc3c8dd77a0e1c977024d4ae150e00456b3/src/backend/replication/slot.c#L1257-L1288
[3]
https://github.com/postgres/postgres/blob/dbaa4dc3c8dd77a0e1c977024d4ae150e00456b3/src/backend/replication/walsender.c#L2648-L2663
[4]
https://github.com/postgres/postgres/blob/dbaa4dc3c8dd77a0e1c977024d4ae150e00456b3/src/include/utils/backend_status.h#L24-L34
--
Best regards,
Shinya Kato
NTT OSS Center
v7-0001-Add-infrastructure-to-identify-what-holds-back-th.patch
Description: Binary data
v7-0002-Report-oldest-xmin-blocker-when-VACUUM-cannot-rem.patch
Description: Binary data
