public inbox for [email protected]  
help / color / mirror / Atom feed
From: Scott Ray <[email protected]>
To: Shinya Kato <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Report oldest xmin source when autovacuum cannot remove tuples
Date: Sat, 06 Jun 2026 18:40:31 +0000
Message-ID: <GhIqK8CfmeX-u9drRyP0V0gBMYk0SO6dMHYW-woGxASgasMOG6nK_K6tMCYqIc_jkelCvF1RRScvQ7UHkDzSeJgIxKOY2CO1lJBJLulrbzg=@scottray.io> (raw)
In-Reply-To: <CAOzEurTohto7mfbmGu0GCHgAF+6roKxZgartyYydWsDkg3HzTg@mail.gmail.com>
References: <[email protected]>
	<3bnBUxwx2npXqvHL0trI11LOOvzQ7LI0GzWqbaj5SJnk7DTb1uzStGveKwj0JJmBW4ebzGIF3az7of4I4rQeaO_PRqDnnClCduPyjM6gPgM=@scottray.io>
	<CAOzEurTC+5xuH_X2EBWjW0Fs9iuTq3OMA=1FfT4xx1wYbCAR6g@mail.gmail.com>
	<[email protected]>
	<qOUvBIi7LC3GLKEjACMQAhPnYaE1m7Ta6XbMXN1gOVo3Mh6-Be6o_kQxQeCNXfo-Ywgc8k6myZrco4a41z-r7Dfqro4-nH73cB_tmSOczJk=@scottray.io>
	<CAOzEurTohto7mfbmGu0GCHgAF+6roKxZgartyYydWsDkg3HzTg@mail.gmail.com>

> > Shinya said [1] that we could have a view in the future.  We could
> > have both the logging and the view call a single function that reads
> > the procArray and other sources to gather the horizon information.  I
> > think the logging and the view would complement each other.
> >
> > Should I start another thread?
> 

> My mild preference would be to keep the discussion on this thread,
> since the shared function design is central to both the log and the
> view and may be easier to keep aligned in one place. That said, I'm
> not strongly attached to that, so please pick whichever feels more
> convenient.

I attached a patch that uses a helper function that could also be used
for a view, but I don't think it's necessary immediately, because
refactoring later wouldn't be difficult.

--
Scott Ray

Attachments:

  [application/octet-stream] v1-0001-Refactor-GetXidHorizonBlockers-around-a-new-GetXidHo.patch (8.4K, 2-v1-0001-Refactor-GetXidHorizonBlockers-around-a-new-GetXidHo.patch)
  download | inline diff:
From b4565351e0337fb759c9b7f511e6aeaa404c33f7 Mon Sep 17 00:00:00 2001
From: scottray-dev-bot <[email protected]>
Date: Sat, 6 Jun 2026 10:34:26 -0700
Subject: [PATCH v1] Refactor GetXidHorizonBlockers around a new
 GetXidHorizonContributors helper

---
 src/backend/storage/ipc/procarray.c | 119 +++++++++++++++++++---------
 src/include/storage/procarray.h     |  16 ++++
 src/tools/pgindent/typedefs.list    |   1 +
 3 files changed, 97 insertions(+), 39 deletions(-)

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index bc7b8bb5571..0e64012bf7f 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -2034,9 +2034,9 @@ GetStandbyAppname(int pid, char *name, Size len)
  * Return XidHorizonBlockerType for a backend whose xid matches the horizon.
  */
 static inline XidHorizonBlockerType
-XidHorizonBlockerTypeForBackend(const PGPROC *proc)
+XidHorizonBlockerTypeForBackend(uint32 wait_event_info)
 {
-	if (proc->wait_event_info == WAIT_EVENT_CLIENT_READ)
+	if (wait_event_info == WAIT_EVENT_CLIENT_READ)
 		return XHB_IDLE_IN_TRANSACTION;
 	return XHB_ACTIVE_TRANSACTION;
 }
@@ -2045,9 +2045,9 @@ XidHorizonBlockerTypeForBackend(const PGPROC *proc)
  * Return XidHorizonBlockerType for a backend whose xmin matches the horizon.
  */
 static inline XidHorizonBlockerType
-XidHorizonBlockerTypeForXminBackend(const PGPROC *proc)
+XidHorizonBlockerTypeForXminBackend(uint32 wait_event_info)
 {
-	if (proc->wait_event_info == WAIT_EVENT_CLIENT_READ)
+	if (wait_event_info == WAIT_EVENT_CLIENT_READ)
 		return XHB_XMIN_IDLE_IN_TRANSACTION;
 	return XHB_XMIN_ACTIVE_TRANSACTION;
 }
