v9-0002-Tweaks-on-top-of-v9.patch

From 8c7fd33e98ec958308979dba9cb7897787ddc48e Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 17 Dec 2024 15:15:23 +0900
Subject: [PATCH v9 2/2] Tweaks on top of v9

Other notes for commit:
Requires a bump of CATALOG_VERSION_NO.
Requires a bump of PGSTAT_FILE_FORMAT_ID.
---
 src/include/catalog/pg_proc.dat             |  3 +-
 src/include/pgstat.h                        | 11 +++--
 src/backend/utils/activity/backend_status.c |  2 +-
 src/backend/utils/activity/pgstat_backend.c | 38 ++++++++-------
 src/backend/utils/activity/pgstat_io.c      |  2 -
 src/backend/utils/adt/pgstatfuncs.c         | 53 +++++++++------------
 src/test/regress/expected/stats.out         | 28 +++++++----
 src/test/regress/sql/stats.sql              | 12 +++--
 doc/src/sgml/monitoring.sgml                | 10 ++--
 9 files changed, 86 insertions(+), 73 deletions(-)

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 437157ffa3..2dcc2d42da 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6061,8 +6061,7 @@
   proname => 'pg_stat_reset_single_function_counters', provolatile => 'v',
   prorettype => 'void', proargtypes => 'oid',
   prosrc => 'pg_stat_reset_single_function_counters' },
-{ oid => '9987',
-  descr => 'statistics: reset statistics for a single backend',
+{ oid => '8807', descr => 'statistics: reset statistics for a single backend',
   proname => 'pg_stat_reset_backend_stats', provolatile => 'v',
   prorettype => 'void', proargtypes => 'int4',
   prosrc => 'pg_stat_reset_backend_stats' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 479773cfd2..56ed1b8bb3 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -363,11 +363,11 @@ typedef struct PgStat_BktypeIO
 	PgStat_Counter times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
 } PgStat_BktypeIO;
 
-typedef struct PgStat_BackendPendingIO
+typedef struct PgStat_PendingIO
 {
 	PgStat_Counter counts[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
 	instr_time	pending_times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
-} PgStat_BackendPendingIO;
+} PgStat_PendingIO;
 
 typedef struct PgStat_IO
 {
@@ -375,6 +375,9 @@ typedef struct PgStat_IO
 	PgStat_BktypeIO stats[BACKEND_NUM_TYPES];
 } PgStat_IO;
 
+/* Backend statistics store the same amount of IO data as PGSTAT_KIND_IO */
+typedef PgStat_PendingIO PgStat_BackendPendingIO;
+
 typedef struct PgStat_Backend
 {
 	TimestampTz stat_reset_timestamp;
@@ -565,9 +568,9 @@ extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void);
  * Functions in pgstat_backend.c
  */
 
-extern PgStat_Backend *pgstat_fetch_proc_stat_io(ProcNumber procNumber);
+extern PgStat_Backend *pgstat_fetch_stat_backend(ProcNumber procNumber);
 extern bool pgstat_tracks_backend_bktype(BackendType bktype);
-extern void pgstat_create_backend_stat(ProcNumber procnum);
+extern void pgstat_create_backend(ProcNumber procnum);
 
 /*
  * Functions in pgstat_bgwriter.c
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index e9c3b87ac2..bf33e33a4e 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -428,7 +428,7 @@ pgstat_bestart(void)
 
 	/* Create the backend statistics entry */
 	if (pgstat_tracks_backend_bktype(MyBackendType))
-		pgstat_create_backend_stat(MyProcNumber);
+		pgstat_create_backend(MyProcNumber);
 
 	/* Update app name to current GUC setting */
 	if (application_name)
diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
index e2d83024c2..6b2c9baa8c 100644
--- a/src/backend/utils/activity/pgstat_backend.c
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -3,11 +3,17 @@
  * pgstat_backend.c
  *	  Implementation of backend statistics.
  *
- * This file contains the implementation of backend statistics. It is kept
+ * This file contains the implementation of backend statistics.  It is kept
  * separate from pgstat.c to enforce the line between the statistics access /
- * storage implementation and the details about individual types of statistics.
+ * storage implementation and the details about individual types of
+ * statistics.
  *
- * Copyright (c) 2024, PostgreSQL Global Development Group
+ * This statistics kind uses a proc number as object ID for the hash table
+ * of pgstats.  Entries are created each time a process is spawned, and are
+ * dropped when the process exits.  These are not written to the pgstats file
+ * on disk.
+ *
+ * Copyright (c) 2001-2024, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
  *	  src/backend/utils/activity/pgstat_backend.c
@@ -19,10 +25,10 @@
 #include "utils/pgstat_internal.h"
 
 /*
- * Returns backend's IO stats.
+ * Returns statistics of a backend by proc number.
  */
 PgStat_Backend *
-pgstat_fetch_proc_stat_io(ProcNumber procNumber)
+pgstat_fetch_stat_backend(ProcNumber procNumber)
 {
 	PgStat_Backend *backend_entry;
 
@@ -81,21 +87,21 @@ pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 void
 pgstat_flush_backend(bool nowait)
 {
-	if (pgstat_tracks_backend_bktype(MyBackendType))
-	{
-		PgStat_EntryRef *entry_ref;
+	PgStat_EntryRef *entry_ref;
 
-		entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid,
-										 MyProcNumber, false, NULL);
-		(void) pgstat_backend_flush_cb(entry_ref, nowait);
-	}
+	if (!pgstat_tracks_backend_bktype(MyBackendType))
+		return;
+
+	entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid,
+									 MyProcNumber, false, NULL);
+	(void) pgstat_backend_flush_cb(entry_ref, nowait);
 }
 
 /*
- * Create the backend statistics entry for procnum.
+ * Create backend statistics entry for proc number.
  */
 void
-pgstat_create_backend_stat(ProcNumber procnum)
+pgstat_create_backend(ProcNumber procnum)
 {
 	PgStat_EntryRef *entry_ref;
 	PgStatShared_Backend *shstatent;
@@ -113,7 +119,7 @@ pgstat_create_backend_stat(ProcNumber procnum)
 }
 
 /*
- * Find or create a local PgStat_BackendPendingIO entry for procnum.
+ * Find or create a local PgStat_BackendPendingIO entry for proc number.
  */
 PgStat_BackendPendingIO *
 pgstat_prep_backend_pending(ProcNumber procnum)
@@ -136,7 +142,7 @@ pgstat_prep_backend_pending(ProcNumber procnum)
  * I/O stats are already visible in pg_stat_io and there is only one of those.
  *
  * Function returns true if BackendType participates in the backend stats
- * subsystem for IO and false if it does not.
+ * subsystem and false if it does not.
  *
  * When adding a new BackendType, also consider adding relevant restrictions to
  * pgstat_tracks_io_object() and pgstat_tracks_io_op().
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index e0c206a453..011a3326da 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -20,8 +20,6 @@
 #include "storage/bufmgr.h"
 #include "utils/pgstat_internal.h"
 
-typedef PgStat_BackendPendingIO PgStat_PendingIO;
-
 static PgStat_PendingIO PendingIOStats;
 static bool have_iostats = false;
 
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index b939551d36..640e687604 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1474,20 +1474,22 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 	return (Datum) 0;
 }
 
+/*
+ * Returns I/O statistics for a backend with given PID.
+ */
 Datum
 pg_stat_get_backend_io(PG_FUNCTION_ARGS)
 {
 	ReturnSetInfo *rsinfo;
-	PgStat_Backend *backend_stats;
 	Datum		bktype_desc;
-	PgStat_BktypeIO *bktype_stats;
 	BackendType bktype;
 	Datum		reset_time;
-	int			num_backends = pgstat_fetch_stat_numbackends();
-	int			curr_backend;
 	int			pid;
 	PGPROC	   *proc;
 	ProcNumber	procNumber;
+	PgStat_Backend *backend_stats;
+	PgStat_BktypeIO *bktype_stats;
+	PgBackendStatus *beentry;
 
 	InitMaterializedSRF(fcinfo, 0);
 	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
@@ -1496,40 +1498,28 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS)
 	proc = BackendPidGetProc(pid);
 
 	/*
-	 * Could be an auxiliary process but would not report any stats due to
-	 * pgstat_tracks_backend_bktype() anyway. So don't need an extra call to
-	 * AuxiliaryPidGetProc().
+	 * This could be an auxiliary process but these do not report backend
+	 * statistics due to pgstat_tracks_backend_bktype(), so there is no need
+	 * for an extra call to AuxiliaryPidGetProc().
 	 */
 	if (!proc)
 		return (Datum) 0;
 
 	procNumber = GetNumberFromPGProc(proc);
-	backend_stats = pgstat_fetch_proc_stat_io(procNumber);
+
+	backend_stats = pgstat_fetch_stat_backend(procNumber);
 
 	if (!backend_stats)
 		return (Datum) 0;
 
-	bktype = B_INVALID;
+	beentry = pgstat_get_beentry_by_proc_number(procNumber);
+	bktype = beentry->st_backendType;
 
-	/* Look for the backend type */
-	for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
-	{
-		LocalPgBackendStatus *local_beentry;
-		PgBackendStatus *beentry;
+	/* If PID does not match, leave */
+	if (beentry->st_procpid != pid)
+		return (Datum) 0;
 
-		/* Get the next one in the list */
-		local_beentry = pgstat_get_local_beentry_by_index(curr_backend);
-		beentry = &local_beentry->backendStatus;
-
-		/* Looking for specific PID, ignore all the others */
-		if (beentry->st_procpid != pid)
-			continue;
-
-		bktype = beentry->st_backendType;
-		break;
-	}
-
-	/* Backend is gone */
+	/* Backend may be gone, so recheck in case */
 	if (bktype == B_INVALID)
 		return (Datum) 0;
 
