From 590d9b545805f99b80103bfe900d175ad1d0668c Mon Sep 17 00:00:00 2001
From: Michael Zhilin <m.zhilin@postgrespro.ru>
Date: Thu, 10 Mar 2022 17:23:11 +0300
Subject: [PATCH 1/2] fix column wait_event of view pg_stat_activity

In case of high amount of connections, we have observed value 'ClientRead' of
column wait_event for active connections. Actually backend sets wait_event as
'ClientRead' only in case of idle or idle in transaction state.
The reason of discrepancy is different time moments of gathering information
from internal structures. At first it gathers state information for all backends.
Then it iterates over connections to gather wait information for each backend.

Before this patch the columns 'state' and 'wait_event' of view pg_stat_activity
may contain contadictive values.

The aim of change is to make wait information correct. To achieve it the state
and wait information should be gathered at same time.

Patch makes no change of system behaviour, but makes pg_stat_activity view more
consistent.

Tested on CentOS 7.9 / Debian 11 / FreeBSD 14 with 8-32 cores.

Co-authored-by: Yura Sokolov <y.sokolov@postgrespro.ru>
---
 src/backend/utils/activity/backend_status.c | 18 +++++++++
 src/backend/utils/adt/pgstatfuncs.c         | 45 ++++-----------------
 src/include/utils/backend_status.h          | 10 +++++
 3 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 415604db322..64567ff5cd7 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -845,6 +845,24 @@ pgstat_read_current_status(void)
 		/* Only valid entries get included into the local array */
 		if (localentry->backendStatus.st_procpid > 0)
 		{
+			PGPROC	*proc = BackendIdGetProc(i);
+			if (proc != NULL)
+			{
+				PGPROC	*leader = proc->lockGroupLeader;
+
+				/*
+				 * Show the leader only for active parallel workers.  This
+				 * leaves the field as NULL for the leader of a parallel
+				 * group.
+				 */
+				if (leader && leader->pid != localentry->backendStatus.st_procpid)
+					localentry->leader_pid = leader->pid;
+				else
+					localentry->leader_pid = 0;
+
+				localentry->wait_event_info = proc->wait_event_info;
+			}
+
 			BackendIdGetTransactionIds(i,
 									   &localentry->backend_xid,
 									   &localentry->backend_xmin);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index db061754c8b..82f169a2d8b 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -555,7 +555,6 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 		bool		nulls[PG_STAT_GET_ACTIVITY_COLS];
 		LocalPgBackendStatus *local_beentry;
 		PgBackendStatus *beentry;
-		PGPROC	   *proc;
 		const char *wait_event_type = NULL;
 		const char *wait_event = NULL;
 
@@ -652,46 +651,18 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 			pfree(clipped_activity);
 
 			/* leader_pid */
-			nulls[28] = true;
-
-			proc = BackendPidGetProc(beentry->st_procpid);
-
-			if (proc == NULL && (beentry->st_backendType != B_BACKEND))
+			if (local_beentry->leader_pid)
 			{
-				/*
-				 * For an auxiliary process, retrieve process info from
-				 * AuxiliaryProcs stored in shared-memory.
-				 */
-				proc = AuxiliaryPidGetProc(beentry->st_procpid);
+				values[28] = Int32GetDatum(local_beentry->leader_pid);
+				nulls[28] = false;
 			}
+			else
+				nulls[28] = true;
 
-			/*
-			 * If a PGPROC entry was retrieved, display wait events and lock
-			 * group leader information if any.  To avoid extra overhead, no
-			 * extra lock is being held, so there is no guarantee of
-			 * consistency across multiple rows.
-			 */
-			if (proc != NULL)
+			if (local_beentry->wait_event_info)
 			{
-				uint32		raw_wait_event;
-				PGPROC	   *leader;
-
-				raw_wait_event = UINT32_ACCESS_ONCE(proc->wait_event_info);
-				wait_event_type = pgstat_get_wait_event_type(raw_wait_event);
-				wait_event = pgstat_get_wait_event(raw_wait_event);
-
-				leader = proc->lockGroupLeader;
-
-				/*
-				 * Show the leader only for active parallel workers.  This
-				 * leaves the field as NULL for the leader of a parallel
-				 * group.
-				 */
-				if (leader && leader->pid != beentry->st_procpid)
-				{
-					values[28] = Int32GetDatum(leader->pid);
-					nulls[28] = false;
-				}
+				wait_event_type = pgstat_get_wait_event_type(local_beentry->wait_event_info);
+				wait_event = pgstat_get_wait_event(local_beentry->wait_event_info);
 			}
 
 			if (wait_event_type)
diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h
index 7403bca25ed..8f890911486 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -258,6 +258,16 @@ typedef struct LocalPgBackendStatus
 	 * not.
 	 */
 	TransactionId backend_xmin;
+
+	/*
+	 * The process wait information
+	 */
+	uint32 wait_event_info;
+
+	/*
+	 * The leader process ID
+	 */
+	int leader_pid;
 } LocalPgBackendStatus;
 
 
-- 
2.17.1