@@ -2079,33 +2079,32 @@ XidHorizonBlockerTypeForXminBackend(const PGPROC *proc)
 static XidHorizonBlocker *
 GetXidHorizonBlockers(TransactionId horizon, int *nblockers)
 {
-	ProcArrayStruct *arrayP = procArray;
-	TransactionId *other_xids = ProcGlobal->xids;
+	XidHorizonContributor *contributors;
+	int			n_contributors;
 	XidHorizonBlocker *result;
 	int			count = 0;
 	int			max_blockers;
+	ProcNumber	my_pgprocno;
 
 	Assert(TransactionIdIsValid(horizon));
 	Assert(nblockers != NULL);
 
+	my_pgprocno = MyProc ? GetNumberFromPGProc(MyProc) : INVALID_PROC_NUMBER;
+
+	contributors = GetXidHorizonContributors(&n_contributors);
+
 	/*
-	 * Allocate enough space for every PGPROC plus all replication slots. This
-	 * is a generous upper bound (typically only 0-2 entries are returned),
-	 * but keeps the logic simple for a diagnostic function that runs
-	 * infrequently.
+	 * Allocate enough space for every contributor plus all replication slots.
+	 * This is a generous upper bound (typically only 0-2 entries are
+	 * returned), but keeps the logic simple for a diagnostic function that
+	 * runs infrequently.
 	 */
-	max_blockers = arrayP->maxProcs + max_replication_slots;
+	max_blockers = n_contributors + max_replication_slots;
 	result = palloc0_array(XidHorizonBlocker, max_blockers);
 
-	LWLockAcquire(ProcArrayLock, LW_SHARED);
-
-	for (int index = 0; index < arrayP->numProcs; index++)
+	for (int i = 0; i < n_contributors; i++)
 	{
-		int			pgprocno = arrayP->pgprocnos[index];
-		PGPROC	   *proc = &allProcs[pgprocno];
-		int8		statusFlags = ProcGlobal->statusFlags[index];
-		TransactionId proc_xid;
-		TransactionId proc_xmin;
+		XidHorizonContributor *c = &contributors[i];
 		XidHorizonBlockerType candidate_type = XHB_NONE;
 		int			candidate_pid = 0;
 		TransactionId candidate_xid = InvalidTransactionId;
@@ -2115,44 +2114,40 @@ GetXidHorizonBlockers(TransactionId horizon, int *nblockers)
 		 * removed, as long as pg_subtrans is not truncated), doing logical
 		 * decoding (which manages xmin separately, check below), or myself.
 		 */
-		if (statusFlags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING) ||
-			proc == MyProc)
+		if (c->status_flags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING) ||
+			c->proc_number == my_pgprocno)
 			continue;
 
-		/* Fetch xid just once - see GetNewTransactionId */
-		proc_xid = UINT32_ACCESS_ONCE(other_xids[index]);
-		proc_xmin = UINT32_ACCESS_ONCE(proc->xmin);
-
 		/* Check if this proc's xid matches */
-		if (TransactionIdEquals(proc_xid, horizon))
+		if (TransactionIdEquals(c->xid, horizon))
 		{
-			if (proc->pid == 0)
+			if (c->pid == 0)
 			{
 				candidate_type = XHB_PREPARED_TRANSACTION;
 				candidate_pid = 0;
-				candidate_xid = proc_xid;
+				candidate_xid = c->xid;
 			}
 			else
 			{
-				candidate_type = XidHorizonBlockerTypeForBackend(proc);
-				candidate_pid = proc->pid;
-				candidate_xid = proc_xid;
+				candidate_type = XidHorizonBlockerTypeForBackend(c->wait_event_info);
+				candidate_pid = c->pid;
+				candidate_xid = c->xid;
 			}
 		}
 		/* Check if this proc's xmin matches */
-		else if (TransactionIdEquals(proc_xmin, horizon))
+		else if (TransactionIdEquals(c->xmin, horizon))
 		{
-			if (statusFlags & PROC_AFFECTS_ALL_HORIZONS)
+			if (c->status_flags & PROC_AFFECTS_ALL_HORIZONS)
 			{
 				candidate_type = XHB_HOT_STANDBY_FEEDBACK;
-				candidate_pid = proc->pid;
-				candidate_xid = proc_xmin;
+				candidate_pid = c->pid;
+				candidate_xid = c->xmin;
 			}
 			else
 			{
-				candidate_type = XidHorizonBlockerTypeForXminBackend(proc);
-				candidate_pid = proc->pid;
-				candidate_xid = proc_xmin;
+				candidate_type = XidHorizonBlockerTypeForXminBackend(c->wait_event_info);
+				candidate_pid = c->pid;
+				candidate_xid = c->xmin;
 			}
 		}
 
