Thanks for starting this thread! This is a very useful
feature that users will find beneficial to easily narrow
down the reason the xmin horizon is being held back,
and take action.
Adding this information to the vacuum logging is useful, but
I can see this information being exposed in a view as well in
the future.
I have a few comments:
A few minor ones:
1/ pid should be declared as "pid_t"
2/ last value of an enum should be have a traling comma
+typedef enum OldestXminSource
+{
+ OLDESTXMIN_SOURCE_ACTIVE_TRANSACTION,
+ OLDESTXMIN_SOURCE_HOT_STANDBY_FEEDBACK,
+ OLDESTXMIN_SOURCE_PREPARED_TRANSACTION,
+ OLDESTXMIN_SOURCE_REPLICATION_SLOT,
+ OLDESTXMIN_SOURCE_OTHER
+} OldestXminSource;
More importantly:
3/ As mentioned earlier in the thread, the "idle-in-transaction"
transactions is not being reported correctly, particularly for write
tansactions. I think that is an important missing case. The reason
for this is the cutoff xmin is not being looked up against the current
list of xid's, so we are not blaming the correct pid.
4/
Thinking about point 3 above, I began to wonder if this
whole thing can be simplified with inspiration. Looking at the
existing BackendXidGetPid(), I think it can.
Based on BackendXidGetPid(), I tried a new routine called
BackendXidFindCutOffReason() which can take in the cutoff xmin,
passed in by vacuum and can walk though the proc array and
determine the reason. We don't need to touch ComputeXidHorizons()
to make this work, it seems to me. This comes with an additional
walk though the procarray holding a shared lock, but I don't think
this will be an issue.
Attached is a rough sketch of BackendXidFindCutOffReason()
For now, I just added NOTICE messages which will log with
VACUUM (verbose) for testing.
This takes what you are doing in v1 inside ComputeXidHorizons()
into a new routine. I think this is a cleaner approach.
5/ Also, I think we should also include tests for serializable
transactions
What do you think?
--
Sami Imseih
Amazon Web Services (AWS)
From 53915bc1fd06790fc112cb2ac9e4b9caa742cf92 Mon Sep 17 00:00:00 2001
From: Sami Imseih <[email protected]>
Date: Fri, 14 Nov 2025 18:15:25 -0600
Subject: [PATCH 1/1] sketch of cutoff reasons
---
src/backend/access/heap/vacuumlazy.c | 5 +++
src/backend/storage/ipc/procarray.c | 60 ++++++++++++++++++++++++++++
src/include/storage/procarray.h | 1 +
3 files changed, 66 insertions(+)
diff --git a/src/backend/access/heap/vacuumlazy.c
b/src/backend/access/heap/vacuumlazy.c
index deb9a3dc0d1..df1df6b0733 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -152,6 +152,7 @@
#include "storage/bufmgr.h"
#include "storage/freespace.h"
#include "storage/lmgr.h"
+#include "storage/procarray.h"
#include "storage/read_stream.h"
#include "utils/lsyscache.h"
#include "utils/pg_rusage.h"
@@ -1047,6 +1048,10 @@ heap_vacuum_rel(Relation rel, const VacuumParams params,
appendStringInfo(&buf,
_("removable cutoff:
%u, which was %d XIDs old when operation ended\n"),
vacrel->cutoffs.OldestXmin, diff);
+
+ if (vacrel->recently_dead_tuples > 0)
+
BackendXidFindCutOffReason(vacrel->cutoffs.OldestXmin);
+
if (frozenxid_updated)
{
diff = (int32) (vacrel->NewRelfrozenXid -
diff --git a/src/backend/storage/ipc/procarray.c
b/src/backend/storage/ipc/procarray.c
index 200f72c6e25..45dfe8a9be2 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3244,6 +3244,66 @@ BackendXidGetPid(TransactionId xid)
return result;
}
+void
+BackendXidFindCutOffReason(TransactionId xid)
+{
+ ProcArrayStruct *arrayP = procArray;
+ TransactionId *other_xids = ProcGlobal->xids;
+ bool found_reason = false;
+
+ Assert(xid != InvalidTransactionId);
+
+ LWLockAcquire(ProcArrayLock, LW_SHARED);
+
+ elog(NOTICE, ">>>>>> looking up reason for %d", xid);
+
+ for (int index = 0; index < arrayP->numProcs; index++)
+ {
+ int pgprocno = arrayP->pgprocnos[index];
+ PGPROC *proc = &allProcs[pgprocno];
+
+ /* Case 1: xid matches the session's backend XID */
+ if (other_xids[index] == xid)
+ {
+ if (proc->pid == 0)
+ /* with a prepared transaction */
+ elog(NOTICE, ">>>>>> prepared transaction
proc->statusFlags %u", proc->statusFlags);
+ else
+ /* or a write transaction */
+ elog(NOTICE, ">>>>>> xid: transaction
BackendXidGetPid = %d proc->statusFlags %u", proc->pid, proc->statusFlags);
+
+ found_reason = true;
+ break;
+ }
+
+ /* Case 2: xid matches xmin */
+ if (proc->xmin == xid)
+ {
+ /* or affects horizons, which is due to
hot_standby_feedback */
+ if (proc->statusFlags & PROC_AFFECTS_ALL_HORIZONS)
+ {
+ elog(NOTICE, ">>>>>> hot_standby_feedback ==
pid of walreceiver %d proc->statusFlags %u", proc->pid, proc->statusFlags);
+ found_reason = true;
+ break;
+ }
+
+ /* or a read-only transaction */
+ elog(NOTICE, ">>>>>> xmin: transaction BackendXidGetPid
= %d proc->statusFlags = %u", proc->pid, proc->statusFlags);
+ found_reason = true;
+ break;
+ }
+ }
+
+ /*
+ * we failed to find reason, so it's likely a logical replication slot,
or
+ * some other reason
+ */
+ if (!found_reason)
+ elog(NOTICE, ">>>>>> other reasons, including logical
replication slot");
+
+ LWLockRelease(ProcArrayLock);
+}
+
/*
* IsBackendPid -- is a given pid a running backend
*
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index 2f4ae06c279..b0cdeedb848 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -71,6 +71,7 @@ extern void ProcNumberGetTransactionIds(int procNumber,
TransactionId *xid,
extern PGPROC *BackendPidGetProc(int pid);
extern PGPROC *BackendPidGetProcWithLock(int pid);
extern int BackendXidGetPid(TransactionId xid);
+extern void BackendXidFindCutOffReason(TransactionId xid);
extern bool IsBackendPid(int pid);
extern VirtualTransactionId *GetCurrentVirtualXIDs(TransactionId limitXmin,
--
2.50.1 (Apple Git-155)