@@ -1927,6 +1917,9 @@ pg_stat_reset_single_function_counters(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+/*
+ * Reset statistics of backend with given PID.
+ */
 Datum
 pg_stat_reset_backend_stats(PG_FUNCTION_ARGS)
 {
@@ -1936,9 +1929,9 @@ pg_stat_reset_backend_stats(PG_FUNCTION_ARGS)
 	proc = BackendPidGetProc(backend_pid);
 
 	/*
-	 * Could be an auxiliary process but would not report any stats due to
-	 * pgstat_tracks_backend_bktype() anyway. So don't need an extra call to
-	 * AuxiliaryPidGetProc().
+	 * This could be an auxiliary process but these do not report backend
+	 * statistics due to pgstat_tracks_backend_bktype(), so there is no need
+	 * for an extra call to AuxiliaryPidGetProc().
 	 */
 	if (!proc)
 		PG_RETURN_VOID();
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 3447e7b75d..150b6dcf74 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1249,7 +1249,8 @@ SELECT pg_stat_get_subscription_stats(NULL);
  
 (1 row)
 
--- Test that the following operations are tracked in pg_stat_io and in backend stats:
+-- Test that the following operations are tracked in pg_stat_io and in
+-- backend stats:
 -- - reads of target blocks into shared buffers
 -- - writes of shared buffers to permanent storage
 -- - extends of relations using shared buffers
@@ -1335,14 +1336,6 @@ SELECT current_setting('fsync') = 'off'
  t
 (1 row)
 
--- Don't return any rows if querying other backend's stats that are excluded
--- from the backend stats collection (like the checkpointer).
-SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid);
- ?column? 
-----------
- t
-(1 row)
-
 -- Change the tablespace so that the table is rewritten directly, then SELECT
 -- from it to cause it to be read back into shared buffers.
 SELECT sum(reads) AS io_sum_shared_before_reads
@@ -1603,6 +1596,23 @@ SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
  t
 (1 row)
 
+-- Check invalid input for pg_stat_get_backend_io()
+SELECT pg_stat_get_backend_io(NULL);
+ pg_stat_get_backend_io 
+------------------------
+(0 rows)
+
+SELECT pg_stat_get_backend_io(0);
+ pg_stat_get_backend_io 
+------------------------
+(0 rows)
+
+-- Auxiliary processes return no data.
+SELECT pg_stat_get_backend_io(:checkpointer_pid);
+ pg_stat_get_backend_io 
+------------------------
+(0 rows)
+
 -- test BRIN index doesn't block HOT update
 CREATE TABLE brin_hot (
   id  integer PRIMARY KEY,
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 9c925005be..1e7d0ff665 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -595,7 +595,8 @@ SELECT pg_stat_get_replication_slot(NULL);
 SELECT pg_stat_get_subscription_stats(NULL);
 
 
--- Test that the following operations are tracked in pg_stat_io and in backend stats:
+-- Test that the following operations are tracked in pg_stat_io and in
+-- backend stats:
 -- - reads of target blocks into shared buffers
 -- - writes of shared buffers to permanent storage
 -- - extends of relations using shared buffers
@@ -650,10 +651,6 @@ SELECT current_setting('fsync') = 'off'
   OR (:my_io_sum_shared_after_fsyncs = :my_io_sum_shared_before_fsyncs
       AND :my_io_sum_shared_after_fsyncs= 0);
 
--- Don't return any rows if querying other backend's stats that are excluded
--- from the backend stats collection (like the checkpointer).
-SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid);
-
 -- Change the tablespace so that the table is rewritten directly, then SELECT
 -- from it to cause it to be read back into shared buffers.
 SELECT sum(reads) AS io_sum_shared_before_reads
@@ -801,6 +798,11 @@ SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) +
   FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
 SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
 
+-- Check invalid input for pg_stat_get_backend_io()
+SELECT pg_stat_get_backend_io(NULL);
+SELECT pg_stat_get_backend_io(0);
+-- Auxiliary processes return no data.
+SELECT pg_stat_get_backend_io(:checkpointer_pid);
 
 -- test BRIN index doesn't block HOT update
 CREATE TABLE brin_hot (
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 4a9464da61..d0d176cc54 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4801,11 +4801,13 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        <para>
         Returns I/O statistics about the backend with the specified
         process ID. The output fields are exactly the same as the ones in the
-        <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
-        view. The function does not return I/O statistics for the checkpointer,
+        <structname>pg_stat_io</structname> view.
+       </para>
+       <para>
+        The function does not return I/O statistics for the checkpointer,
         the background writer, the startup process and the autovacuum launcher
-        as they are already visible in the <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
-        view and there is only one of those.
+        as they are already visible in the <structname>pg_stat_io</structname>
+        view and there is only one of each.
        </para></entry>
       </row>
 
-- 
2.45.2