@@ -2171,7 +2166,7 @@ GetXidHorizonBlockers(TransactionId horizon, int *nblockers)
 		}
 	}
 
-	LWLockRelease(ProcArrayLock);
+	pfree(contributors);
 
 	/*
 	 * Now that ProcArrayLock is released, fetch any extra details we want to
@@ -3398,6 +3393,52 @@ ProcNumberGetTransactionIds(ProcNumber procNumber, TransactionId *xid,
 	LWLockRelease(ProcArrayLock);
 }
 
+/*
+ * GetXidHorizonContributors -- scan the procArray and return horizon
+ * contribution information for each process.
+ */
+XidHorizonContributor *
+GetXidHorizonContributors(int *n)
+{
+	XidHorizonContributor *result;
+	ProcArrayStruct *arrayP = procArray;
+	int			count = 0;
+	int			index;
+
+	result = palloc_array(XidHorizonContributor, arrayP->maxProcs);
+
+	LWLockAcquire(ProcArrayLock, LW_SHARED);
+
+	for (index = 0; index < arrayP->numProcs; index++)
+	{
+		int			pgprocno = arrayP->pgprocnos[index];
+		PGPROC	   *proc = &allProcs[pgprocno];
+		XidHorizonContributor *c = &result[count++];
+
+		c->proc_number = pgprocno;
+		c->pid = proc->pid;
+		c->database_id = proc->databaseId;
+		c->status_flags = ProcGlobal->statusFlags[index];
+
+		/* Fetch xid just once - see GetNewTransactionId */
+		c->xid = UINT32_ACCESS_ONCE(ProcGlobal->xids[index]);
+		c->xmin = UINT32_ACCESS_ONCE(proc->xmin);
+
+		/*
+		 * wait_event_info is written without lock by pgstat_report_wait_*();
+		 * read with UINT32_ACCESS_ONCE for a benign-racy 4-byte fetch, same
+		 * as proc->xmin above.
+		 */
+		c->wait_event_info = UINT32_ACCESS_ONCE(proc->wait_event_info);
+	}
+
+	LWLockRelease(ProcArrayLock);
+
+	*n = count;
+
+	return result;
+}
+
 /*
  * BackendPidGetProc -- get a backend's PGPROC given its PID
  *
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index 4954c82ab99..a63c6dc64d4 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -102,6 +102,22 @@ extern PGPROC *ProcNumberGetProc(int procNumber);
 extern void ProcNumberGetTransactionIds(int procNumber, TransactionId *xid,
 										TransactionId *xmin, int *nsubxid,
 										bool *overflowed);
+
+typedef struct XidHorizonContributor
+{
+	ProcNumber	proc_number;
+	int			pid;			/* OS pid; 0 for prepared-xact dummy procs */
+	Oid			database_id;
+	uint8		status_flags;
+	TransactionId xid;
+	TransactionId xmin;
+	uint32		wait_event_info;	/* sampled under ProcArrayLock via
+									 * UINT32_ACCESS_ONCE; written locklessly
+									 * by pgstat_report_wait_*(). */
+} XidHorizonContributor;
+
+extern XidHorizonContributor *GetXidHorizonContributors(int *n);
+
 extern PGPROC *BackendPidGetProc(int pid);
 extern PGPROC *BackendPidGetProcWithLock(int pid);
 extern int	BackendXidGetPid(TransactionId xid);
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 83b880b426e..590cc852858 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3546,6 +3546,7 @@ XidCacheStatus
 XidCommitStatus
 XidHorizonBlocker
 XidHorizonBlockerType
+XidHorizonContributor
 XidStatus
 XmlExpr
 XmlExprOp
-- 
2.50.1 (Apple Git-155)



  [application/pgp-signature] signature.asc (343B, 3-signature.asc)
  download

view thread (34+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Report oldest xmin source when autovacuum cannot remove tuples
  In-Reply-To: <GhIqK8CfmeX-u9drRyP0V0gBMYk0SO6dMHYW-woGxASgasMOG6nK_K6tMCYqIc_jkelCvF1RRScvQ7UHkDzSeJgIxKOY2CO1lJBJLulrbzg=@scottray.io>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox