From b4565351e0337fb759c9b7f511e6aeaa404c33f7 Mon Sep 17 00:00:00 2001 From: scottray-dev-bot <280544850+scottray-dev-bot@users.noreply.github.com> 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)