Subject: Re: per backend I/O statistics

From: Bertrand Drouvot <[email-masked]>
Date: Mon, 16 Dec 2024 15:42:04 +0000

Hi,

On Mon, Dec 16, 2024 at 05:07:52PM +0900, Michael Paquier wrote:
> On Fri, Dec 13, 2024 at 09:20:13AM +0000, Bertrand Drouvot wrote:
> 
> Not feeling so sure about the value brought by the backend_type
> returned in pg_stat_get_backend_io(), but well..

It's not necessary per say, but it ensures that pg_stat_get_backend_io() does not
return rows full of "invalid" combinations.

For example:

With the filtering in place, on a "client backend" we would get:

postgres=# select * from pg_stat_get_backend_io(pg_backend_pid());
  backend_type  |    object     |  context  | reads | read_time | writes | write_time | writebacks | writeback_time | extends | extend_time | op_bytes | hits | evictions | reuses | fsyncs | fsync_time | stats_reset
----------------+---------------+-----------+-------+-----------+--------+------------+------------+----------------+---------+-------------+----------+------+-----------+--------+--------+------------+-------------
 client backend | relation      | bulkread  |     0 |         0 |      0 |          0 |          0 |              0 |         |             |     8192 |    0 |         0 |      0 |        |            |
 client backend | relation      | bulkwrite |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |    0 |         0 |      0 |        |            |
 client backend | relation      | normal    |    86 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 | 1604 |         0 |        |      0 |          0 |
 client backend | relation      | vacuum    |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |    0 |         0 |      0 |        |            |
 client backend | temp relation | normal    |     0 |         0 |      0 |          0 |            |                |       0 |           0 |     8192 |    0 |         0 |        |        |            |
(5 rows)

and for example for the walsender:

postgres=# select * from pg_stat_get_backend_io(3982910);
 backend_type |    object     |  context  | reads | read_time | writes | write_time | writebacks | writeback_time | extends | extend_time | op_bytes | hits | evictions | reuses | fsyncs | fsync_time | stats_reset
--------------+---------------+-----------+-------+-----------+--------+------------+------------+----------------+---------+-------------+----------+------+-----------+--------+--------+------------+-------------
 walsender    | relation      | bulkread  |     0 |         0 |      0 |          0 |          0 |              0 |         |             |     8192 |    0 |         0 |      0 |        |            |
 walsender    | relation      | bulkwrite |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |    0 |         0 |      0 |        |            |
 walsender    | relation      | normal    |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |   54 |         0 |        |      0 |          0 |
 walsender    | relation      | vacuum    |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |    0 |         0 |      0 |        |            |
 walsender    | temp relation | normal    |     0 |         0 |      0 |          0 |            |                |       0 |           0 |     8192 |    0 |         0 |        |        |            |
(5 rows)

While, without the filtering we would get:

postgres=# select * from pg_stat_get_backend_io(pg_backend_pid());
  backend_type  |    object     |  context  | reads | read_time | writes | write_time | writebacks | writeback_time | extends | extend_time | op_bytes | hits | evictions | reuses | fsyncs | fsync_time | stats_reset
----------------+---------------+-----------+-------+-----------+--------+------------+------------+----------------+---------+-------------+----------+------+-----------+--------+--------+------------+-------------
 client backend | relation      | bulkread  |     0 |         0 |      0 |          0 |          0 |              0 |         |             |     8192 |    0 |         0 |      0 |        |            |
 client backend | relation      | bulkwrite |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |    0 |         0 |      0 |        |            |
 client backend | relation      | normal    |     4 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |   50 |         0 |        |      0 |          0 |
 client backend | relation      | vacuum    |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |    0 |         0 |      0 |        |            |
 client backend | temp relation | bulkread  |       |           |        |            |            |                |         |             |     8192 |      |           |        |        |            |
 client backend | temp relation | bulkwrite |       |           |        |            |            |                |         |             |     8192 |      |           |        |        |            |
 client backend | temp relation | normal    |     0 |         0 |      0 |          0 |            |                |       0 |           0 |     8192 |    0 |         0 |        |        |            |
 client backend | temp relation | vacuum    |       |           |        |            |            |                |         |             |     8192 |      |           |        |        |            |
(8 rows)

and for a walsender:

postgres=# select * from pg_stat_get_backend_io(3981588);
 backend_type |    object     |  context  | reads | read_time | writes | write_time | writebacks | writeback_time | extends | extend_time | op_bytes | hits | evictions | reuses | fsyncs | fsync_time | stats_reset
--------------+---------------+-----------+-------+-----------+--------+------------+------------+----------------+---------+-------------+----------+------+-----------+--------+--------+------------+-------------
 walsender    | relation      | bulkread  |     0 |         0 |      0 |          0 |          0 |              0 |         |             |     8192 |    0 |         0 |      0 |        |            |
 walsender    | relation      | bulkwrite |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |    0 |         0 |      0 |        |            |
 walsender    | relation      | normal    |     6 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |   48 |         0 |        |      0 |          0 |
 walsender    | relation      | vacuum    |     0 |         0 |      0 |          0 |          0 |              0 |       0 |           0 |     8192 |    0 |         0 |      0 |        |            |
 walsender    | temp relation | bulkread  |       |           |        |            |            |                |         |             |     8192 |      |           |        |        |            |
 walsender    | temp relation | bulkwrite |       |           |        |            |            |                |         |             |     8192 |      |           |        |        |            |
 walsender    | temp relation | normal    |     0 |         0 |      0 |          0 |            |                |       0 |           0 |     8192 |    0 |         0 |        |        |            |
 walsender    | temp relation | vacuum    |       |           |        |            |            |                |         |             |     8192 |      |           |        |        |            |
(8 rows)

It would not be possible to remove those "extra 3 rows" with a join on the
backend type on pg_stat_activity because only the C code knows about what the
compatibility is (though pgstat_tracks_io_object).

> +	/* drop the backend stats entry */
> +	pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber);
> 
> Oh, I've missed something.  Shouldn't pgstat_request_entry_refs_gc()
> be called when this returns false?

I'm not sure that would be possible for a PGSTAT_KIND_BACKEND kind to return
false here but I agree that's better to call pgstat_request_entry_refs_gc()
if that's the case. Done in v9 attached.

> The creation of the dshash entry is a bit too early, I think..  How
> about delaying it more so as we don't create entries that could be
> useless if we fail the last steps of authentication?  One spot would
> be to delay the creation of the new entry at the end of
> pgstat_bestart(), where we know that we are done with authentication
> and that the backend is ready to report back to the client connected
> to it.  It is true that some subsystems could produce stats as of the 
> early transactions they generate, which is why pgstat_initialize() is
> done that early in BaseInit(), but that's not really relevant for this
> case?

I think that makes sense to move the stats entry creation in pgstat_bestart(),
done that way in v9.

> I'm still feeling a bit uneasy about the drop done in
> pgstat_shutdown_hook(); it would be nice to make sure that this
> happens in a path that would run just after we're done with the
> creation of the entry to limit windows where we have an entry but no
> way to drop it, or vice-versa,

I don't believe that's possible as MyProcNumber can't be "reused" (and is
guaranteed to remain valid) until ProcKill() is done (which happens after the
pgstat_shutdown_hook()).

> Perhaps there's an argument for an entirely separate
> callback that would run before pgstat is plugged off, like a new
> before_shmem_exit() callback registered after the entry is created?

As the ProcKill() is run in shmem_exit() (and so after before_shmem_exit()),
I think that the way we currently drop the backend stats entry is fine (unless
I miss something about your concern).

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
>From 5bd09f37d6fdb3db38eff8ce30677d3989fcf64c Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Mon, 28 Oct 2024 12:50:32 +0000
Subject: [PATCH v9] per backend I/O statistics

While pg_stat_io provides cluster-wide I/O statistics, this commit adds the
ability to track and display per backend I/O statistics.

It adds a new statistics kind and 2 new functions:

- pg_stat_reset_backend_stats() to be able to reset the stats for a given
backend pid.
- pg_stat_get_backend_io() to retrieve I/O statistics for a given backend pid.

The new KIND is named PGSTAT_KIND_BACKEND as it could be used in the future
to store other statistics (than the I/O ones) per backend. The new KIND is
a variable-numbered one and has an automatic cap on the maximum number of
entries (as its hash key contains the proc number).

There is no need to write the per backend I/O stats to disk (no point to
see stats for backends that do not exist anymore after a re-start), so using
"write_to_file = false".

Note that per backend I/O statistics are not collected for the checkpointer,
the background writer, the startup process and the autovacuum launcher as those
are already visible in pg_stat_io and there is only one of those.

XXX: Bump catalog version needs to be done.
---
 doc/src/sgml/config.sgml                     |   8 +-
 doc/src/sgml/monitoring.sgml                 |  37 ++++
 src/backend/catalog/system_functions.sql     |   2 +
 src/backend/utils/activity/Makefile          |   1 +
 src/backend/utils/activity/backend_status.c  |   4 +
 src/backend/utils/activity/meson.build       |   1 +
 src/backend/utils/activity/pgstat.c          |  23 ++-
 src/backend/utils/activity/pgstat_backend.c  | 182 +++++++++++++++++++
 src/backend/utils/activity/pgstat_io.c       |  25 ++-
 src/backend/utils/activity/pgstat_relation.c |   2 +
 src/backend/utils/adt/pgstatfuncs.c          | 169 +++++++++++++++++
 src/include/catalog/pg_proc.dat              |  14 ++
 src/include/pgstat.h                         |  31 +++-
 src/include/utils/pgstat_internal.h          |  14 ++
 src/test/regress/expected/stats.out          |  72 +++++++-
 src/test/regress/sql/stats.sql               |  38 +++-
 src/tools/pgindent/typedefs.list             |   3 +
 17 files changed, 605 insertions(+), 21 deletions(-)
   9.9% doc/src/sgml/
  31.3% src/backend/utils/activity/
  22.0% src/backend/utils/adt/
   4.4% src/include/catalog/
   7.0% src/include/
  12.8% src/test/regress/expected/
  11.5% src/test/regress/sql/

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e0c8325a39..8afca9b110 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8403,9 +8403,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
         displayed in <link linkend="monitoring-pg-stat-database-view">
         <structname>pg_stat_database</structname></link>,
         <link linkend="monitoring-pg-stat-io-view">
-        <structname>pg_stat_io</structname></link>, in the output of
-        <xref linkend="sql-explain"/> when the <literal>BUFFERS</literal> option
-        is used, in the output of <xref linkend="sql-vacuum"/> when
+        <structname>pg_stat_io</structname></link>, in the output of the
+        <link linkend="pg-stat-get-backend-io">
+        <function>pg_stat_get_backend_io()</function></link> function, in the
+        output of <xref linkend="sql-explain"/> when the <literal>BUFFERS</literal>
+        option is used, in the output of <xref linkend="sql-vacuum"/> when
         the <literal>VERBOSE</literal> option is used, by autovacuum
         for auto-vacuums and auto-analyzes, when <xref
         linkend="guc-log-autovacuum-min-duration"/> is set and by
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 840d7f8161..4a9464da61 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4790,6 +4790,25 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        </para></entry>
       </row>
 
+      <row>
+       <entry id="pg-stat-get-backend-io" role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>pg_stat_get_backend_io</primary>
+        </indexterm>
+        <function>pg_stat_get_backend_io</function> ( <type>integer</type> )
+        <returnvalue>setof record</returnvalue>
+       </para>
+       <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,
+        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.
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
@@ -4971,6 +4990,24 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        </para></entry>
       </row>
 
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>pg_stat_reset_backend_stats</primary>
+        </indexterm>
+        <function>pg_stat_reset_backend_stats</function> ( <type>integer</type> )
+        <returnvalue>void</returnvalue>
+       </para>
+       <para>
+        Resets statistics for a single backend with the specified process ID
+        to zero.
+       </para>
+       <para>
+        This function is restricted to superusers by default, but other users
+        can be granted EXECUTE to run the function.
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index c51dfca802..14eb99cd47 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -711,6 +711,8 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public;
 
+REVOKE EXECUTE ON FUNCTION pg_stat_reset_backend_stats(integer) FROM public;
+
 REVOKE EXECUTE ON FUNCTION pg_stat_reset_replication_slot(text) FROM public;
 
 REVOKE EXECUTE ON FUNCTION pg_stat_have_stats(text, oid, int8) FROM public;
diff --git a/src/backend/utils/activity/Makefile b/src/backend/utils/activity/Makefile
index b9fd66ea17..24b64a2742 100644
--- a/src/backend/utils/activity/Makefile
+++ b/src/backend/utils/activity/Makefile
@@ -20,6 +20,7 @@ OBJS = \
 	backend_status.o \
 	pgstat.o \
 	pgstat_archiver.o \
+	pgstat_backend.o \
 	pgstat_bgwriter.o \
 	pgstat_checkpointer.o \
 	pgstat_database.o \
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 22c6dc378c..e9c3b87ac2 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -426,6 +426,10 @@ pgstat_bestart(void)
 
 	PGSTAT_END_WRITE_ACTIVITY(vbeentry);
 
+	/* Create the backend statistics entry */
+	if (pgstat_tracks_backend_bktype(MyBackendType))
+		pgstat_create_backend_stat(MyProcNumber);
+
 	/* Update app name to current GUC setting */
 	if (application_name)
 		pgstat_report_appname(application_name);
diff --git a/src/backend/utils/activity/meson.build b/src/backend/utils/activity/meson.build
index f73c22905c..380d3dd70c 100644
--- a/src/backend/utils/activity/meson.build
+++ b/src/backend/utils/activity/meson.build
@@ -5,6 +5,7 @@ backend_sources += files(
   'backend_status.c',
   'pgstat.c',
   'pgstat_archiver.c',
+  'pgstat_backend.c',
   'pgstat_bgwriter.c',
   'pgstat_checkpointer.c',
   'pgstat_database.c',
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 18b7d9b47d..9dd9d60b00 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -77,6 +77,7 @@
  *
  * Each statistics kind is handled in a dedicated file:
  * - pgstat_archiver.c
+ * - pgstat_backend.c
  * - pgstat_bgwriter.c
  * - pgstat_checkpointer.c
  * - pgstat_database.c
@@ -358,6 +359,22 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.reset_timestamp_cb = pgstat_subscription_reset_timestamp_cb,
 	},
 
+	[PGSTAT_KIND_BACKEND] = {
+		.name = "backend",
+
+		.fixed_amount = false,
+		.write_to_file = false,
+
+		.accessed_across_databases = true,
+
+		.shared_size = sizeof(PgStatShared_Backend),
+		.shared_data_off = offsetof(PgStatShared_Backend, stats),
+		.shared_data_len = sizeof(((PgStatShared_Backend *) 0)->stats),
+		.pending_size = sizeof(PgStat_BackendPendingIO),
+
+		.flush_pending_cb = pgstat_backend_flush_cb,
+		.reset_timestamp_cb = pgstat_backend_reset_timestamp_cb,
+	},
 
 	/* stats for fixed-numbered (mostly 1) objects */
 
@@ -602,6 +619,10 @@ pgstat_shutdown_hook(int code, Datum arg)
 	Assert(dlist_is_empty(&pgStatPending));
 	dlist_init(&pgStatPending);
 
+	/* drop the backend stats entry */
+	if (!pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber))
+		pgstat_request_entry_refs_gc();
+
 	pgstat_detach_shmem();
 
 #ifdef USE_ASSERT_CHECKING
@@ -768,7 +789,7 @@ pgstat_report_stat(bool force)
 
 	partial_flush = false;
 
-	/* flush database / relation / function / ... stats */
+	/* flush database / relation / function / backend / ... stats */
 	partial_flush |= pgstat_flush_pending_entries(nowait);
 
 	/* flush of fixed-numbered stats */
diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
new file mode 100644
index 0000000000..e2d83024c2
--- /dev/null
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -0,0 +1,182 @@
+/* -------------------------------------------------------------------------
+ *
+ * pgstat_backend.c
+ *	  Implementation of backend statistics.
+ *
+ * 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.
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *	  src/backend/utils/activity/pgstat_backend.c
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "utils/pgstat_internal.h"
+
+/*
+ * Returns backend's IO stats.
+ */
+PgStat_Backend *
+pgstat_fetch_proc_stat_io(ProcNumber procNumber)
+{
+	PgStat_Backend *backend_entry;
+
+	backend_entry = (PgStat_Backend *) pgstat_fetch_entry(PGSTAT_KIND_BACKEND,
+														  InvalidOid, procNumber);
+
+	return backend_entry;
+}
+
+/*
+ * Flush out locally pending backend statistics
+ *
+ * If no stats have been recorded, this function returns false.
+ */
+bool
+pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
+{
+	PgStatShared_Backend *shbackendioent;
+	PgStat_BackendPendingIO *pendingent;
+	PgStat_BktypeIO *bktype_shstats;
+
+	if (!pgstat_lock_entry(entry_ref, nowait))
+		return false;
+
+	shbackendioent = (PgStatShared_Backend *) entry_ref->shared_stats;
+	bktype_shstats = &shbackendioent->stats.stats;
+	pendingent = (PgStat_BackendPendingIO *) entry_ref->pending;
+
+	for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
+	{
+		for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+		{
+			for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+			{
+				instr_time	time;
+
+				bktype_shstats->counts[io_object][io_context][io_op] +=
+					pendingent->counts[io_object][io_context][io_op];
+
+				time = pendingent->pending_times[io_object][io_context][io_op];
+
+				bktype_shstats->times[io_object][io_context][io_op] +=
+					INSTR_TIME_GET_MICROSEC(time);
+			}
+		}
+	}
+
+	pgstat_unlock_entry(entry_ref);
+
+	return true;
+}
+
+/*
+ * Simpler wrapper of pgstat_backend_flush_cb()
+ */
+void
+pgstat_flush_backend(bool nowait)
+{
+	if (pgstat_tracks_backend_bktype(MyBackendType))
+	{
+		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);
+	}
+}
+
+/*
+ * Create the backend statistics entry for procnum.
+ */
+void
+pgstat_create_backend_stat(ProcNumber procnum)
+{
+	PgStat_EntryRef *entry_ref;
+	PgStatShared_Backend *shstatent;
+
+	entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_BACKEND, InvalidOid,
+										  procnum, NULL);
+
+	shstatent = (PgStatShared_Backend *) entry_ref->shared_stats;
+
+	/*
+	 * NB: need to accept that there might be stats from an older backend,
+	 * e.g. if we previously used this proc number.
+	 */
+	memset(&shstatent->stats, 0, sizeof(shstatent->stats));
+}
+
+/*
+ * Find or create a local PgStat_BackendPendingIO entry for procnum.
+ */
+PgStat_BackendPendingIO *
+pgstat_prep_backend_pending(ProcNumber procnum)
+{
+	PgStat_EntryRef *entry_ref;
+
+	entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_BACKEND, InvalidOid,
+										  procnum, NULL);
+
+	return entry_ref->pending;
+}
+
+/*
+ * Backend statistics are not collected for all BackendTypes.
+ *
+ * The following BackendTypes do not participate in the backend stats
+ * subsystem:
+ * - The same and for the same reasons as in pgstat_tracks_io_bktype().
+ * - B_BG_WRITER, B_CHECKPOINTER, B_STARTUP and B_AUTOVAC_LAUNCHER because their
+ * 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.
+ *
+ * When adding a new BackendType, also consider adding relevant restrictions to
+ * pgstat_tracks_io_object() and pgstat_tracks_io_op().
+ */
+bool
+pgstat_tracks_backend_bktype(BackendType bktype)
+{
+	/*
+	 * List every type so that new backend types trigger a warning about
+	 * needing to adjust this switch.
+	 */
+	switch (bktype)
+	{
+		case B_INVALID:
+		case B_AUTOVAC_LAUNCHER:
+		case B_DEAD_END_BACKEND:
+		case B_ARCHIVER:
+		case B_LOGGER:
+		case B_WAL_RECEIVER:
+		case B_WAL_WRITER:
+		case B_WAL_SUMMARIZER:
+		case B_BG_WRITER:
+		case B_CHECKPOINTER:
+		case B_STARTUP:
+			return false;
+
+		case B_AUTOVAC_WORKER:
+		case B_BACKEND:
+		case B_BG_WORKER:
+		case B_STANDALONE_BACKEND:
+		case B_SLOTSYNC_WORKER:
+		case B_WAL_SENDER:
+			return true;
+	}
+
+	return false;
+}
+
+void
+pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts)
+{
+	((PgStatShared_Backend *) header)->stats.stat_reset_timestamp = ts;
+}
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index f9883af2b3..e0c206a453 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -20,13 +20,7 @@
 #include "storage/bufmgr.h"
 #include "utils/pgstat_internal.h"
 
-
-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_PendingIO;
-
+typedef PgStat_BackendPendingIO PgStat_PendingIO;
 
 static PgStat_PendingIO PendingIOStats;
 static bool have_iostats = false;
@@ -87,6 +81,14 @@ pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint3
 	Assert((unsigned int) io_op < IOOP_NUM_TYPES);
 	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
 
+	if (pgstat_tracks_backend_bktype(MyBackendType))
+	{
+		PgStat_PendingIO *entry_ref;
+
+		entry_ref = pgstat_prep_backend_pending(MyProcNumber);
+		entry_ref->counts[io_object][io_context][io_op] += cnt;
+	}
+
 	PendingIOStats.counts[io_object][io_context][io_op] += cnt;
 
 	have_iostats = true;
@@ -148,6 +150,15 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
 
 		INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op],
 					   io_time);
+
+		if (pgstat_tracks_backend_bktype(MyBackendType))
+		{
+			PgStat_PendingIO *entry_ref;
+
+			entry_ref = pgstat_prep_backend_pending(MyProcNumber);
+			INSTR_TIME_ADD(entry_ref->pending_times[io_object][io_context][io_op],
+						   io_time);
+		}
 	}
 
 	pgstat_count_io_op_n(io_object, io_context, io_op, cnt);
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index faba8b64d2..85e65557bb 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -264,6 +264,7 @@ pgstat_report_vacuum(Oid tableoid, bool shared,
 	 * VACUUM command has processed all tables and committed.
 	 */
 	pgstat_flush_io(false);
+	pgstat_flush_backend(false);
 }
 
 /*
@@ -350,6 +351,7 @@ pgstat_report_analyze(Relation rel,
 
 	/* see pgstat_report_vacuum() */
 	pgstat_flush_io(false);
+	pgstat_flush_backend(false);
 }
 
 /*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index cdf37403e9..b939551d36 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1474,6 +1474,154 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 	return (Datum) 0;
 }
 
+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;
+
+	InitMaterializedSRF(fcinfo, 0);
+	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+	pid = PG_GETARG_INT32(0);
+	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().
+	 */
+	if (!proc)
+		return (Datum) 0;
+
+	procNumber = GetNumberFromPGProc(proc);
+	backend_stats = pgstat_fetch_proc_stat_io(procNumber);
+
+	if (!backend_stats)
+		return (Datum) 0;
+
+	bktype = B_INVALID;
+
+	/* Look for the backend type */
+	for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
+	{
+		LocalPgBackendStatus *local_beentry;
+		PgBackendStatus *beentry;
+
+		/* 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 */
+	if (bktype == B_INVALID)
+		return (Datum) 0;
+
+	bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
+	bktype_stats = &backend_stats->stats;
+	reset_time = TimestampTzGetDatum(backend_stats->stat_reset_timestamp);
+
+	/*
+	 * In Assert builds, we can afford an extra loop through all of the
+	 * counters checking that only expected stats are non-zero, since it keeps
+	 * the non-Assert code cleaner.
+	 */
+	Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+
+	for (int io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
+	{
+		const char *obj_name = pgstat_get_io_object_name(io_obj);
+
+		for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+		{
+			const char *context_name = pgstat_get_io_context_name(io_context);
+
+			Datum		values[IO_NUM_COLUMNS] = {0};
+			bool		nulls[IO_NUM_COLUMNS] = {0};
+
+			/*
+			 * Some combinations of BackendType, IOObject, and IOContext are
+			 * not valid for any type of IOOp. In such cases, omit the entire
+			 * row from the view.
+			 */
+			if (!pgstat_tracks_io_object(bktype, io_obj, io_context))
+				continue;
+
+			values[IO_COL_BACKEND_TYPE] = bktype_desc;
+			values[IO_COL_CONTEXT] = CStringGetTextDatum(context_name);
+			values[IO_COL_OBJECT] = CStringGetTextDatum(obj_name);
+			if (backend_stats->stat_reset_timestamp != 0)
+				values[IO_COL_RESET_TIME] = reset_time;
+			else
+				nulls[IO_COL_RESET_TIME] = true;
+
+			/*
+			 * Hard-code this to the value of BLCKSZ for now. Future values
+			 * could include XLOG_BLCKSZ, once WAL IO is tracked, and constant
+			 * multipliers, once non-block-oriented IO (e.g. temporary file
+			 * IO) is tracked.
+			 */
+			values[IO_COL_CONVERSION] = Int64GetDatum(BLCKSZ);
+
+			for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+			{
+				int			op_idx = pgstat_get_io_op_index(io_op);
+				int			time_idx = pgstat_get_io_time_index(io_op);
+
+				/*
+				 * Some combinations of BackendType and IOOp, of IOContext and
+				 * IOOp, and of IOObject and IOOp are not tracked. Set these
+				 * cells in the view NULL.
+				 */
+				if (pgstat_tracks_io_op(bktype, io_obj, io_context, io_op))
+				{
+					PgStat_Counter count =
+						bktype_stats->counts[io_obj][io_context][io_op];
+
+					values[op_idx] = Int64GetDatum(count);
+				}
+				else
+					nulls[op_idx] = true;
+
+				/* not every operation is timed */
+				if (time_idx == IO_COL_INVALID)
+					continue;
+
+				if (!nulls[op_idx])
+				{
+					PgStat_Counter time =
+						bktype_stats->times[io_obj][io_context][io_op];
+
+					values[time_idx] = Float8GetDatum(pg_stat_us_to_ms(time));
+				}
+				else
+					nulls[time_idx] = true;
+			}
+
+			tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
+								 values, nulls);
+		}
+	}
+
+	return (Datum) 0;
+}
+
 /*
  * Returns statistics of WAL activity
  */
@@ -1779,6 +1927,27 @@ pg_stat_reset_single_function_counters(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+Datum
+pg_stat_reset_backend_stats(PG_FUNCTION_ARGS)
+{
+	PGPROC	   *proc;
+	int			backend_pid = PG_GETARG_INT32(0);
+
+	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().
+	 */
+	if (!proc)
+		PG_RETURN_VOID();
+
+	pgstat_reset(PGSTAT_KIND_BACKEND, InvalidOid, GetNumberFromPGProc(proc));
+
+	PG_RETURN_VOID();
+}
+
 /* Reset SLRU counters (a specific one or all of them). */
 Datum
 pg_stat_reset_slru(PG_FUNCTION_ARGS)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0f22c21723..437157ffa3 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5913,6 +5913,15 @@
   proargnames => '{backend_type,object,context,reads,read_time,writes,write_time,writebacks,writeback_time,extends,extend_time,op_bytes,hits,evictions,reuses,fsyncs,fsync_time,stats_reset}',
   prosrc => 'pg_stat_get_io' },
 
+{ oid => '8806', descr => 'statistics: backend IO statistics',
+  proname => 'pg_stat_get_backend_io', prorows => '5', proretset => 't',
+  provolatile => 'v', proparallel => 'r', prorettype => 'record',
+  proargtypes => 'int4',
+  proallargtypes => '{int4,text,text,text,int8,float8,int8,float8,int8,float8,int8,float8,int8,int8,int8,int8,int8,float8,timestamptz}',
+  proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{backend_pid,backend_type,object,context,reads,read_time,writes,write_time,writebacks,writeback_time,extends,extend_time,op_bytes,hits,evictions,reuses,fsyncs,fsync_time,stats_reset}',
+  prosrc => 'pg_stat_get_backend_io' },
+
 { oid => '1136', descr => 'statistics: information about WAL activity',
   proname => 'pg_stat_get_wal', proisstrict => 'f', provolatile => 's',
   proparallel => 'r', prorettype => 'record', proargtypes => '',
@@ -6052,6 +6061,11 @@
   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',
+  proname => 'pg_stat_reset_backend_stats', provolatile => 'v',
+  prorettype => 'void', proargtypes => 'int4',
+  prosrc => 'pg_stat_reset_backend_stats' },
 { oid => '2307',
   descr => 'statistics: reset collected statistics for a single SLRU',
   proname => 'pg_stat_reset_slru', proisstrict => 'f', provolatile => 'v',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index ebfeef2f46..479773cfd2 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -49,14 +49,15 @@
 #define PGSTAT_KIND_FUNCTION	3	/* per-function statistics */
 #define PGSTAT_KIND_REPLSLOT	4	/* per-slot statistics */
 #define PGSTAT_KIND_SUBSCRIPTION	5	/* per-subscription statistics */
+#define PGSTAT_KIND_BACKEND	6	/* per-backend statistics */
 
 /* stats for fixed-numbered objects */
-#define PGSTAT_KIND_ARCHIVER	6
-#define PGSTAT_KIND_BGWRITER	7
-#define PGSTAT_KIND_CHECKPOINTER	8
-#define PGSTAT_KIND_IO	9
-#define PGSTAT_KIND_SLRU	10
-#define PGSTAT_KIND_WAL	11
+#define PGSTAT_KIND_ARCHIVER	7
+#define PGSTAT_KIND_BGWRITER	8
+#define PGSTAT_KIND_CHECKPOINTER	9
+#define PGSTAT_KIND_IO	10
+#define PGSTAT_KIND_SLRU	11
+#define PGSTAT_KIND_WAL	12
 
 #define PGSTAT_KIND_BUILTIN_MIN PGSTAT_KIND_DATABASE
 #define PGSTAT_KIND_BUILTIN_MAX PGSTAT_KIND_WAL
@@ -362,12 +363,23 @@ typedef struct PgStat_BktypeIO
 	PgStat_Counter times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
 } PgStat_BktypeIO;
 
+typedef struct PgStat_BackendPendingIO
+{
+	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;
+
 typedef struct PgStat_IO
 {
 	TimestampTz stat_reset_timestamp;
 	PgStat_BktypeIO stats[BACKEND_NUM_TYPES];
 } PgStat_IO;
 
+typedef struct PgStat_Backend
+{
+	TimestampTz stat_reset_timestamp;
+	PgStat_BktypeIO stats;
+} PgStat_Backend;
 
 typedef struct PgStat_StatDBEntry
 {
@@ -549,6 +561,13 @@ extern bool pgstat_have_entry(PgStat_Kind kind, Oid dboid, uint64 objid);
 extern void pgstat_report_archiver(const char *xlog, bool failed);
 extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void);
 
+/*
+ * Functions in pgstat_backend.c
+ */
+
+extern PgStat_Backend *pgstat_fetch_proc_stat_io(ProcNumber procNumber);
+extern bool pgstat_tracks_backend_bktype(BackendType bktype);
+extern void pgstat_create_backend_stat(ProcNumber procnum);
 
 /*
  * Functions in pgstat_bgwriter.c
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 7338bc1e28..811ed9b005 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -450,6 +450,11 @@ typedef struct PgStatShared_ReplSlot
 	PgStat_StatReplSlotEntry stats;
 } PgStatShared_ReplSlot;
 
+typedef struct PgStatShared_Backend
+{
+	PgStatShared_Common header;
+	PgStat_Backend stats;
+} PgStatShared_Backend;
 
 /*
  * Central shared memory entry for the cumulative stats system.
@@ -604,6 +609,15 @@ extern void pgstat_archiver_init_shmem_cb(void *stats);
 extern void pgstat_archiver_reset_all_cb(TimestampTz ts);
 extern void pgstat_archiver_snapshot_cb(void);
 
+/*
+ * Functions in pgstat_backend.c
+ */
+
+extern void pgstat_flush_backend(bool nowait);
+
+extern PgStat_BackendPendingIO *pgstat_prep_backend_pending(ProcNumber procnum);
+extern bool pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
+extern void pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts);
 
 /*
  * Functions in pgstat_bgwriter.c
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 56771f83ed..3447e7b75d 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1249,7 +1249,7 @@ SELECT pg_stat_get_subscription_stats(NULL);
  
 (1 row)
 
--- Test that the following operations are tracked in pg_stat_io:
+-- 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
@@ -1259,11 +1259,19 @@ SELECT pg_stat_get_subscription_stats(NULL);
 -- be sure of the state of shared buffers at the point the test is run.
 -- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
 -- extends.
+SELECT pid AS checkpointer_pid FROM pg_stat_activity
+  WHERE backend_type = 'checkpointer' \gset
 SELECT sum(extends) AS io_sum_shared_before_extends
   FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS my_io_sum_shared_before_extends
+  FROM pg_stat_get_backend_io(pg_backend_pid())
+  WHERE context = 'normal' AND object = 'relation' \gset
 SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
   FROM pg_stat_io
   WHERE object = 'relation' \gset io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+  FROM pg_stat_get_backend_io(pg_backend_pid())
+  WHERE object = 'relation' \gset my_io_sum_shared_before_
 CREATE TABLE test_io_shared(a int);
 INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
 SELECT pg_stat_force_next_flush();
@@ -1280,8 +1288,17 @@ SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
  t
 (1 row)
 
+SELECT sum(extends) AS my_io_sum_shared_after_extends
+  FROM pg_stat_get_backend_io(pg_backend_pid())
+  WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :my_io_sum_shared_after_extends > :my_io_sum_shared_before_extends;
+ ?column? 
+----------
+ t
+(1 row)
+
 -- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
--- and fsyncs.
+-- and fsyncs in the global stats (not for the backend).
 -- See comment above for rationale for two explicit CHECKPOINTs.
 CHECKPOINT;
 CHECKPOINT;
@@ -1301,6 +1318,31 @@ SELECT current_setting('fsync') = 'off'
  t
 (1 row)
 
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+  FROM pg_stat_get_backend_io(pg_backend_pid())
+  WHERE object = 'relation' \gset my_io_sum_shared_after_
+SELECT :my_io_sum_shared_after_writes >= :my_io_sum_shared_before_writes;
+ ?column? 
+----------
+ t
+(1 row)
+
+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);
+ ?column? 
+----------
+ 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
@@ -1521,6 +1563,8 @@ SELECT pg_stat_have_stats('io', 0, 0);
 
 SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_pre_reset
   FROM pg_stat_io \gset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_pre_reset
+  FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
 SELECT pg_stat_reset_shared('io');
  pg_stat_reset_shared 
 ----------------------
@@ -1535,6 +1579,30 @@ SELECT :io_stats_post_reset < :io_stats_pre_reset;
  t
 (1 row)
 
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_reset
+  FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+-- pg_stat_reset_shared() did not reset backend IO stats
+SELECT :my_io_stats_pre_reset <= :my_io_stats_post_reset;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- but pg_stat_reset_backend_stats() does
+SELECT pg_stat_reset_backend_stats(pg_backend_pid());
+ pg_stat_reset_backend_stats 
+-----------------------------
+ 
+(1 row)
+
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_backend_reset
+  FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
+ ?column? 
+----------
+ t
+(1 row)
+
 -- 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 7147cc2f89..9c925005be 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -595,7 +595,7 @@ 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:
+-- 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
@@ -607,20 +607,32 @@ SELECT pg_stat_get_subscription_stats(NULL);
 
 -- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
 -- extends.
+SELECT pid AS checkpointer_pid FROM pg_stat_activity
+  WHERE backend_type = 'checkpointer' \gset
 SELECT sum(extends) AS io_sum_shared_before_extends
   FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS my_io_sum_shared_before_extends
+  FROM pg_stat_get_backend_io(pg_backend_pid())
+  WHERE context = 'normal' AND object = 'relation' \gset
 SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
   FROM pg_stat_io
   WHERE object = 'relation' \gset io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+  FROM pg_stat_get_backend_io(pg_backend_pid())
+  WHERE object = 'relation' \gset my_io_sum_shared_before_
 CREATE TABLE test_io_shared(a int);
 INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
 SELECT pg_stat_force_next_flush();
 SELECT sum(extends) AS io_sum_shared_after_extends
   FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
 SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
+SELECT sum(extends) AS my_io_sum_shared_after_extends
+  FROM pg_stat_get_backend_io(pg_backend_pid())
+  WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :my_io_sum_shared_after_extends > :my_io_sum_shared_before_extends;
 
 -- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
--- and fsyncs.
+-- and fsyncs in the global stats (not for the backend).
 -- See comment above for rationale for two explicit CHECKPOINTs.
 CHECKPOINT;
 CHECKPOINT;
@@ -630,6 +642,17 @@ SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
 SELECT :io_sum_shared_after_writes > :io_sum_shared_before_writes;
 SELECT current_setting('fsync') = 'off'
   OR :io_sum_shared_after_fsyncs > :io_sum_shared_before_fsyncs;
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+  FROM pg_stat_get_backend_io(pg_backend_pid())
+  WHERE object = 'relation' \gset my_io_sum_shared_after_
+SELECT :my_io_sum_shared_after_writes >= :my_io_sum_shared_before_writes;
+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.
@@ -762,10 +785,21 @@ SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_ext
 SELECT pg_stat_have_stats('io', 0, 0);
 SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_pre_reset
   FROM pg_stat_io \gset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_pre_reset
+  FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
 SELECT pg_stat_reset_shared('io');
 SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_post_reset
   FROM pg_stat_io \gset
 SELECT :io_stats_post_reset < :io_stats_pre_reset;
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_reset
+  FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+-- pg_stat_reset_shared() did not reset backend IO stats
+SELECT :my_io_stats_pre_reset <= :my_io_stats_post_reset;
+-- but pg_stat_reset_backend_stats() does
+SELECT pg_stat_reset_backend_stats(pg_backend_pid());
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_backend_reset
+  FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;

 -- test BRIN index doesn't block HOT update
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ce33e55bf1..398dd92527 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2121,6 +2121,7 @@ PgFdwSamplingMethod
 PgFdwScanState
 PgIfAddrCallback
 PgStatShared_Archiver
+PgStatShared_Backend
 PgStatShared_BgWriter
 PgStatShared_Checkpointer
 PgStatShared_Common
@@ -2136,6 +2137,8 @@ PgStatShared_SLRU
 PgStatShared_Subscription
 PgStatShared_Wal
 PgStat_ArchiverStats
+PgStat_Backend
+PgStat_BackendPendingIO
 PgStat_BackendSubEntry
 PgStat_BgWriterStats
 PgStat_BktypeIO
-- 
2.34.1

---

From: Michael Paquier <[email-masked]>
Date: Tue, 17 Dec 2024 15:26:02 +0900

> return rows full of "invalid" combinations.

Okay.  I am not planning to fight more over this point.

> if that's the case. Done in v9 attached.

It seems to me that it could be possible if a different backend keeps
a reference to this entry.

> I miss something about your concern).

Looking closely at this one, I think that you're right as long as the
shutdown callback is registered before the entry is created.  So I'm
OK because the drop is a no-op if the entry cannot be found, and we
would not try a pgstat_request_entry_refs_gc().

Anyway, I have put my hands on v9.  A couple of notes, while hacking
through it.  See v9-0002 for the parts I have modified, that applies
on top of your v9-0001.

You may have noticed fee2b3ea2ecdg, which led me to fix two comments
as these paths are not related to only database objects.

Added more documentation, tweaked quite a bit the comments, a bit less
the docs (no need for the two linkends) and applied an indentation.

pg_stat_get_backend_io() can be simpler, and does not need the loop
with the extra LocalPgBackendStatus.  It is possible to call once
pgstat_get_beentry_by_proc_number() and retrieve the PID and the
bktype for the two sanity checks we want to do on them.

s/pgstat_fetch_proc_stat_io/pgstat_fetch_stat_backend/.
s/pgstat_create_backend_stat/pgstat_create_backend/.

I've been wondering for quite a bit about PgStat_BackendPendingIO and
PgStat_PendingIO, and concluded to define both in pgstat.h, with the
former being defined based on the latter to keep the dependency
between both at the same place.
--
Michael
From d0df4be6cd052fc91970ec99933268f9b55940b7 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Mon, 28 Oct 2024 12:50:32 +0000
Subject: [PATCH v9 1/2] per backend I/O statistics

 src/backend/utils/activity/pgstat.c          |  21 +++
 17 files changed, 604 insertions(+), 20 deletions(-)
 create mode 100644 src/backend/utils/activity/pgstat_backend.c

 

 

+/*
 
 /*

 /*
 
+/*
 
 /*

 

index b4e357c8a4..b72c779b2c 100644

 

 
-

 
 	have_iostats = true;

 
 /*

 /*
 
+Datum
 
+Datum
  
 (1 row)

 

 

index 24bd504c21..fbdd6ce574 100644
 
+      <row>
 
+      <row>
2.45.2

From 8c7fd33e98ec958308979dba9cb7897787ddc48e Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
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

---

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 17 Dec 2024 08:13:59 +0000

Hi,

On Tue, Dec 17, 2024 at 03:26:02PM +0900, Michael Paquier wrote:
> shutdown callback is registered before the entry is created.

Yeah, which is the case.

> through it.  See v9-0002 for the parts I have modified, that applies

Thanks!

> as these paths are not related to only database objects.

Yeap ;-)

> the docs (no need for the two linkends) and applied an indentation.

Looks good to me.

> bktype for the two sanity checks we want to do on them.

Looked at the changes you've done and agree that's simpler.

> s/pgstat_fetch_proc_stat_io/pgstat_fetch_stat_backend/.

Not sure about this one as being called in pg_stat_get_backend_io().

> s/pgstat_create_backend_stat/pgstat_create_backend/.

LGTM.

> between both at the same place.

That's fine by me.

Also:

=== 1

the changes in pgstat_flush_backend() make sense to me.

=== 2

Yeah, "proc number" looks better as already used in multiple places.

=== 3

the changes in stats.sql look good to me.

Having said that, v9-0002 looks good to me (except the pgstat_fetch_proc_stat_io
renaming).

Regards,

---

From: Michael Paquier <[email-masked]>
Date: Tue, 17 Dec 2024 18:13:44 +0900

> Having said that, v9-0002 looks good to me (except the pgstat_fetch_proc_stat_io

This routine returns a PgStat_Backend from a pgstats entry of type
PGSTAT_KIND_BACKEND, so the name in v9-0001 would not be true once
more types of stats data are attached to this structure.  The name in
v9-0002 is more consistent long-term.
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 17 Dec 2024 09:35:37 +0000

Hi,

On Tue, Dec 17, 2024 at 06:13:44PM +0900, Michael Paquier wrote:
> more types of stats data are attached to this structure.

Fair enough, let's go with the name change done in v9-0002.

> The name in

Agree, we may need to add parameters to it but we'll see when the time comes.

Regards,

---

From: Michael Paquier <[email-masked]>
Date: Wed, 18 Dec 2024 13:57:53 +0900

> Agree, we may need to add parameters to it but we'll see when the time comes.

Another thing that's been itching me: the loop in
pg_stat_get_backend_io() that exists as well in pg_stat_get_io() looks
worth refactoring in a single routine.  The only difference between
both is the reset timestamp, still both of them can pass a value for
it depending on their stats kind entry.  This shaves a bit more code
in your own patch, even if the check on pgstat_tracks_io_bktype() and
the Assert are not in the inner routine that fills the tuplestore with
the I/O data.  See pg_stat_fill_io_data() in v10-0002.  If you have a
better name for this routine, feel free..

What do you think about this refactoring?  This should come in first,
of course, so as the final patch introducing the backend stats is
easier to parse.
--
Michael
From 75a2549543edf418b6e282acab3f8ce272c5c42a Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Mon, 28 Oct 2024 12:50:32 +0000
Subject: [PATCH v10 1/2] per backend I/O statistics

 

 

 
+/*
 
 /*

 /*
 
+/*
 
 /*

 

 

 

-

 
 	have_iostats = true;

 
 /*

 /*
 
+Datum
 
+Datum
  
 (1 row)

 

 

 
+      <row>
 
+      <row>

From 0e3c2c066698acaaad69824a619e78f2cc16a358 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Wed, 18 Dec 2024 13:48:34 +0900
Subject: [PATCH v10 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         | 305 ++++++++------------
 src/test/regress/expected/stats.out         |  28 +-
 src/test/regress/sql/stats.sql              |  12 +-
 doc/src/sgml/monitoring.sgml                |  10 +-
 9 files changed, 184 insertions(+), 227 deletions(-)

 
 typedef struct PgStat_IO
 {

 
 /*

 
 /*

 
 /*
 
 /*

diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index b939551d36..18c74807a4 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1274,8 +1274,9 @@ pg_stat_get_buf_alloc(PG_FUNCTION_ARGS)
 }
 
 /*
-* When adding a new column to the pg_stat_io view, add a new enum value
-* here above IO_NUM_COLUMNS.
+* When adding a new column to the pg_stat_io view and the
+* pg_stat_get_backend_io() function, add a new enum value here above
+* IO_NUM_COLUMNS.
 */
 typedef enum io_stat_col
 {
@@ -1365,184 +1366,20 @@ pg_stat_us_to_ms(PgStat_Counter val_ms)
 	return val_ms * (double) 0.001;
 }
 
-Datum
-pg_stat_get_io(PG_FUNCTION_ARGS)
+/*
+ * pg_stat_fill_io_data
+ *
+ * Helper routine for pg_stat_get_io() and pg_stat_get_backend_io(),
+ * filling in a result tuplestore with one tuple for each object and each
+ * context supported by the caller, based on the contents of bktype_stats.
+ */
+static void
+pg_stat_fill_io_data(ReturnSetInfo *rsinfo,
+					 PgStat_BktypeIO *bktype_stats,
+					 BackendType bktype,
+					 TimestampTz stat_reset_timestamp)
 {
-	ReturnSetInfo *rsinfo;
-	PgStat_IO  *backends_io_stats;
-	Datum		reset_time;
-
-	InitMaterializedSRF(fcinfo, 0);
-	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
-
-	backends_io_stats = pgstat_fetch_stat_io();
-
-	reset_time = TimestampTzGetDatum(backends_io_stats->stat_reset_timestamp);
-
-	for (int bktype = 0; bktype < BACKEND_NUM_TYPES; bktype++)
-	{
-		Datum		bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
-		PgStat_BktypeIO *bktype_stats = &backends_io_stats->stats[bktype];
-
-		/*
-		 * In Assert builds, we can afford an extra loop through all of the
-		 * counters checking that only expected stats are non-zero, since it
-		 * keeps the non-Assert code cleaner.
-		 */
-		Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
-
-		/*
-		 * For those BackendTypes without IO Operation stats, skip
-		 * representing them in the view altogether.
-		 */
-		if (!pgstat_tracks_io_bktype(bktype))
-			continue;
-
-		for (int io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
-		{
-			const char *obj_name = pgstat_get_io_object_name(io_obj);
-
-			for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
-			{
-				const char *context_name = pgstat_get_io_context_name(io_context);
-
-				Datum		values[IO_NUM_COLUMNS] = {0};
-				bool		nulls[IO_NUM_COLUMNS] = {0};
-
-				/*
-				 * Some combinations of BackendType, IOObject, and IOContext
-				 * are not valid for any type of IOOp. In such cases, omit the
-				 * entire row from the view.
-				 */
-				if (!pgstat_tracks_io_object(bktype, io_obj, io_context))
-					continue;
-
-				values[IO_COL_BACKEND_TYPE] = bktype_desc;
-				values[IO_COL_CONTEXT] = CStringGetTextDatum(context_name);
-				values[IO_COL_OBJECT] = CStringGetTextDatum(obj_name);
-				values[IO_COL_RESET_TIME] = reset_time;
-
-				/*
-				 * Hard-code this to the value of BLCKSZ for now. Future
-				 * values could include XLOG_BLCKSZ, once WAL IO is tracked,
-				 * and constant multipliers, once non-block-oriented IO (e.g.
-				 * temporary file IO) is tracked.
-				 */
-				values[IO_COL_CONVERSION] = Int64GetDatum(BLCKSZ);
-
-				for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
-				{
-					int			op_idx = pgstat_get_io_op_index(io_op);
-					int			time_idx = pgstat_get_io_time_index(io_op);
-
-					/*
-					 * Some combinations of BackendType and IOOp, of IOContext
-					 * and IOOp, and of IOObject and IOOp are not tracked. Set
-					 * these cells in the view NULL.
-					 */
-					if (pgstat_tracks_io_op(bktype, io_obj, io_context, io_op))
-					{
-						PgStat_Counter count =
-							bktype_stats->counts[io_obj][io_context][io_op];
-
-						values[op_idx] = Int64GetDatum(count);
-					}
-					else
-						nulls[op_idx] = true;
-
-					/* not every operation is timed */
-					if (time_idx == IO_COL_INVALID)
-						continue;
-
-					if (!nulls[op_idx])
-					{
-						PgStat_Counter time =
-							bktype_stats->times[io_obj][io_context][io_op];
-
-						values[time_idx] = Float8GetDatum(pg_stat_us_to_ms(time));
-					}
-					else
-						nulls[time_idx] = true;
-				}
-
-				tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
-									 values, nulls);
-			}
-		}
-	}
-
-	return (Datum) 0;
-}
-
-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;
-
-	InitMaterializedSRF(fcinfo, 0);
-	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
-
-	pid = PG_GETARG_INT32(0);
-	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().
-	 */
-	if (!proc)
-		return (Datum) 0;
-
-	procNumber = GetNumberFromPGProc(proc);
-	backend_stats = pgstat_fetch_proc_stat_io(procNumber);
-
-	if (!backend_stats)
-		return (Datum) 0;
-
-	bktype = B_INVALID;
-
-	/* Look for the backend type */
-	for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
-	{
-		LocalPgBackendStatus *local_beentry;
-		PgBackendStatus *beentry;
-
-		/* 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 */
-	if (bktype == B_INVALID)
-		return (Datum) 0;
-
-	bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
-	bktype_stats = &backend_stats->stats;
-	reset_time = TimestampTzGetDatum(backend_stats->stat_reset_timestamp);
-
-	/*
-	 * In Assert builds, we can afford an extra loop through all of the
-	 * counters checking that only expected stats are non-zero, since it keeps
-	 * the non-Assert code cleaner.
-	 */
-	Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+	Datum		bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
 
 	for (int io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
 	{
@@ -1566,8 +1403,8 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS)
 			values[IO_COL_BACKEND_TYPE] = bktype_desc;
 			values[IO_COL_CONTEXT] = CStringGetTextDatum(context_name);
 			values[IO_COL_OBJECT] = CStringGetTextDatum(obj_name);
-			if (backend_stats->stat_reset_timestamp != 0)
-				values[IO_COL_RESET_TIME] = reset_time;
+			if (stat_reset_timestamp != 0)
+				values[IO_COL_RESET_TIME] = TimestampTzGetDatum(stat_reset_timestamp);
 			else
 				nulls[IO_COL_RESET_TIME] = true;
 
@@ -1618,7 +1455,104 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS)
 								 values, nulls);
 		}
 	}
+}
 
+Datum
+pg_stat_get_io(PG_FUNCTION_ARGS)
+{
+	ReturnSetInfo *rsinfo;
+	PgStat_IO  *backends_io_stats;
+
+	InitMaterializedSRF(fcinfo, 0);
+	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+	backends_io_stats = pgstat_fetch_stat_io();
+
+	for (int bktype = 0; bktype < BACKEND_NUM_TYPES; bktype++)
+	{
+		PgStat_BktypeIO *bktype_stats = &backends_io_stats->stats[bktype];
+
+		/*
+		 * In Assert builds, we can afford an extra loop through all of the
+		 * counters checking that only expected stats are non-zero, since it
+		 * keeps the non-Assert code cleaner.
+		 */
+		Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+
+		/*
+		 * For those BackendTypes without IO Operation stats, skip
+		 * representing them in the view altogether.
+		 */
+		if (!pgstat_tracks_io_bktype(bktype))
+			continue;
+
+		/* Save tuples with data from this PgStat_BktypeIO. */
+		pg_stat_fill_io_data(rsinfo, bktype_stats, bktype,
+							 backends_io_stats->stat_reset_timestamp);
+	}
+
+	return (Datum) 0;
+}
+
+/*
+ * Returns I/O statistics for a backend with given PID.
+ */
+Datum
+pg_stat_get_backend_io(PG_FUNCTION_ARGS)
+{
+	ReturnSetInfo *rsinfo;
+	BackendType bktype;
+	int			pid;
+	PGPROC	   *proc;
+	ProcNumber	procNumber;
+	PgStat_Backend *backend_stats;
+	PgStat_BktypeIO *bktype_stats;
+	PgBackendStatus *beentry;
+
+	InitMaterializedSRF(fcinfo, 0);
+	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+	pid = PG_GETARG_INT32(0);
+	proc = BackendPidGetProc(pid);
+
+	/*
+	 * 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_stat_backend(procNumber);
+
+	if (!backend_stats)
+		return (Datum) 0;
+
+	beentry = pgstat_get_beentry_by_proc_number(procNumber);
+	bktype = beentry->st_backendType;
+
+	/* If PID does not match, leave */
+	if (beentry->st_procpid != pid)
+		return (Datum) 0;
+
+	/* Backend may be gone, so recheck in case */
+	if (bktype == B_INVALID)
+		return (Datum) 0;
+
+	bktype_stats = &backend_stats->stats;
+
+	/*
+	 * In Assert builds, we can afford an extra loop through all of the
+	 * counters checking that only expected stats are non-zero, since it keeps
+	 * the non-Assert code cleaner.
+	 */
+	Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+
+	/* Save tuples with data from this PgStat_BktypeIO */
+	pg_stat_fill_io_data(rsinfo, bktype_stats, bktype,
+						 backend_stats->stat_reset_timestamp);
 	return (Datum) 0;
 }
 
@@ -1927,6 +1861,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 +1873,9 @@ pg_stat_reset_backend_stats(PG_FUNCTION_ARGS)
 	proc = BackendPidGetProc(backend_pid);
 
 	/*
  
 (1 row)

 

 

-- 
2.45.2

---

From: Bertrand Drouvot <[email-masked]>
Date: Wed, 18 Dec 2024 08:11:55 +0000

Hi,

On Wed, Dec 18, 2024 at 01:57:53PM +0900, Michael Paquier wrote:
> it depending on their stats kind entry.

Yeah, I also had something like this in mind (see R2 in [1]). So, +1 for it.

> This shaves a bit more code
> the I/O data.

Yeah, that's fine by me.

> See pg_stat_fill_io_data() in v10-0002.  If you have a

I think I prefer pg_stat_io_build_tuples() and used that name in v11
attached.

> What do you think about this refactoring?

Makes fully sense to me (as per my first comment above).

> This should come in first,

Yeah, it's in 0001 attached, with a few changes:

=== 1

s/Save tuples with data from this PgStat_BktypeIO./save tuples with data from this PgStat_BktypeIO/

to be consistent with single line comments around.

=== 2

This is not necessary until the per backend I/O stat is introduced but I 
added it in 0001 to ease 0002 parsing.

=== 3

"In Assert builds, we can afford an extra loop through all of the"

I think that this comment is now confusing since the extra loop would be
done in pg_stat_io_build_tuples() and not where the comment is written. One
option could have been to move the assert and the comment in pg_stat_io_build_tuples()
but I think it's better to have the assert before the pgstat_tracks_io_bktype()
call in pg_stat_get_io(), so modified the comment a bit instead.

0002 is the per-backend stats I/O with yours tweaks.

Regards,

>From 636d82991ec029c9d792d54cfdb2c665063424cf Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Wed, 18 Dec 2024 05:45:49 +0000
Subject: [PATCH v11 1/2] Refactor pg_stat_get_io() to extract tuple building
 logic

Adding pg_stat_io_build_tuples(), a helper for pg_stat_get_io() filling in a
result tuplestore. The tuple building logic is now isolated in its own function,
so that it could be reused elsewhere (like in the following backend I/O stats
commit).
---
 src/backend/utils/adt/pgstatfuncs.c | 174 ++++++++++++++++------------
 1 file changed, 97 insertions(+), 77 deletions(-)
 100.0% src/backend/utils/adt/

diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index cdf37403e9..efd35f700a 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1365,29 +1365,117 @@ pg_stat_us_to_ms(PgStat_Counter val_ms)
 	return val_ms * (double) 0.001;
 }
 
+/*
+ * pg_stat_io_build_tuples
+ *
+ * Helper routine for pg_stat_get_io() filling in a result tuplestore with one
+ * tuple for each object and each context supported by the caller, based on the
+ * contents of bktype_stats.
+ */
+static void
+pg_stat_io_build_tuples(ReturnSetInfo *rsinfo,
 pg_stat_get_io(PG_FUNCTION_ARGS)
 {
 	ReturnSetInfo *rsinfo;
 	PgStat_IO  *backends_io_stats;
-	Datum		reset_time;

 	backends_io_stats = pgstat_fetch_stat_io();
 
-	reset_time = TimestampTzGetDatum(backends_io_stats->stat_reset_timestamp);
-
 	for (int bktype = 0; bktype < BACKEND_NUM_TYPES; bktype++)
 	{
-		Datum		bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
 		PgStat_BktypeIO *bktype_stats = &backends_io_stats->stats[bktype];
 
 		/*
 		 * In Assert builds, we can afford an extra loop through all of the
-		 * counters checking that only expected stats are non-zero, since it
-		 * keeps the non-Assert code cleaner.
+		 * counters (in pg_stat_io_build_tuples()), checking that only
+		 * expected stats are non-zero, since it keeps the non-Assert code
+		 * cleaner.
 		 */
 		Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
 
@@ -1398,77 +1486,9 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 		if (!pgstat_tracks_io_bktype(bktype))
 			continue;
 
+		/* save tuples with data from this PgStat_BktypeIO */
+		pg_stat_io_build_tuples(rsinfo, bktype_stats, bktype,
 
 	return (Datum) 0;
-- 
2.34.1

>From 6447dbe922e1db379dfd65f8041bc55cf3a3c573 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Wed, 18 Dec 2024 06:39:02 +0000
Subject: [PATCH v11 2/2] per backend I/O statistics

XXX: Requires a bump of CATALOG_VERSION_NO.
XXX: Requires a bump of PGSTAT_FILE_FORMAT_ID.
---
 doc/src/sgml/config.sgml                     |   8 +-
 doc/src/sgml/monitoring.sgml                 |  39 ++++
 src/backend/catalog/system_functions.sql     |   2 +
 src/backend/utils/activity/Makefile          |   1 +
 src/backend/utils/activity/backend_status.c  |   4 +
 src/backend/utils/activity/meson.build       |   1 +
 src/backend/utils/activity/pgstat.c          |  21 +++
 src/backend/utils/activity/pgstat_backend.c  | 188 +++++++++++++++++++
 src/backend/utils/activity/pgstat_io.c       |  25 ++-
 src/backend/utils/activity/pgstat_relation.c |   2 +
 src/backend/utils/adt/pgstatfuncs.c          |  97 +++++++++-
 src/include/catalog/pg_proc.dat              |  13 ++
 src/include/pgstat.h                         |  34 +++-
 src/include/utils/pgstat_internal.h          |  14 ++
 src/test/regress/expected/stats.out          |  82 +++++++-
 src/test/regress/sql/stats.sql               |  42 ++++-
 src/tools/pgindent/typedefs.list             |   3 +
 17 files changed, 549 insertions(+), 27 deletions(-)
  10.3% doc/src/sgml/
  34.1% src/backend/utils/activity/
  14.7% src/backend/utils/adt/
   4.7% src/include/catalog/
   8.1% src/include/
  14.6% src/test/regress/expected/
  12.4% src/test/regress/sql/

index 840d7f8161..d0d176cc54 100644
@@ -4790,6 +4790,27 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        </para></entry>
       </row>
 
+      <row>
@@ -4971,6 +4992,24 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        </para></entry>
       </row>
 
+      <row>

 
index 22c6dc378c..bf33e33a4e 100644

 

 
 
index 0000000000..6b2c9baa8c
@@ -0,0 +1,188 @@
index f9883af2b3..011a3326da 100644
@@ -20,14 +20,6 @@
 
-
 
@@ -87,6 +79,14 @@ pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint3
 	Assert((unsigned int) io_op < IOOP_NUM_TYPES);
 	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));

 	have_iostats = true;
@@ -148,6 +148,15 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,

 
 /*

 /*
index efd35f700a..8387e11a93 100644
 
 /*
-* When adding a new column to the pg_stat_io view, add a new enum value
-* here above IO_NUM_COLUMNS.
+* When adding a new column to the pg_stat_io view and the
+* pg_stat_get_backend_io() function, add a new enum value here above
+* IO_NUM_COLUMNS.
 */
 typedef enum io_stat_col
 {
@@ -1368,9 +1369,9 @@ pg_stat_us_to_ms(PgStat_Counter val_ms)
 /*
  * pg_stat_io_build_tuples
  *
- * Helper routine for pg_stat_get_io() filling in a result tuplestore with one
- * tuple for each object and each context supported by the caller, based on the
- * contents of bktype_stats.
+ * Helper routine for pg_stat_get_io() and pg_stat_get_backend_io(),
+ * filling in a result tuplestore with one tuple for each object and each
+ * context supported by the caller, based on the contents of bktype_stats.
  */
 static void
 pg_stat_io_build_tuples(ReturnSetInfo *rsinfo,
@@ -1494,6 +1495,68 @@ 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;
+	BackendType bktype;
+	int			pid;
+	PGPROC	   *proc;
+	ProcNumber	procNumber;
+	PgStat_Backend *backend_stats;
+	PgStat_BktypeIO *bktype_stats;
+	PgBackendStatus *beentry;
+
+	InitMaterializedSRF(fcinfo, 0);
+	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+	pid = PG_GETARG_INT32(0);
+	proc = BackendPidGetProc(pid);
+
+	/*
+	 * 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_stat_backend(procNumber);
+
+	if (!backend_stats)
+		return (Datum) 0;
+
+	beentry = pgstat_get_beentry_by_proc_number(procNumber);
+	bktype = beentry->st_backendType;
+
+	/* if PID does not match, leave */
+	if (beentry->st_procpid != pid)
+		return (Datum) 0;
+
+	/* backend may be gone, so recheck in case */
+	if (bktype == B_INVALID)
+		return (Datum) 0;
+
+	bktype_stats = &backend_stats->stats;
+
+	/*
+	 * In Assert builds, we can afford an extra loop through all of the
+	 * counters (in pg_stat_io_build_tuples()), checking that only expected
+	 * stats are non-zero, since it keeps the non-Assert code cleaner.
+	 */
+	Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+
+	/* save tuples with data from this PgStat_BktypeIO */
+	pg_stat_io_build_tuples(rsinfo, bktype_stats, bktype,
+							backend_stats->stat_reset_timestamp);
+	return (Datum) 0;
+}
+
 /*
  * Returns statistics of WAL activity
  */
@@ -1799,6 +1862,30 @@ pg_stat_reset_single_function_counters(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+/*
index 0f22c21723..2dcc2d42da 100644
 
@@ -6052,6 +6061,10 @@
index ebfeef2f46..56ed1b8bb3 100644

 #define PGSTAT_KIND_BUILTIN_MIN PGSTAT_KIND_DATABASE
 #define PGSTAT_KIND_BUILTIN_MAX PGSTAT_KIND_WAL
@@ -362,12 +363,26 @@ typedef struct PgStat_BktypeIO
 	PgStat_Counter times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
 } PgStat_BktypeIO;

 
 typedef struct PgStat_StatDBEntry
 {
@@ -549,6 +564,13 @@ extern bool pgstat_have_entry(PgStat_Kind kind, Oid dboid, uint64 objid);
 extern void pgstat_report_archiver(const char *xlog, bool failed);
 extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void);
 
+/*
 
 /*

 /*
 
+/*
 
 /*
index 56771f83ed..150b6dcf74 100644
  
 (1 row)
 
@@ -1259,11 +1260,19 @@ SELECT pg_stat_get_subscription_stats(NULL);
@@ -1280,8 +1289,17 @@ SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
  t
 (1 row)
 
@@ -1301,6 +1319,23 @@ SELECT current_setting('fsync') = 'off'
  t
 (1 row)
 
@@ -1521,6 +1556,8 @@ SELECT pg_stat_have_stats('io', 0, 0);
 
@@ -1535,6 +1572,47 @@ SELECT :io_stats_post_reset < :io_stats_pre_reset;
  t
 (1 row)
 
index 7147cc2f89..1e7d0ff665 100644

@@ -607,20 +608,32 @@ SELECT pg_stat_get_subscription_stats(NULL);

@@ -630,6 +643,13 @@ SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
 
@@ -762,11 +782,27 @@ SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_ext

---

From: Bertrand Drouvot <[email-masked]>
Date: Wed, 18 Dec 2024 08:16:12 +0000

> Yeah, I also had something like this in mind (see R2 in [1]). So, +1 for it.

[1] being: https://www.postgresql.org/message-id/Zy4bmvgHqGjcK1pI%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

---

From: Michael Paquier <[email-masked]>
Date: Thu, 19 Dec 2024 13:21:54 +0900

> I think I prefer pg_stat_io_build_tuples() and used that name in v11

I'll rely to your naming sense than mine, then :D

> 0002 is the per-backend stats I/O with yours tweaks.

While doing more tests with backends exiting concurrently with their
stats scanned, I have detected one path in pg_stat_get_backend_io()
after calling pgstat_get_beentry_by_proc_number() where we should also
check that it does not return NULL, or we would crash on a pointer
dereference when reading the backend type.

Fixed that, bumped the two version counters, and done.
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Thu, 19 Dec 2024 06:12:04 +0000

Hi,

On Thu, Dec 19, 2024 at 01:21:54PM +0900, Michael Paquier wrote:
> Fixed that,

Oh right, indeed all the others pgstat_get_beentry_by_proc_number() callers are
checking for a NULL returned value.

> bumped the two version counters, and done.

Thanks!

I think I'll start a dedicated thread to discuss the stats_fetch_consistency/'snapshot'
point (will be easier to follow than resuming the discussion in this thread).

Regards,

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 20 Dec 2024 09:57:19 +0000

Hi,

> >    The existing structure could be expanded in the
> >    future to add more information about other statistics related to
> >    backends, depending on requirements or ideas.

BTW, now that the per backend I/O statistics is done, I'll start working on per
backend wal statistics.

Regards,

---

From: Michael Paquier <[email-masked]>
Date: Tue, 24 Dec 2024 14:35:16 +0900

> BTW, now that the per backend I/O statistics is done, I'll start working on per

I think that this is a good idea.  It does not actually overlap the
proposal in [1] as the stats persistency is not the same.  What this
thread has taught me is that you could just plug in the stats of the
new backend-level structure a PgStat_WalStats and retrieve them with a
new function that returns a single tuple with the WAL stats
attributes.  That should be rather straight-forward to achieve.

[1]: https://commitfest.postgresql.org/51/4950/
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 3 Jan 2025 10:48:41 +0000

Hi,

On Tue, Dec 24, 2024 at 02:35:16PM +0900, Michael Paquier wrote:
> I think that this is a good idea.

Thanks for sharing your thoughts.

> It does not actually overlap the

I started to look at it and should be able to share a patch next week.

Regards,

---

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 7 Jan 2025 08:54:59 +0000

Hi,

> I started to look at it and should be able to share a patch next week.

A dedicated thread and a patch have been created, see [1].

[1]: https://www.postgresql.org/message-id/flat/Z3zqc4o09dM/Ezyz%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

---

From: Michael Paquier <[email-masked]>
Date: Wed, 8 Jan 2025 08:54:40 +0900

> [1]: https://www.postgresql.org/message-id/flat/Z3zqc4o09dM/Ezyz%40ip-10-97-1-34.eu-west-3.compute.internal

Hmm.  I can see that you have some refactoring pieces in the lines of
some stuff I wanted to structure for the sake of this thread, but I
have discarded them to simplify the final result at the end and it
did not change much.

Will look at all that on this other thread, no need to start a
separate thread just for the sake of the refactoring bits.
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 10 Jan 2025 13:56:01 +0000

Hi,

> point (will be easier to follow than resuming the discussion in this thread).

I gave more thoughts on it and did some research in the mailing list.

I've read those comments, from [1]:

"
We had a lot of discussion around what consistency model we want, and Tom was
adamant that there needs to be a mode that behaves like the current consistency
model (which is what snapshot behaves like, with very minor differences)
"

and from [2]:

"
One thing we definitely need to add documentation for is the
stats_fetch_consistency GUC. I think we should change its default to 'cache',
because that still gives the ability to "self-join", without the cost of the
current method.
"

So now, I think that it is not a good idea for the per backend stats to 
behave differently than the other stats kinds. I think we should get rid of
the "snapshot" for all of them or for none of them. Reading the above comments,
it looks like it has already been discussed and that the outcome is to keep
it for all of them.

Regards,

[1]: https://www.postgresql.org/message-id/20220404210610.6dgbwzpbiveyl5y3%40alap3.anarazel.de
[2]: https://www.postgresql.org/message-id/20220329072651.imjqtzxwk6xycojv%40alap3.anarazel.de

---

From: Michael Paquier <[email-masked]>
Date: Tue, 14 Jan 2025 13:56:13 +0900

> it looks like it has already been discussed and that the outcome is to keep

Thanks a lot for diving into these details!  I did not recall that
there was a strict argument for the current behavior, even if I was
pretty sure that introducing an exception for the backend stats was
not quite right compared to the other stats kinds.  The consistency
line prevails here, then.
--
Michael

---

From: Nazir Bilal Yavuz <[email-masked]>
Date: Wed, 15 Jan 2025 11:03:54 +0300

Hi,

> Fixed that, bumped the two version counters, and done.

I encountered a problem while trying to add WAL stats to pg_stat_io
and I wanted to hear your thoughts.

Right now, pgstat_prep_backend_pending() is called in both
pgstat_count_io_op() and pgstat_count_io_op_time() to create a local
PgStat_BackendPending entry. In that process,
pgstat_prep_pending_entry() -> MemoryContextAllocZero() is called. The
problem is that MemoryContextAllocZero() can not be called in the
critical sections.

For example, here is what happens in the walsender backend:

'''
... ->
exec_replication_command() ->
SendBaseBackup() ->
...  ->
XLogInsertRecord() ->
START_CRIT_SECTION() /* Now we are in the critical section */ ->
... ->
XLogWrite() ->
pgstat_count_io_op_time() for the pg_pwrite() IO ->
pgstat_prep_backend_pending() ->
pgstat_prep_pending_entry() ->
MemoryContextAllocZero() ->
Failed at Assert("CritSectionCount == 0 || (context)->allowInCritSection")
'''

With this commit it may not be possible to count IOs in the critical
sections. I think the problem happens only if the local
PgStat_BackendPending entry is being created for the first time for
this backend in the critical section.

--
Regards,
Nazir Bilal Yavuz
Microsoft

---

From: Bertrand Drouvot <[email-masked]>
Date: Wed, 15 Jan 2025 08:30:20 +0000

Hi,

> this backend in the critical section.

Yeah, I encountered the exact same thing and mentioned it in [1] (see R1.).

In [1] I did propose to use a new PendingBackendWalStats variable to "bypass" the
pgstat_prep_backend_pending() usage.

Michael mentioned in [2] that is not really consistent with the rest (what I
agree with) and that "we should rethink a bit the way pending entries are
retrieved". I did not think about it yet but that might be the way to
go, thoughts? 

[1]: https://www.postgresql.org/message-id/Z3zqc4o09dM/Ezyz%40ip-10-97-1-34.eu-west-3.compute.internal
[2]: https://www.postgresql.org/message-id/Z4dRlNuhSQ3hPPv2%40paquier.xyz

Regards,

---

From: Michael Paquier <[email-masked]>
Date: Wed, 15 Jan 2025 18:27:22 +0900

> [2]: https://www.postgresql.org/message-id/Z4dRlNuhSQ3hPPv2%40paquier.xyz

My problem is that this is not only related to backend stats, but to
all variable-numbered stats kinds that require this behavior.  One
other case where I've seen this as being an issue is injection point
stats, for example, while enabling these stats for all callbacks and
some of them are run in critical sections.

A generic solution to the problem would be to allow
pgStatPendingContext to have MemoryContextAllowInCriticalSection() set
temporarily for the allocation of the pending data.
Perhaps not for all stats kinds, just for these where we're OK with
the potential memory footprint so we could have a flag in
PgStat_KindInfo.  Giving to all stats kinds the responsibility to
allocate a pending entry beforehand outside a critical section is
another option, but that means going through different tweaks for all
stats kinds that require them.

Andres, do you have any thoughts about that?  Not sure what would be 
your view on this matter.  MemoryContextAllowInCriticalSection() for
stats kinds that allow the case would be OK for me I guess, we should
not be talking about a lot of memory.
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Wed, 15 Jan 2025 10:55:44 +0000

Hi,

On Wed, Jan 15, 2025 at 06:27:22PM +0900, Michael Paquier wrote:
> all variable-numbered stats kinds that require this behavior.

Yeah, agree that it's better to have a "generic" solution.

> stats kinds that allow the case would be OK for me I guess,

Same here, that looks ok to me too.

Regards,

---

From: Nazir Bilal Yavuz <[email-masked]>
Date: Wed, 15 Jan 2025 17:20:57 +0300

Hi,

> > [2]: https://www.postgresql.org/message-id/Z4dRlNuhSQ3hPPv2%40paquier.xyz

Thanks! I must have missed these emails.

> stats kinds that require them.

I think allowing only pgStatPendingContext to have
MemoryContextAllowInCriticalSection() is not enough. We need to allow
at least pgStatSharedRefContext as well to have
MemoryContextAllowInCriticalSection() as it can be allocated too.

'''
pgstat_prep_pending_entry() ->
pgstat_get_entry_ref() ->
pgstat_get_entry_ref_cached() ->
MemoryContextAlloc(pgStatSharedRefContext, sizeof(PgStat_EntryRef))
'''

Also, I encountered another problem. I did not write it in the first
email because I thought I fixed it but it seems I did not.

While doing the initdb, we are restoring stats with the
pgstat_restore_stats() and we do not expect any pending stats. The
problem goes like that:

'''
pgstat_restore_stats() ->
pgstat_read_statsfile() ->
pgstat_reset_after_failure() ->
pgstat_drop_all_entries() ->
pgstat_drop_entry_internal() ->
We have an assertion there which checks if there is a pending stat entry:

    /* should already have released local reference */
    if (pgStatEntryRefHash)
        Assert(!pgstat_entry_ref_hash_lookup(pgStatEntryRefHash, shent->key));
'''

And we have this at the same time:

'''
BootstrapModeMain() ->
InitPostgres() ->
StartupXLOG() ->
ReadCheckpointRecord() ->
InitWalRecovery() ->
... ->
XLogReadAhead() ->
XLogDecodeNextRecord() ->
ReadPageInternal() ->
state->routine.page_read = XLogPageRead() then WAL read happens
'''

So, this assert fails because we have pending stats for the
PGSTAT_KIND_BACKEND. My fix was simply not restoring stats when the
bootstrap is happening; but it did not work. Because, we reset all
fixed-numbered stats 'pgstat_reset_after_failure() ->
kind_info->reset_all_cb()' there, so if we skip it; we have some NULL
stats, for example reset timestamps. Some of the tests do not expect
them to be NULL at the start like below:

SELECT stats_reset AS archiver_reset_ts FROM pg_stat_archiver \gset
SELECT pg_stat_reset_shared('archiver');
SELECT stats_reset > :'archiver_reset_ts'::timestamptz FROM pg_stat_archiver;

This test fails because archiver_reset_ts is NULL when the server is started.

I was a bit surprised that Bertrand did not encounter the same problem
while working on the 'per backend WAL statistics' patch. Then I found
the reason, it is because this problem happens only for WAL read and
WAL init IOs which are starting to be tracked in my patch. By saying
that, I could not decide which thread to write about this problem,
migrating WAL stats thread or this thread. Since this thread is active
and other stats may cause the same problem, I decided to write here.
Please warn me if you think I need to write this to the migrating WAL
stats thread.

[1]
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index bf3dbda901d..0538859cc7f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5681,7 +5681,7 @@ StartupXLOG(void)
      */
     if (didCrash)
         pgstat_discard_stats();
-    else
+    else if (!IsBootstrapProcessingMode())
         pgstat_restore_stats(checkPoint.redo);

--
Regards,
Nazir Bilal Yavuz
Microsoft

---

From: Bertrand Drouvot <[email-masked]>
Date: Wed, 15 Jan 2025 16:00:12 +0000

Hi,

> Please warn me if you think I need to write this to the migrating WAL

Thanks for the report! As it looks like that you hit the issue for "WAL read and
WAL init IOs which are starting to be tracked in my patch", then I think it would
make more sense to mention the issue in the "migrating WAL stats thread" so that
one could try to reproduce it (with a patch producing the issue at hands).

So it seems to me that the "migrating WAL stats thread" is a better place to
discuss about it.

Regards,

---

From: Michael Paquier <[email-masked]>
Date: Thu, 16 Jan 2025 09:55:10 +0900

> MemoryContextAlloc(pgStatSharedRefContext, sizeof(PgStat_EntryRef))

Yep, I was pretty sure that we have a bit more going on.  Last time I
began digging into the issue I was loading injection_points in
shared_preload_libraries with stats enabled to see how much I could
break, and this was one pattern once I've forced the pending part.  I
didn't get through the whole exercise.

>         Assert(!pgstat_entry_ref_hash_lookup(pgStatEntryRefHash, shent->key));

You mean that this is not something that happens on HEAD, but as an
effect of trying to add WAL stats to pg_stat_io, right?

> Please warn me if you think I need to write this to the migrating WAL

I'd rather treat that on a separate thread (please add me in CC if I'm
not there yet!).  I am OK to discuss any issues related to backend
statistics here if HEAD is impacted based on how the feature is
limited currently now, though.

Another, simpler, idea would be to force more calls
pgstat_prep_backend_pending() where this is problematic, like before
entering a critical section when generating a WAL record.  I am not
sure that it would take care of everything, as you mention; that would
leave some holes depending on the kind of interactions with the
pgstats dshash we do in these paths.
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Thu, 16 Jan 2025 06:48:58 +0000

Hi,

> didn't get through the whole exercise.

I'll look at it and come back with a proposal as part of [1].

[1]: https://www.postgresql.org/message-id/[email-masked]

Regards,

---

From: Bertrand Drouvot <[email-masked]>
Date: Thu, 16 Jan 2025 16:05:35 +0000

Hi,

> I'll look at it and come back with a proposal as part of [1].

I looked at it and shared the outcome in [1]. It looks like it's more complicated
as we could also hit a failed assertion in MemoryContextCreate(), like:

TRAP: failed Assert("CritSectionCount == 0"), File: "mcxt.c", Line: 1107, PID: 3295726
pg18/bin/postgres(ExceptionalCondition+0xbb)[0x59668bee1f6d]
pg18/bin/postgres(MemoryContextCreate+0x46)[0x59668bf2a8fe]
pg18/bin/postgres(AllocSetContextCreateInternal+0x1df)[0x59668bf1bb11]
pg18/bin/postgres(pgstat_prep_pending_entry+0x86)[0x59668bcff8cc]
pg18/bin/postgres(pgstat_prep_backend_pending+0x2b)[0x59668bd024a9]

I propose that as of now we discuss the issue in [1] instead of here.

[1]: https://www.postgresql.org/message-id/Z4ks4%2BwnAz8eEyX9%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

---

From: Andres Freund <[email-masked]>
Date: Thu, 16 Jan 2025 11:28:43 -0500

Hi,

> all variable-numbered stats kinds that require this behavior.

The issue here imo is that recently IO stats were turned into a variable
numbered stat, where it previously wasn't.  It seems like a bad idea that now
we regularly do a somewhat sizable memory allocation for backend stats. For
table stats allocating the pending memory is somewhat a necessity - there
could be a lot of different tables after all. And later transactions won't
necessarily access the same set of tables, so we can't just keep the memory
for pending around - but that's not true for IO.

> One other case where I've seen this as being an issue is injection point
> stats, for example, while enabling these stats for all callbacks and some of
> them are run in critical sections.

That seems somewhat fundamental - you could have arbitrary numbers of
injection points and thus need arbitrary amounts of memory. You can't just do
that in a critical section. And it's not just about a potential memory
allocation for pending stats, the DSM allocation can't be done in a critical
section either.

> PgStat_KindInfo.

That seems like an *incredibly* bad idea. That restriction exists for a
reason.

> not be talking about a lot of memory.

My view is that for IO stats no memory allocation should be required - that
used to be the case and should be the case again. And for something like
injection points (not really convinced that it's a good thing to have stats
for, but ...), allocation the shared memory stats object outside of the
critical section and not having any pending stats seems to be the right
approach.

Greetings,

Andres Freund

---

From: Michael Paquier <[email-masked]>
Date: Fri, 17 Jan 2025 09:08:02 +0900

> for pending around - but that's not true for IO.

Yep, for the reason that backend stats require that with one entry in
the pgstats dshash for each backend as these are sized depending on
max_connections & co using a proc number for the hash key.

> critical section and not having any pending stats seems to be the right

Not sure to completely agree on that as this pushes the responsibility
of how to deal with the critical section handling to each stats kind,
but I won't fight over it for this case, either.  It would not be the
first time we have specific logic to do allocations beforehand when
dealing with critical sections (38c579b08988 is the most recent
example coming in mind, there are many others). 

Anyway, let's just switch pgstat_backend.c so as we use a static
PgStat_BackendPending (pending name) to store the pending IO stats
that could be reused for also the WAL bits.  It does not seem that
complicated as far as I can see, removing pgstat_prep_backend_pending
and changing pgstat_flush_backend_entry_io to use the static pending
data.  The patch set posted here is doing that within its
pgstat_flush_backend_entry_wal():
https://www.postgresql.org/message-id/[email-masked]

I could tweak that around the beginning of next week with a proposal
of patch.  Bertrand, perhaps you'd prefer hack on this one?
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 17 Jan 2025 06:06:35 +0000

Hi,

> of patch.  Bertrand, perhaps you'd prefer hack on this one?

Yeah, I had in mind to work on it (in the exact same line as you are describing
above). I'll work on it.

Regards,

---

From: Michael Paquier <[email-masked]>
Date: Fri, 17 Jan 2025 15:12:36 +0900

> Yeah, I had in mind to work on it (in the exact same line as you are describing

Thanks.
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Fri, 17 Jan 2025 10:23:48 +0000

Hi,

> > Yeah, I had in mind to work on it (in the exact same line as you are describing

Please find attached a patch implementing the ideas above, meaning:

- It creates a new PendingBackendStats variable
- It uses this variable to increment and flush per backend pending IO statistics

That way we get rid of the memory allocation for pending IO statistics.

One remark: a special case has been added in pgstat_flush_pending_entries(). The
reason is that while the per backend stats are "variable-numbered" stats, it could
be that their pending stats are not part of pgStatPending. This is currently the
case (with this patch) as the per backend pending IO stats are not tracked with
pgstat_prep_backend_pending() and friends anymore.

Thoughts?

Regards,

>From 48d715a4a8b667628bb8ccf573e0003eb8f54633 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Fri, 17 Jan 2025 06:44:47 +0000
Subject: [PATCH v1] Rework per backend pending stats

9aea73fc61d added per backend pending statistics but not all of them are well
suited to memory allocation and have to be outside of critical sections.

For those (the only current one is I/O statistics but WAL statistics is under
discussion), let's rely on a new PendingBackendStats instead.
---
 src/backend/utils/activity/pgstat.c         | 27 ++++++++++++++++++-
 src/backend/utils/activity/pgstat_backend.c | 29 +++++++++++++++------
 src/backend/utils/activity/pgstat_io.c      | 14 +++-------
 src/include/pgstat.h                        | 12 +++++++++
 4 files changed, 62 insertions(+), 20 deletions(-)
  89.0% src/backend/utils/activity/
  10.9% src/include/

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 34520535d54..27aa0491001 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -1369,6 +1369,7 @@ static bool
 pgstat_flush_pending_entries(bool nowait)
 {
 	bool		have_pending = false;
+	bool		backend_have_pending = false;
 	dlist_node *cur = NULL;
 
 	/*
@@ -1416,9 +1417,33 @@ pgstat_flush_pending_entries(bool nowait)
 		cur = next;
 	}
 
+	/*
+	 * There is a special case for some pending stats that are tracked in
+	 * PendingBackendStats. It's possible that those have not been flushed
+	 * above, hence the extra check here.
+	 */
+	if (!pg_memory_is_all_zeros(&PendingBackendStats,
+								sizeof(struct PgStat_BackendPending)))
+	{
+		PgStat_EntryRef *entry_ref;
+
+		entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid,
+										 MyProcNumber, false, NULL);
+
+		if (entry_ref)
+		{
+			PgStat_HashKey key = entry_ref->shared_entry->key;
+			PgStat_Kind kind = key.kind;
+			const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
+
+			if (!kind_info->flush_pending_cb(entry_ref, nowait))
+				backend_have_pending = true;
+		}
+	}
+
 	Assert(dlist_is_empty(&pgStatPending) == !have_pending);
 
-	return have_pending;
+	return (have_pending || backend_have_pending);
 }

diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
index 79e4d0a3053..ca5cec348b2 100644
--- a/src/backend/utils/activity/pgstat_backend.c
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -22,8 +22,11 @@
 
 #include "postgres.h"
 
+#include "utils/memutils.h"
 #include "utils/pgstat_internal.h"
 
+PgStat_BackendPending PendingBackendStats = {0};
+
 /*
  * Returns statistics of a backend by proc number.
  */
@@ -46,14 +49,20 @@ static void
 pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref)
 {
 	PgStatShared_Backend *shbackendent;
-	PgStat_BackendPending *pendingent;
 	PgStat_BktypeIO *bktype_shstats;
-	PgStat_PendingIO *pending_io;
+	PgStat_PendingIO pending_io;
+
+	/*
+	 * This function can be called even if nothing at all has happened. In
+	 * this case, avoid unnecessarily modifying the stats entry.
+	 */
+	if (pg_memory_is_all_zeros(&PendingBackendStats.pending_io,
+							   sizeof(struct PgStat_PendingIO)))
+		return;
 
 	shbackendent = (PgStatShared_Backend *) entry_ref->shared_stats;
-	pendingent = (PgStat_BackendPending *) entry_ref->pending;
 	bktype_shstats = &shbackendent->stats.io_stats;
-	pending_io = &pendingent->pending_io;
+	pending_io = PendingBackendStats.pending_io;
 
 	for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
 	{
@@ -64,17 +73,21 @@ pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref)
 				instr_time	time;
 
 				bktype_shstats->counts[io_object][io_context][io_op] +=
-					pending_io->counts[io_object][io_context][io_op];
+					pending_io.counts[io_object][io_context][io_op];
 				bktype_shstats->bytes[io_object][io_context][io_op] +=
-					pending_io->bytes[io_object][io_context][io_op];
-
-				time = pending_io->pending_times[io_object][io_context][io_op];
+					pending_io.bytes[io_object][io_context][io_op];
+				time = pending_io.pending_times[io_object][io_context][io_op];
 
 				bktype_shstats->times[io_object][io_context][io_op] +=
 					INSTR_TIME_GET_MICROSEC(time);
 			}
 		}
 	}
+
+	/*
+	 * Clear out the statistics buffer, so it can be re-used.
+	 */
+	MemSet(&PendingBackendStats.pending_io, 0, sizeof(PgStat_PendingIO));
 }
 
 /*
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index 027aad8b24e..b8f19515ae1 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -75,11 +75,8 @@ pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op,
 
 	if (pgstat_tracks_backend_bktype(MyBackendType))
 	{
-		PgStat_BackendPending *entry_ref;
-
-		entry_ref = pgstat_prep_backend_pending(MyProcNumber);
-		entry_ref->pending_io.counts[io_object][io_context][io_op] += cnt;
-		entry_ref->pending_io.bytes[io_object][io_context][io_op] += bytes;
+		PendingBackendStats.pending_io.counts[io_object][io_context][io_op] += cnt;
+		PendingBackendStats.pending_io.bytes[io_object][io_context][io_op] += bytes;
 	}
 
 	PendingIOStats.counts[io_object][io_context][io_op] += cnt;
@@ -146,13 +143,8 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
 					   io_time);
 
 		if (pgstat_tracks_backend_bktype(MyBackendType))
-		{
-			PgStat_BackendPending *entry_ref;
-
-			entry_ref = pgstat_prep_backend_pending(MyProcNumber);
-			INSTR_TIME_ADD(entry_ref->pending_io.pending_times[io_object][io_context][io_op],
+			INSTR_TIME_ADD(PendingBackendStats.pending_io.pending_times[io_object][io_context][io_op],
 						   io_time);
-		}
 	}
 
 	pgstat_count_io_op(io_object, io_context, io_op, cnt, bytes);
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 2d40fe3e70f..3eb7b6614fe 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -826,5 +826,17 @@ extern PGDLLIMPORT SessionEndType pgStatSessionEndCause;
 /* updated directly by backends and background processes */
 extern PGDLLIMPORT PgStat_PendingWalStats PendingWalStats;
 
+/*
+ * Variables in pgstat_backend.c
+ */
+
+/* updated directly by backends */
+
+/*
+ * Some pending statistics are not well suited to memory allocation and have
+ * to be outside of critical sections. For those, let's rely on this variable
+ * instead.
+ */
+extern PGDLLIMPORT PgStat_BackendPending PendingBackendStats;
 
 #endif							/* PGSTAT_H */
-- 
2.34.1

---

From: Michael Paquier <[email-masked]>
Date: Sat, 18 Jan 2025 17:53:31 +0900

+    /*

Hmm.  Such special complexities in pgstat.c are annoying.  There is
a stupid thing I am wondering here.  For the WAL stats, why couldn't
we place some calls of pgstat_prep_backend_pending() in strategic
places like XLogBeginInsert() to force all the allocation steps of the
pending entry to happen before we would enter the critical sections
when doing a WAL insertion?  As far as I can see, there is a special
case with 2PC where XLogBeginInsert() could be called in a critical
section, but that seems to be the only one at quick glance.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Mon, 20 Jan 2025 15:34:41 +0900

> section, but that seems to be the only one at quick glance.

I've looked first at this idea over the week-end with a quick hack,
and it did not finish well.

And then it struck me that with a bit of redesign of the callbacks, so
as the existing flush_fixed_cb and have_fixed_pending_cb are usable
with variable-numbered stats, we should be able to avoid the exception
your first version of the patch has introduced.  Attached is a patch
to achieve what I have in mind, which is more generic than your
previous patch.  I've built my stuff as 0002 on top of your 0001.

One key choice I have made is to hide PendingBackendStats within
pgstat_backend.c so as it is possible to enforce some sanity checks
there.
--
Michael
From 90ffa6d32a9cd0034a2fd97c31991199f2089698 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Fri, 17 Jan 2025 06:44:47 +0000
Subject: [PATCH v2 1/2] Rework per backend pending stats

index 2d40fe3e70..3eb7b6614f 100644
 
+/*
 
 #endif							/* PGSTAT_H */
index 34520535d5..27aa049100 100644
 
 	/*
 
+	/*
 
-	return have_pending;

diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
index 79e4d0a305..ca5cec348b 100644
--- a/src/backend/utils/activity/pgstat_backend.c
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -22,8 +22,11 @@
 
 #include "postgres.h"

 

 
 
 /*
index 027aad8b24..b8f19515ae 100644

 
 
2.47.1

From 99561e65c3e266157ac21588097c8222a1df1d28 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Mon, 20 Jan 2025 15:31:57 +0900
Subject: [PATCH v2 2/2] More work to allow backend statistics in critical
 sections

Callbacks are refactored so as variable-numbered stats can use them if
they don't want to use pending data, like the backend stats.
---
 src/include/pgstat.h                         |  21 ++-
 src/include/utils/pgstat_internal.h          |  29 ++--
 src/backend/utils/activity/pgstat.c          |  67 +++------
 src/backend/utils/activity/pgstat_backend.c  | 142 ++++++++++++-------
 src/backend/utils/activity/pgstat_io.c       |  15 +-
 src/backend/utils/activity/pgstat_relation.c |   4 +-
 6 files changed, 140 insertions(+), 138 deletions(-)

diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 3eb7b6614f..d0d4515097 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -540,6 +540,15 @@ extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void);
  * Functions in pgstat_backend.c
  */
 
+/* used by pgstat_io.c for I/O stats tracked in backends */
+extern void pgstat_count_backend_io_op_time(IOObject io_object,
+											IOContext io_context,
+											IOOp io_op,
+											instr_time io_time);
+extern void pgstat_count_backend_io_op(IOObject io_object,
+									   IOContext io_context,
+									   IOOp io_op, uint32 cnt,
+									   uint64 bytes);
 extern PgStat_Backend *pgstat_fetch_stat_backend(ProcNumber procNumber);
 extern bool pgstat_tracks_backend_bktype(BackendType bktype);
 extern void pgstat_create_backend(ProcNumber procnum);
@@ -826,17 +835,5 @@ extern PGDLLIMPORT SessionEndType pgStatSessionEndCause;
 /* updated directly by backends and background processes */
 extern PGDLLIMPORT PgStat_PendingWalStats PendingWalStats;
 
-/*
- * Variables in pgstat_backend.c
- */
-
-/* updated directly by backends */
-
-/*
- * Some pending statistics are not well suited to memory allocation and have
- * to be outside of critical sections. For those, let's rely on this variable
- * instead.
- */
-extern PGDLLIMPORT PgStat_BackendPending PendingBackendStats;
 
 #endif							/* PGSTAT_H */
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 4bb8e5c53a..000ed5b36f 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -259,8 +259,8 @@ typedef struct PgStat_KindInfo
 	void		(*init_backend_cb) (void);
 
 	/*
-	 * For variable-numbered stats: flush pending stats. Required if pending
-	 * data is used.  See flush_fixed_cb for fixed-numbered stats.
+	 * For variable-numbered stats: flush pending stats within the dshash.
+	 * Required if pending data interacts with the pgstats dshash.
 	 */
 	bool		(*flush_pending_cb) (PgStat_EntryRef *sr, bool nowait);
 
@@ -289,17 +289,19 @@ typedef struct PgStat_KindInfo
 	void		(*init_shmem_cb) (void *stats);
 
 	/*
-	 * For fixed-numbered statistics: Flush pending stats. Returns true if
-	 * some of the stats could not be flushed, due to lock contention for
-	 * example. Optional.
+	 * For fixed-numbered or variable-numbered statistics: Check for pending
+	 * stats in need of flush, when these do not use the pgstats dshash.
+	 * Returns true if there are any stats pending for flush, triggering
+	 * flush_cb. Optional.
 	 */
-	bool		(*flush_fixed_cb) (bool nowait);
+	bool		(*have_pending_cb) (void);
 
 	/*
-	 * For fixed-numbered statistics: Check for pending stats in need of
-	 * flush. Returns true if there are any stats pending for flush. Optional.
+	 * For fixed-numbered or variable-numbered statistics: Flush pending
+	 * stats. Returns true if some of the stats could not be flushed, due
+	 * to lock contention for example. Optional.
 	 */
-	bool		(*have_fixed_pending_cb) (void);
+	bool		(*flush_cb) (bool nowait);
 
 	/*
 	 * For fixed-numbered statistics: Reset All.
@@ -617,10 +619,11 @@ extern void pgstat_archiver_snapshot_cb(void);
 #define PGSTAT_BACKEND_FLUSH_IO		(1 << 0)	/* Flush I/O statistics */
 #define PGSTAT_BACKEND_FLUSH_ALL	(PGSTAT_BACKEND_FLUSH_IO)
 
-extern void pgstat_flush_backend(bool nowait, bits32 flags);
-extern PgStat_BackendPending *pgstat_prep_backend_pending(ProcNumber procnum);
-extern bool pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
-extern void pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts);
+extern bool pgstat_flush_backend(bool nowait, bits32 flags);
+extern bool pgstat_backend_flush_cb(bool nowait);
+extern bool pgstat_backend_have_pending_cb(void);
+extern void pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header,
+											  TimestampTz ts);
 
 /*
  * Functions in pgstat_bgwriter.c
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 27aa049100..bd1ffad6f4 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -372,7 +372,8 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Backend *) 0)->stats),
 		.pending_size = sizeof(PgStat_BackendPending),
 
-		.flush_pending_cb = pgstat_backend_flush_cb,
+		.have_pending_cb = pgstat_backend_have_pending_cb,
+		.flush_cb = pgstat_backend_flush_cb,
 		.reset_timestamp_cb = pgstat_backend_reset_timestamp_cb,
 	},
 
@@ -437,8 +438,8 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_off = offsetof(PgStatShared_IO, stats),
 		.shared_data_len = sizeof(((PgStatShared_IO *) 0)->stats),
 
-		.flush_fixed_cb = pgstat_io_flush_cb,
-		.have_fixed_pending_cb = pgstat_io_have_pending_cb,
+		.flush_cb = pgstat_io_flush_cb,
+		.have_pending_cb = pgstat_io_have_pending_cb,
 		.init_shmem_cb = pgstat_io_init_shmem_cb,
 		.reset_all_cb = pgstat_io_reset_all_cb,
 		.snapshot_cb = pgstat_io_snapshot_cb,
@@ -455,8 +456,8 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_off = offsetof(PgStatShared_SLRU, stats),
 		.shared_data_len = sizeof(((PgStatShared_SLRU *) 0)->stats),
 
-		.flush_fixed_cb = pgstat_slru_flush_cb,
-		.have_fixed_pending_cb = pgstat_slru_have_pending_cb,
+		.flush_cb = pgstat_slru_flush_cb,
+		.have_pending_cb = pgstat_slru_have_pending_cb,
 		.init_shmem_cb = pgstat_slru_init_shmem_cb,
 		.reset_all_cb = pgstat_slru_reset_all_cb,
 		.snapshot_cb = pgstat_slru_snapshot_cb,
@@ -474,8 +475,8 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Wal *) 0)->stats),
 
 		.init_backend_cb = pgstat_wal_init_backend_cb,
-		.flush_fixed_cb = pgstat_wal_flush_cb,
-		.have_fixed_pending_cb = pgstat_wal_have_pending_cb,
+		.flush_cb = pgstat_wal_flush_cb,
+		.have_pending_cb = pgstat_wal_have_pending_cb,
 		.init_shmem_cb = pgstat_wal_init_shmem_cb,
 		.reset_all_cb = pgstat_wal_reset_all_cb,
 		.snapshot_cb = pgstat_wal_snapshot_cb,
@@ -713,22 +714,17 @@ pgstat_report_stat(bool force)
 	{
 		bool		do_flush = false;
 
-		/* Check for pending fixed-numbered stats */
+		/* Check for pending stats */
 		for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
 		{
 			const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
 
 			if (!kind_info)
 				continue;
-			if (!kind_info->fixed_amount)
-			{
-				Assert(kind_info->have_fixed_pending_cb == NULL);
-				continue;
-			}
-			if (!kind_info->have_fixed_pending_cb)
+			if (!kind_info->have_pending_cb)
 				continue;
 
-			if (kind_info->have_fixed_pending_cb())
+			if (kind_info->have_pending_cb())
 			{
 				do_flush = true;
 				break;
@@ -792,22 +788,20 @@ pgstat_report_stat(bool force)
 	/* flush of variable-numbered stats */
 	partial_flush |= pgstat_flush_pending_entries(nowait);
 
-	/* flush of fixed-numbered stats */
+	/*
+	 * Flush of other stats, which could be variable-numbered or
+	 * fixed-numbered.
+	 */
 	for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
 	{
 		const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
 
 		if (!kind_info)
 			continue;
-		if (!kind_info->fixed_amount)
-		{
-			Assert(kind_info->flush_fixed_cb == NULL);
-			continue;
-		}
-		if (!kind_info->flush_fixed_cb)
+		if (!kind_info->flush_cb)
 			continue;
 
-		partial_flush |= kind_info->flush_fixed_cb(nowait);
+		partial_flush |= kind_info->flush_cb(nowait);
 	}
 
 	last_flush = now;
@@ -1369,7 +1363,6 @@ static bool
 pgstat_flush_pending_entries(bool nowait)
 {
 	bool		have_pending = false;
-	bool		backend_have_pending = false;
 	dlist_node *cur = NULL;
 
 	/*
@@ -1417,33 +1410,9 @@ pgstat_flush_pending_entries(bool nowait)
 		cur = next;
 	}
 
-	/*
-	 * There is a special case for some pending stats that are tracked in
-	 * PendingBackendStats. It's possible that those have not been flushed
-	 * above, hence the extra check here.
-	 */
-	if (!pg_memory_is_all_zeros(&PendingBackendStats,
-								sizeof(struct PgStat_BackendPending)))
-	{
-		PgStat_EntryRef *entry_ref;
-
-		entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid,
-										 MyProcNumber, false, NULL);
-
-		if (entry_ref)
-		{
-			PgStat_HashKey key = entry_ref->shared_entry->key;
-			PgStat_Kind kind = key.kind;
-			const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
-
-			if (!kind_info->flush_pending_cb(entry_ref, nowait))
-				backend_have_pending = true;
-		}
-	}
-
 	Assert(dlist_is_empty(&pgStatPending) == !have_pending);
 
-	return (have_pending || backend_have_pending);
+	return have_pending;
 }

diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
index ca5cec348b..fb7abf64a0 100644
--- a/src/backend/utils/activity/pgstat_backend.c
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -11,7 +11,9 @@
  * 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.
+ * on disk.  Pending statistics are managed without direct interactions with
+ * the pgstats dshash, relying on PendingBackendStats instead so as it is
+ * possible to report data within critical sections.
  *
  * Copyright (c) 2001-2025, PostgreSQL Global Development Group
  *
@@ -22,10 +24,53 @@
 
 #include "postgres.h"
 
+#include "storage/bufmgr.h"
 #include "utils/memutils.h"
 #include "utils/pgstat_internal.h"
 
-PgStat_BackendPending PendingBackendStats = {0};
+/*
+ * Backend statistics counts waiting to be flushed out.  We assume this variable
+ * inits to zeroes.  These counters may be reported within critical sections so
+ * we use static memory in order to avoid memory allocation.
+ */
+static PgStat_BackendPending PendingBackendStats = {0};
+static bool have_backendstats = false;
+
+/*
+ * Utility routines to report I/O stats for backends, kept here to avoid
+ * exposing PendingBackendStats to the outside world.
+ */
+void
+pgstat_count_backend_io_op_time(IOObject io_object, IOContext io_context,
+								IOOp io_op, instr_time io_time)
+{
+	Assert(track_io_timing);
+
+	if (!pgstat_tracks_backend_bktype(MyBackendType))
+		return;
+
+	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
+
+	INSTR_TIME_ADD(PendingBackendStats.pending_io.pending_times[io_object][io_context][io_op],
+				   io_time);
+
+	have_backendstats = true;
+}
+
+void
+pgstat_count_backend_io_op(IOObject io_object, IOContext io_context,
+						   IOOp io_op, uint32 cnt, uint64 bytes)
+{
+	if (!pgstat_tracks_backend_bktype(MyBackendType))
+		return;
+
+	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
+
+	PendingBackendStats.pending_io.counts[io_object][io_context][io_op] += cnt;
+	PendingBackendStats.pending_io.bytes[io_object][io_context][io_op] += bytes;
+
+	have_backendstats = true;
+}
 
 /*
  * Returns statistics of a backend by proc number.
@@ -53,8 +98,9 @@ pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref)
 	PgStat_PendingIO pending_io;
 
 	/*
-	 * This function can be called even if nothing at all has happened. In
-	 * this case, avoid unnecessarily modifying the stats entry.
+	 * This function can be called even if nothing at all has happened for
+	 * IO statistics.  In this case, avoid unnecessarily modifying the stats
+	 * entry.
 	 */
 	if (pg_memory_is_all_zeros(&PendingBackendStats.pending_io,
 							   sizeof(struct PgStat_PendingIO)))
@@ -83,25 +129,29 @@ pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref)
 			}
 		}
 	}
-
-	/*
-	 * Clear out the statistics buffer, so it can be re-used.
-	 */
-	MemSet(&PendingBackendStats.pending_io, 0, sizeof(PgStat_PendingIO));
 }
 
 /*
- * Wrapper routine to flush backend statistics.
+ * Flush out locally pending backend statistics
+ *
+ * "flags" parameter controls which statistics to flush.  Returns true
+ * if some statistics could not be flushed.
  */
-static bool
-pgstat_flush_backend_entry(PgStat_EntryRef *entry_ref, bool nowait,
-						   bits32 flags)
+bool
+pgstat_flush_backend(bool nowait, bits32 flags)
 {
+	PgStat_EntryRef *entry_ref;
+
+	if (!have_backendstats)
+		return false;
+
 	if (!pgstat_tracks_backend_bktype(MyBackendType))
 		return false;
 
-	if (!pgstat_lock_entry(entry_ref, nowait))
-		return false;
+	entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_BACKEND, InvalidOid,
+											MyProcNumber, nowait);
+	if (!entry_ref)
+		return true;
 
 	/* Flush requested statistics */
 	if (flags & PGSTAT_BACKEND_FLUSH_IO)
@@ -109,36 +159,33 @@ pgstat_flush_backend_entry(PgStat_EntryRef *entry_ref, bool nowait,
 
 	pgstat_unlock_entry(entry_ref);
 
-	return true;
+	/*
+	 * Clear out the statistics buffer, so it can be re-used.
+	 */
+	MemSet(&PendingBackendStats, 0, sizeof(PgStat_BackendPending));
+
+	have_backendstats = false;
+	return false;
+}
+
+/*
+ * Check if there are any backend stats waiting for flush.
+ */
+bool
+pgstat_backend_have_pending_cb(void)
+{
+	return have_backendstats;
 }
 
 /*
  * Callback to flush out locally pending backend statistics.
  *
- * If no stats have been recorded, this function returns false.
+ * If some stats could not be flushed, return true.
  */
 bool
-pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
+pgstat_backend_flush_cb(bool nowait)
 {
-	return pgstat_flush_backend_entry(entry_ref, nowait, PGSTAT_BACKEND_FLUSH_ALL);
-}
-
-/*
- * Flush out locally pending backend statistics
- *
- * "flags" parameter controls which statistics to flush.
- */
-void
-pgstat_flush_backend(bool nowait, bits32 flags)
-{
-	PgStat_EntryRef *entry_ref;
-
-	if (!pgstat_tracks_backend_bktype(MyBackendType))
-		return;
-
-	entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid,
-									 MyProcNumber, false, NULL);
-	(void) pgstat_flush_backend_entry(entry_ref, nowait, flags);
+	return pgstat_flush_backend(nowait, PGSTAT_BACKEND_FLUSH_ALL);
 }
 
 /*
@@ -150,9 +197,8 @@ pgstat_create_backend(ProcNumber procnum)
 	PgStat_EntryRef *entry_ref;
 	PgStatShared_Backend *shstatent;
 
-	entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_BACKEND, InvalidOid,
-										  procnum, NULL);
-
+	entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_BACKEND, InvalidOid,
+											MyProcNumber, false);
 	shstatent = (PgStatShared_Backend *) entry_ref->shared_stats;
 
 	/*
@@ -160,20 +206,10 @@ pgstat_create_backend(ProcNumber procnum)
 	 * e.g. if we previously used this proc number.
 	 */
 	memset(&shstatent->stats, 0, sizeof(shstatent->stats));
-}
+	pgstat_unlock_entry(entry_ref);
 
-/*
- * Find or create a local PgStat_BackendPending entry for proc number.
- */
-PgStat_BackendPending *
-pgstat_prep_backend_pending(ProcNumber procnum)
-{
-	PgStat_EntryRef *entry_ref;
-
-	entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_BACKEND, InvalidOid,
-										  procnum, NULL);
-
-	return entry_ref->pending;
+	MemSet(&PendingBackendStats, 0, sizeof(PgStat_BackendPending));
+	have_backendstats = false;
 }
 
 /*
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index b8f19515ae..6ff5d9e96a 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -73,15 +73,12 @@ pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op,
 	Assert(pgstat_is_ioop_tracked_in_bytes(io_op) || bytes == 0);
 	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
 
-	if (pgstat_tracks_backend_bktype(MyBackendType))
-	{
-		PendingBackendStats.pending_io.counts[io_object][io_context][io_op] += cnt;
-		PendingBackendStats.pending_io.bytes[io_object][io_context][io_op] += bytes;
-	}
-
 	PendingIOStats.counts[io_object][io_context][io_op] += cnt;
 	PendingIOStats.bytes[io_object][io_context][io_op] += bytes;
 
+	/* Add the per-backend counts */
+	pgstat_count_backend_io_op(io_object, io_context, io_op, cnt, bytes);
+
 	have_iostats = true;
 }
 
@@ -142,9 +139,9 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
 		INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op],
 					   io_time);
 
-		if (pgstat_tracks_backend_bktype(MyBackendType))
-			INSTR_TIME_ADD(PendingBackendStats.pending_io.pending_times[io_object][io_context][io_op],
-						   io_time);
+		/* Add the per-backend count */
+		pgstat_count_backend_io_op_time(io_object, io_context, io_op,
+										io_time);
 	}
 
 	pgstat_count_io_op(io_object, io_context, io_op, cnt, bytes);
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index 09247ba097..965a7fe2c6 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -264,7 +264,7 @@ pgstat_report_vacuum(Oid tableoid, bool shared,
 	 * VACUUM command has processed all tables and committed.
 	 */
 	pgstat_flush_io(false);
-	pgstat_flush_backend(false, PGSTAT_BACKEND_FLUSH_IO);
+	(void) pgstat_flush_backend(false, PGSTAT_BACKEND_FLUSH_IO);
 }
 
 /*
@@ -351,7 +351,7 @@ pgstat_report_analyze(Relation rel,
 
 	/* see pgstat_report_vacuum() */
 	pgstat_flush_io(false);
-	pgstat_flush_backend(false, PGSTAT_BACKEND_FLUSH_IO);
+	(void) pgstat_flush_backend(false, PGSTAT_BACKEND_FLUSH_IO);
 }
 
 /*
-- 
2.47.1

---

From: Bertrand Drouvot <[email-masked]>
Date: Mon, 20 Jan 2025 11:10:40 +0000

Hi,

> and it did not finish well.

Yeah, that would probably also mean moving the current WAL pending increments to
the same "new" place (to be consistent) which does not seem great.

Thanks! 

> previous patch.  I've built my stuff as 0002 on top of your 0001.

Indeed. Another idea that I had would have been to create another callback,
but I prefer your idea.

=== 1

I think that flush_pending_cb, flush_cb and have_pending_cb are now somehow
confusing (one could think that have_pending_cb is only linked to
flush_pending_cb).

I think that it would be better to make the distinction based on "local/static"
vs "dynamic memory" pending stats instead: I did so in v3 attached, using:

.flush_dynamic_cb(): flushes pending entries tracked in dynamic memory
.flush_static_cb(): flushes pending stats from static/global variable

Also I reworded the comments a bit.

=== 2

-       /*

The inital intent was to clear *only" the pending IO stats, so that each
flush (IO, WAL once implemented,...) could clear the pending stats it is
responsible for.

=== 3

+       /*

Not sure about this one, see above. I mean it is currently Ok but once we'll
introduce the WAL part then it will not be correct depending of the flag value
being passed.

So, I did put back the previous logic in place (setting to zero only the stats
the flush callback is responsible for) in v3 attached.

=== 4

I now think we should memset to zero to avoid any padding issues (as more structs
will be added to PgStat_BackendPending). Oh and it's already done in
pgstat_create_backend(), so removing the {0} assignement in the attached.

=== 5

I don't think setting have_backendstats to false in pgstat_flush_backend()
is correct. I mean, it is correct currently but once we'll add the WAL part it
won't necessary be correct if the flags != PGSTAT_BACKEND_FLUSH_ALL. So, using a 
pg_memory_is_all_zeros check on PendingBackendStats instead in the attached.

> pgstat_backend.c so as it is possible to enforce some sanity checks

Yeah, better, thanks!

=== 6

In passing, I realized that:

"

in pgstat_backend.c is incorrect (fixing it in 0002).

PFA:

0001: which is full rebase
0002: the Copyright fix
.txt: the changes I've made on top of your 0002 (to not confuse the cfbot and
to ease your review).

Regards,

>From c945a5800554ec90f42091d8175ed50e00ead45f Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Fri, 17 Jan 2025 06:44:47 +0000
Subject: [PATCH v3 1/2] Rework per backend pending stats

For those (the only current one is I/O statistics but WAL statistics is under
discussion), let's rely on a new PendingBackendStats instead.
---
 src/backend/utils/activity/pgstat.c           |  53 +++----
 src/backend/utils/activity/pgstat_backend.c   | 142 ++++++++++++------
 src/backend/utils/activity/pgstat_io.c        |  23 +--
 src/backend/utils/activity/pgstat_relation.c  |   4 +-
 src/include/pgstat.h                          |   9 ++
 src/include/utils/pgstat_internal.h           |  36 +++--
 .../injection_points/injection_stats.c        |   2 +-
 7 files changed, 153 insertions(+), 116 deletions(-)
  75.8% src/backend/utils/activity/
  19.6% src/include/utils/
   3.6% src/include/

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 34520535d54..0a4b687f045 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -292,7 +292,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Database *) 0)->stats),
 		.pending_size = sizeof(PgStat_StatDBEntry),
 
-		.flush_pending_cb = pgstat_database_flush_cb,
+		.flush_dynamic_cb = pgstat_database_flush_cb,
 		.reset_timestamp_cb = pgstat_database_reset_timestamp_cb,
 	},
 
@@ -307,7 +307,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Relation *) 0)->stats),
 		.pending_size = sizeof(PgStat_TableStatus),
 
-		.flush_pending_cb = pgstat_relation_flush_cb,
+		.flush_dynamic_cb = pgstat_relation_flush_cb,
 		.delete_pending_cb = pgstat_relation_delete_pending_cb,
 	},
 
@@ -322,7 +322,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Function *) 0)->stats),
 		.pending_size = sizeof(PgStat_FunctionCounts),
 
-		.flush_pending_cb = pgstat_function_flush_cb,
+		.flush_dynamic_cb = pgstat_function_flush_cb,
 	},
 
 	[PGSTAT_KIND_REPLSLOT] = {
@@ -355,7 +355,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Subscription *) 0)->stats),
 		.pending_size = sizeof(PgStat_BackendSubEntry),
 
-		.flush_pending_cb = pgstat_subscription_flush_cb,
+		.flush_dynamic_cb = pgstat_subscription_flush_cb,
 		.reset_timestamp_cb = pgstat_subscription_reset_timestamp_cb,
 	},

-		.flush_pending_cb = pgstat_backend_flush_cb,
+		.have_pending_cb = pgstat_backend_have_pending_cb,
+		.flush_static_cb = pgstat_backend_flush_cb,
 		.reset_timestamp_cb = pgstat_backend_reset_timestamp_cb,
 	},

+		.flush_static_cb = pgstat_io_flush_cb,
 
+		.flush_static_cb = pgstat_slru_flush_cb,
 
+		.flush_static_cb = pgstat_wal_flush_cb,

 			if (!kind_info)
 				continue;
 
-			if (kind_info->have_fixed_pending_cb())
+			if (kind_info->have_pending_cb())
 			{
 				do_flush = true;
 				break;
@@ -789,25 +785,20 @@ pgstat_report_stat(bool force)
 
 	partial_flush = false;
 
-	/* flush of variable-numbered stats */
+	/* flush of variable-numbered stats tracked in pending entries list */
 	partial_flush |= pgstat_flush_pending_entries(nowait);
 
-	/* flush of fixed-numbered stats */
+	/* flush stats for each registered kind that has a flush static callback */
 	for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
 	{
 		const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
 
 		if (!kind_info)
 			continue;
-		if (!kind_info->fixed_amount)
-		{
-			Assert(kind_info->flush_fixed_cb == NULL);
-			continue;
-		}
-		if (!kind_info->flush_fixed_cb)
+		if (!kind_info->flush_static_cb)
 			continue;
 
-		partial_flush |= kind_info->flush_fixed_cb(nowait);
+		partial_flush |= kind_info->flush_static_cb(nowait);
 	}
 
 	last_flush = now;
@@ -1297,7 +1288,7 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *creat
 	PgStat_EntryRef *entry_ref;
 
 	/* need to be able to flush out */
-	Assert(pgstat_get_kind_info(kind)->flush_pending_cb != NULL);
+	Assert(pgstat_get_kind_info(kind)->flush_dynamic_cb != NULL);
 
 	if (unlikely(!pgStatPendingContext))
 	{
@@ -1394,10 +1385,10 @@ pgstat_flush_pending_entries(bool nowait)
 		dlist_node *next;
 
 		Assert(!kind_info->fixed_amount);
-		Assert(kind_info->flush_pending_cb != NULL);
+		Assert(kind_info->flush_dynamic_cb != NULL);
 
 		/* flush the stats, if possible */
-		did_flush = kind_info->flush_pending_cb(entry_ref, nowait);
+		did_flush = kind_info->flush_dynamic_cb(entry_ref, nowait);
 
 		Assert(did_flush || nowait);
 
index 79e4d0a3053..bcae6a78169 100644
@@ -22,8 +24,49 @@
 
 #include "postgres.h"

+/*
+ * Backend statistics counts waiting to be flushed out. These counters may be
+ * reported within critical sections so we use static memory in order to avoid
+ * memory allocation.
+ */
+static PgStat_BackendPending PendingBackendStats;
+
+/*
+ * Utility routines to report I/O stats for backends, kept here to avoid
+ * exposing PendingBackendStats to the outside world.
+ */
+void
+pgstat_count_backend_io_op_time(IOObject io_object, IOContext io_context,
+								IOOp io_op, instr_time io_time)
+{
+	Assert(track_io_timing);
+
+	if (!pgstat_tracks_backend_bktype(MyBackendType))
+		return;
+
+	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
+
+	INSTR_TIME_ADD(PendingBackendStats.pending_io.pending_times[io_object][io_context][io_op],
+				   io_time);
+}
+
+void
+pgstat_count_backend_io_op(IOObject io_object, IOContext io_context,
+						   IOOp io_op, uint32 cnt, uint64 bytes)
+{
+	if (!pgstat_tracks_backend_bktype(MyBackendType))
+		return;
+
+	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
+
+	PendingBackendStats.pending_io.counts[io_object][io_context][io_op] += cnt;
+	PendingBackendStats.pending_io.bytes[io_object][io_context][io_op] += bytes;
+}
+
 /*
  * Returns statistics of a backend by proc number.
  */
@@ -46,14 +89,21 @@ static void
 pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref)
 {
 	PgStatShared_Backend *shbackendent;
-	PgStat_BackendPending *pendingent;
 	PgStat_BktypeIO *bktype_shstats;
-	PgStat_PendingIO *pending_io;
+	PgStat_PendingIO pending_io;
+
+	/*
+	 * This function can be called even if nothing at all has happened for IO
+	 * statistics.  In this case, avoid unnecessarily modifying the stats
+	 * entry.
+	 */
+	if (pg_memory_is_all_zeros(&PendingBackendStats.pending_io,
+							   sizeof(struct PgStat_PendingIO)))
+		return;

 	for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
 	{
@@ -64,68 +114,74 @@ pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref)
 				instr_time	time;

 
 /*
+	if (pg_memory_is_all_zeros(&PendingBackendStats,

+	entry_ref = pgstat_get_entry_ref_locked(PGSTAT_KIND_BACKEND, InvalidOid,
+											MyProcNumber, nowait);
+	if (!entry_ref)
+		return true;
+
 	/* Flush requested statistics */
 	if (flags & PGSTAT_BACKEND_FLUSH_IO)
 		pgstat_flush_backend_entry_io(entry_ref);

-	return true;
+	return false;
 }
 
 /*
- * Callback to flush out locally pending backend statistics.
- *
- * If no stats have been recorded, this function returns false.
+ * Check if there are any backend stats waiting for flush.
  */
 bool
-pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
+pgstat_backend_have_pending_cb(void)
 {
-	return pgstat_flush_backend_entry(entry_ref, nowait, PGSTAT_BACKEND_FLUSH_ALL);
+	return (!pg_memory_is_all_zeros(&PendingBackendStats,
+									sizeof(struct PgStat_BackendPending)));
 }
 
 /*
+ * Callback to flush out locally pending backend statistics.
 
 /*
@@ -137,9 +193,8 @@ pgstat_create_backend(ProcNumber procnum)
 	PgStat_EntryRef *entry_ref;
 	PgStatShared_Backend *shstatent;

 	/*
@@ -147,20 +202,9 @@ pgstat_create_backend(ProcNumber procnum)

 /*
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index 027aad8b24e..6ff5d9e96a1 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -73,18 +73,12 @@ pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op,
 	Assert(pgstat_is_ioop_tracked_in_bytes(io_op) || bytes == 0);
 	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));

 
@@ -145,14 +139,9 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
 		INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op],
 					   io_time);

index 09247ba0971..965a7fe2c64 100644
 
 /*

 /*
index 2d40fe3e70f..d0d45150977 100644
 
+/* used by pgstat_io.c for I/O stats tracked in backends */
+extern void pgstat_count_backend_io_op_time(IOObject io_object,
+											IOContext io_context,
+											IOOp io_op,
+											instr_time io_time);
+extern void pgstat_count_backend_io_op(IOObject io_object,
+									   IOContext io_context,
+									   IOOp io_op, uint32 cnt,
+									   uint64 bytes);
 extern PgStat_Backend *pgstat_fetch_stat_backend(ProcNumber procNumber);
 extern bool pgstat_tracks_backend_bktype(BackendType bktype);
 extern void pgstat_create_backend(ProcNumber procnum);
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 4bb8e5c53ab..7222b414779 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -156,8 +156,8 @@ typedef struct PgStat_EntryRef
 	 * Pending statistics data that will need to be flushed to shared memory
 	 * stats eventually. Each stats kind utilizing pending data defines what
 	 * format its pending data has and needs to provide a
-	 * PgStat_KindInfo->flush_pending_cb callback to merge pending into shared
-	 * stats.
+	 * PgStat_KindInfo->flush_dynamic_cb callback to merge pending entries
+	 * that are in dynamic memory into shared stats.
 	 */
 	void	   *pending;
 	dlist_node	pending_node;	/* membership in pgStatPending list */
@@ -259,10 +259,11 @@ typedef struct PgStat_KindInfo
 	void		(*init_backend_cb) (void);
 
 	/*
-	 * For variable-numbered stats: flush pending stats. Required if pending
-	 * data is used.  See flush_fixed_cb for fixed-numbered stats.
+	 * For variable-numbered stats: flush pending stats entries in dynamic
+	 * memory within the dshash. Required if pending data interacts with the
+	 * pgstats dshash.
 	 */
-	bool		(*flush_pending_cb) (PgStat_EntryRef *sr, bool nowait);
+	bool		(*flush_dynamic_cb) (PgStat_EntryRef *sr, bool nowait);
 
 	/*
 	 * For variable-numbered stats: delete pending stats. Optional.
@@ -289,17 +290,19 @@ typedef struct PgStat_KindInfo
 	void		(*init_shmem_cb) (void *stats);
 
 	/*
 
 	/*
-	 * For fixed-numbered statistics: Check for pending stats in need of
-	 * flush. Returns true if there are any stats pending for flush. Optional.
+	 * For fixed-numbered or variable-numbered statistics: Flush pending
+	 * static stats. Returns true if some of the stats could not be flushed,
+	 * due to lock contention for example. Optional.
 	 */
-	bool		(*have_fixed_pending_cb) (void);
+	bool		(*flush_static_cb) (bool nowait);
 
 	/*
 	 * For fixed-numbered statistics: Reset All.
@@ -617,10 +620,11 @@ extern void pgstat_archiver_snapshot_cb(void);
 #define PGSTAT_BACKEND_FLUSH_IO		(1 << 0)	/* Flush I/O statistics */
 #define PGSTAT_BACKEND_FLUSH_ALL	(PGSTAT_BACKEND_FLUSH_IO)

 /*
  * Functions in pgstat_bgwriter.c
diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c
index 5db62bca66f..4f3691c702b 100644
--- a/src/test/modules/injection_points/injection_stats.c
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -48,7 +48,7 @@ static const PgStat_KindInfo injection_stats = {
 	.shared_data_off = offsetof(PgStatShared_InjectionPoint, stats),
 	.shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats),
 	.pending_size = sizeof(PgStat_StatInjEntry),
-	.flush_pending_cb = injection_stats_flush_cb,
+	.flush_dynamic_cb = injection_stats_flush_cb,
 };
 
 /*
-- 
2.34.1

>From 33dbb87cd92e29a71ea2012476aca1d760c8c557 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Mon, 20 Jan 2025 10:39:50 +0000
Subject: [PATCH v3 2/2] Fixing pgstat_backend.c Copyright

The file has been introduced in 9aea73fc61d, in 2024. So fixing its Copyright
accordingly.
---
 src/backend/utils/activity/pgstat_backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 100.0% src/backend/utils/activity/

diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
index bcae6a78169..b0184b3293e 100644
--- a/src/backend/utils/activity/pgstat_backend.c
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -15,7 +15,7 @@
  * the pgstats dshash, relying on PendingBackendStats instead so as it is
  * possible to report data within critical sections.
  *
- * Copyright (c) 2001-2025, PostgreSQL Global Development Group
+ * Copyright (c) 2024-2025, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
  *	  src/backend/utils/activity/pgstat_backend.c
-- 
2.34.1

commit af786645e63b34e0d4eefbef7a8dd0794d358d0f
Author: Bertrand Drouvot <[email-masked]>
Date:   Mon Jan 20 09:57:26 2025 +0000

    Change flush callbacks names
    
    and the way pending backend stats are checks and set to zero.

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index bd1ffad6f46..0a4b687f045 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -292,7 +292,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Database *) 0)->stats),
 		.pending_size = sizeof(PgStat_StatDBEntry),
 
-		.flush_pending_cb = pgstat_database_flush_cb,
+		.flush_dynamic_cb = pgstat_database_flush_cb,
 		.reset_timestamp_cb = pgstat_database_reset_timestamp_cb,
 	},
 
@@ -307,7 +307,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Relation *) 0)->stats),
 		.pending_size = sizeof(PgStat_TableStatus),
 
-		.flush_pending_cb = pgstat_relation_flush_cb,
+		.flush_dynamic_cb = pgstat_relation_flush_cb,
 		.delete_pending_cb = pgstat_relation_delete_pending_cb,
 	},
 
@@ -322,7 +322,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Function *) 0)->stats),
 		.pending_size = sizeof(PgStat_FunctionCounts),
 
-		.flush_pending_cb = pgstat_function_flush_cb,
+		.flush_dynamic_cb = pgstat_function_flush_cb,
 	},
 
 	[PGSTAT_KIND_REPLSLOT] = {
@@ -355,7 +355,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Subscription *) 0)->stats),
 		.pending_size = sizeof(PgStat_BackendSubEntry),
 
-		.flush_pending_cb = pgstat_subscription_flush_cb,
+		.flush_dynamic_cb = pgstat_subscription_flush_cb,
 		.reset_timestamp_cb = pgstat_subscription_reset_timestamp_cb,
 	},
 
@@ -373,7 +373,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.pending_size = sizeof(PgStat_BackendPending),
 
 		.have_pending_cb = pgstat_backend_have_pending_cb,
-		.flush_cb = pgstat_backend_flush_cb,
+		.flush_static_cb = pgstat_backend_flush_cb,
 		.reset_timestamp_cb = pgstat_backend_reset_timestamp_cb,
 	},
 
@@ -438,7 +438,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_off = offsetof(PgStatShared_IO, stats),
 		.shared_data_len = sizeof(((PgStatShared_IO *) 0)->stats),
 
-		.flush_cb = pgstat_io_flush_cb,
+		.flush_static_cb = pgstat_io_flush_cb,
 		.have_pending_cb = pgstat_io_have_pending_cb,
 		.init_shmem_cb = pgstat_io_init_shmem_cb,
 		.reset_all_cb = pgstat_io_reset_all_cb,
@@ -456,7 +456,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_off = offsetof(PgStatShared_SLRU, stats),
 		.shared_data_len = sizeof(((PgStatShared_SLRU *) 0)->stats),
 
-		.flush_cb = pgstat_slru_flush_cb,
+		.flush_static_cb = pgstat_slru_flush_cb,
 		.have_pending_cb = pgstat_slru_have_pending_cb,
 		.init_shmem_cb = pgstat_slru_init_shmem_cb,
 		.reset_all_cb = pgstat_slru_reset_all_cb,
@@ -475,7 +475,7 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
 		.shared_data_len = sizeof(((PgStatShared_Wal *) 0)->stats),
 
 		.init_backend_cb = pgstat_wal_init_backend_cb,
-		.flush_cb = pgstat_wal_flush_cb,
+		.flush_static_cb = pgstat_wal_flush_cb,
 		.have_pending_cb = pgstat_wal_have_pending_cb,
 		.init_shmem_cb = pgstat_wal_init_shmem_cb,
 		.reset_all_cb = pgstat_wal_reset_all_cb,
@@ -785,23 +785,20 @@ pgstat_report_stat(bool force)
 
 	partial_flush = false;
 
-	/* flush of variable-numbered stats */
+	/* flush of variable-numbered stats tracked in pending entries list */
 	partial_flush |= pgstat_flush_pending_entries(nowait);
 
-	/*
-	 * Flush of other stats, which could be variable-numbered or
-	 * fixed-numbered.
-	 */
+	/* flush stats for each registered kind that has a flush static callback */
 	for (PgStat_Kind kind = PGSTAT_KIND_MIN; kind <= PGSTAT_KIND_MAX; kind++)
 	{
 		const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
 
 		if (!kind_info)
 			continue;
-		if (!kind_info->flush_cb)
+		if (!kind_info->flush_static_cb)
 			continue;
 
-		partial_flush |= kind_info->flush_cb(nowait);
+		partial_flush |= kind_info->flush_static_cb(nowait);
 	}
 
 	last_flush = now;
@@ -1291,7 +1288,7 @@ pgstat_prep_pending_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool *creat
 	PgStat_EntryRef *entry_ref;
 
 	/* need to be able to flush out */
-	Assert(pgstat_get_kind_info(kind)->flush_pending_cb != NULL);
+	Assert(pgstat_get_kind_info(kind)->flush_dynamic_cb != NULL);
 
 	if (unlikely(!pgStatPendingContext))
 	{
@@ -1388,10 +1385,10 @@ pgstat_flush_pending_entries(bool nowait)
 		dlist_node *next;
 
 		Assert(!kind_info->fixed_amount);
-		Assert(kind_info->flush_pending_cb != NULL);
+		Assert(kind_info->flush_dynamic_cb != NULL);
 
 		/* flush the stats, if possible */
-		did_flush = kind_info->flush_pending_cb(entry_ref, nowait);
+		did_flush = kind_info->flush_dynamic_cb(entry_ref, nowait);
 
 		Assert(did_flush || nowait);
 
index fb7abf64a0b..bcae6a78169 100644
@@ -29,12 +29,11 @@
 
 /*
- * Backend statistics counts waiting to be flushed out.  We assume this variable
- * inits to zeroes.  These counters may be reported within critical sections so
- * we use static memory in order to avoid memory allocation.
+ * Backend statistics counts waiting to be flushed out. These counters may be
+ * reported within critical sections so we use static memory in order to avoid
+ * memory allocation.
  */
-static PgStat_BackendPending PendingBackendStats = {0};
-static bool have_backendstats = false;
+static PgStat_BackendPending PendingBackendStats;
 
 /*
  * Utility routines to report I/O stats for backends, kept here to avoid
@@ -53,8 +52,6 @@ pgstat_count_backend_io_op_time(IOObject io_object, IOContext io_context,
 
 	INSTR_TIME_ADD(PendingBackendStats.pending_io.pending_times[io_object][io_context][io_op],
 				   io_time);
-
-	have_backendstats = true;
 }
 
 void
@@ -68,8 +65,6 @@ pgstat_count_backend_io_op(IOObject io_object, IOContext io_context,
 
 	PendingBackendStats.pending_io.counts[io_object][io_context][io_op] += cnt;
 	PendingBackendStats.pending_io.bytes[io_object][io_context][io_op] += bytes;
-
-	have_backendstats = true;
 }
 
 /*
@@ -98,8 +93,8 @@ pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref)
 	PgStat_PendingIO pending_io;
 
 	/*
-	 * This function can be called even if nothing at all has happened for
-	 * IO statistics.  In this case, avoid unnecessarily modifying the stats
+	 * This function can be called even if nothing at all has happened for IO
+	 * statistics.  In this case, avoid unnecessarily modifying the stats
 	 * entry.
 	 */
 	if (pg_memory_is_all_zeros(&PendingBackendStats.pending_io,
@@ -129,6 +124,11 @@ pgstat_flush_backend_entry_io(PgStat_EntryRef *entry_ref)
 			}
 		}
 	}
+
+	/*
+	 * Clear out the statistics buffer, so it can be re-used.
+	 */
+	MemSet(&PendingBackendStats.pending_io, 0, sizeof(PgStat_PendingIO));
 }
 
 /*
@@ -142,7 +142,8 @@ pgstat_flush_backend(bool nowait, bits32 flags)
 {
 	PgStat_EntryRef *entry_ref;
 
-	if (!have_backendstats)
+	if (pg_memory_is_all_zeros(&PendingBackendStats,
+							   sizeof(struct PgStat_BackendPending)))
 		return false;
 
 	if (!pgstat_tracks_backend_bktype(MyBackendType))
@@ -159,12 +160,6 @@ pgstat_flush_backend(bool nowait, bits32 flags)

-	/*
-	 * Clear out the statistics buffer, so it can be re-used.
-	 */
-	MemSet(&PendingBackendStats, 0, sizeof(PgStat_BackendPending));
-
-	have_backendstats = false;
 	return false;
 }
 
@@ -174,7 +169,8 @@ pgstat_flush_backend(bool nowait, bits32 flags)
 bool
 pgstat_backend_have_pending_cb(void)
 {
-	return have_backendstats;
+	return (!pg_memory_is_all_zeros(&PendingBackendStats,
+									sizeof(struct PgStat_BackendPending)));
 }
 
 /*
@@ -209,7 +205,6 @@ pgstat_create_backend(ProcNumber procnum)
 	pgstat_unlock_entry(entry_ref);
 
 	MemSet(&PendingBackendStats, 0, sizeof(PgStat_BackendPending));
-	have_backendstats = false;
 }
 
 /*
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 000ed5b36f6..7222b414779 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -156,8 +156,8 @@ typedef struct PgStat_EntryRef
 	 * Pending statistics data that will need to be flushed to shared memory
 	 * stats eventually. Each stats kind utilizing pending data defines what
 	 * format its pending data has and needs to provide a
-	 * PgStat_KindInfo->flush_pending_cb callback to merge pending into shared
-	 * stats.
+	 * PgStat_KindInfo->flush_dynamic_cb callback to merge pending entries
+	 * that are in dynamic memory into shared stats.
 	 */
 	void	   *pending;
 	dlist_node	pending_node;	/* membership in pgStatPending list */
@@ -259,10 +259,11 @@ typedef struct PgStat_KindInfo
 	void		(*init_backend_cb) (void);
 
 	/*
-	 * For variable-numbered stats: flush pending stats within the dshash.
-	 * Required if pending data interacts with the pgstats dshash.
+	 * For variable-numbered stats: flush pending stats entries in dynamic
+	 * memory within the dshash. Required if pending data interacts with the
+	 * pgstats dshash.
 	 */
-	bool		(*flush_pending_cb) (PgStat_EntryRef *sr, bool nowait);
+	bool		(*flush_dynamic_cb) (PgStat_EntryRef *sr, bool nowait);
 
 	/*
 	 * For variable-numbered stats: delete pending stats. Optional.
@@ -298,10 +299,10 @@ typedef struct PgStat_KindInfo
 
 	/*
 	 * For fixed-numbered or variable-numbered statistics: Flush pending
-	 * stats. Returns true if some of the stats could not be flushed, due
-	 * to lock contention for example. Optional.
+	 * static stats. Returns true if some of the stats could not be flushed,
+	 * due to lock contention for example. Optional.
 	 */
-	bool		(*flush_cb) (bool nowait);
+	bool		(*flush_static_cb) (bool nowait);
 
 	/*
 	 * For fixed-numbered statistics: Reset All.
diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c
index 5db62bca66f..4f3691c702b 100644
--- a/src/test/modules/injection_points/injection_stats.c
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -48,7 +48,7 @@ static const PgStat_KindInfo injection_stats = {
 	.shared_data_off = offsetof(PgStatShared_InjectionPoint, stats),
 	.shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats),
 	.pending_size = sizeof(PgStat_StatInjEntry),
-	.flush_pending_cb = injection_stats_flush_cb,
+	.flush_dynamic_cb = injection_stats_flush_cb,
 };
 
 /*

---

From: Michael Paquier <[email-masked]>
Date: Mon, 20 Jan 2025 21:11:16 +0900

> .flush_static_cb(): flushes pending stats from static/global variable

"static" and "dynamic" feel a bit off when used together.  For
"static", the stats are from a static state but they can be pushed to
the dshash, as well, which is in dynamic shared memory.

Hmm.  Thinking more about that, I think that we should leave the
existing "flush_pending_cb" alone.  For the two others, how about
"have_static_pending_cb" and "flush_static_cb" rather than "fixed"?
It seems important to me to show the relationship between these two.

> the flush callback is responsible for) in v3 attached.

Hmm.  I think that I'm OK with what you are doing here with the next
parts in mind, based on this argument.

> pg_memory_is_all_zeros check on PendingBackendStats instead in the attached.

Indeed.  I got this part slightly wrong here.  What you are doing with
an all-zero check looks simpler in the long run.
--
Michael

---

From: Bertrand Drouvot <[email-masked]>
Date: Mon, 20 Jan 2025 13:26:55 +0000

Hi,

> the dshash, as well, which is in dynamic shared memory.

Right, that's another way to find it confusing ;-). But if we keep focus
on where the pending stats are stored, it is not. 

> It seems important to me to show the relationship between these two.

Yeah, that works for me: done in v4 attached.

Regards,

>From d57755402e9a9f672d2057c7f7ffa3832d402a3b Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Fri, 17 Jan 2025 06:44:47 +0000
Subject: [PATCH v4] Rework per backend pending stats

For those (the only current one is I/O statistics but WAL statistics is under
discussion), let's rely on a new PendingBackendStats instead.
---
 src/backend/utils/activity/pgstat.c          |  39 ++---
 src/backend/utils/activity/pgstat_backend.c  | 142 ++++++++++++-------
 src/backend/utils/activity/pgstat_io.c       |  23 +--
 src/backend/utils/activity/pgstat_relation.c |   4 +-
 src/include/pgstat.h                         |   9 ++
 src/include/utils/pgstat_internal.h          |  34 +++--
 6 files changed, 144 insertions(+), 107 deletions(-)
  76.8% src/backend/utils/activity/
  19.1% src/include/utils/
   4.0% src/include/

index 34520535d54..d4fc03b5d39 100644
 
-		.flush_pending_cb = pgstat_backend_flush_cb,
+		.have_static_pending_cb = pgstat_backend_have_pending_cb,
+		.flush_static_cb = pgstat_backend_flush_cb,
 		.reset_timestamp_cb = pgstat_backend_reset_timestamp_cb,
 	},

+		.have_static_pending_cb = pgstat_io_have_pending_cb,
 
+		.have_static_pending_cb = pgstat_slru_have_pending_cb,
 
+		.have_static_pending_cb = pgstat_wal_have_pending_cb,

 			if (!kind_info)
 				continue;
-			if (!kind_info->fixed_amount)
-			{
-				Assert(kind_info->have_fixed_pending_cb == NULL);
-				continue;
-			}
-			if (!kind_info->have_fixed_pending_cb)
+			if (!kind_info->have_static_pending_cb)
 				continue;
 
-			if (kind_info->have_fixed_pending_cb())
+			if (kind_info->have_static_pending_cb())
 			{
 				do_flush = true;
 				break;
@@ -789,25 +785,20 @@ pgstat_report_stat(bool force)
 
 	partial_flush = false;

 
 		if (!kind_info)
 			continue;

 	last_flush = now;
 
 #include "postgres.h"

+/*

 

 /*

 
 
-	return true;
+	return false;
 }
 
 /*
 
 /*
 
 /*

 	/*

 /*

 

 
 /*

 /*
 
index 4bb8e5c53ab..c0e77488aed 100644
+	 * PgStat_KindInfo->flush_pending_cb callback to merge pending entries
@@ -259,8 +259,9 @@ typedef struct PgStat_KindInfo
 
 	/*

 	/*
+	bool		(*have_static_pending_cb) (void);
 
 	/*
 
 	/*

 /*

---

From: Michael Paquier <[email-masked]>
Date: Tue, 21 Jan 2025 11:56:12 +0900

> on where the pending stats are stored, it is not.

The name of the callbacks can be tuned infinitely.  I have split the
renaming into its own patch, then the result looked fine after a
second look so I have applied both things separately to unlock all the
WAL patches.
--
Michael