Subject: Remove wal_[sync|write][_time] from pg_stat_wal and track_wal_io_timing

From: Bertrand Drouvot <[email-masked]>
Date: Tue, 18 Feb 2025 10:43:15 +0000

Hi hackers,

a051e71e28a added the WAL writes and fsyncs information (and their related timing)
into the pg_stat_io view so that one can now find the same information in 2 places:
pg_stat_wal and pg_stat_io (for the wal object). This lead to a discussion in 
[1] to remove this information from the pg_stat_wal view.

=== Let's sump up in this dedicated thread what has been discussed in [1]

1. The pg_stat_io's writes and fsyncs (and their relative timing) are incremented
at the exact same places as their pg_stat_wal counterparts.

2. pgstat_tracks_io_bktype() returns false for backend's that do not generate WAL
(so that we don't lost WAL information in pg_stat_io).

3. pg_stat_stat.wal_write is the same value as "select sum(writes)
from pg_stat_io where object = 'wal' and context = 'normal'" as these
are incremented in XLogWrite().

4. Same argument about pg_stat_wal.wal_write_time with
pg_stat_io.write_time.

5. issue_xlog_fsync() tells that pg_stat_wal.wal_sync_time and
sum(pg_stat_io.fsync_time) under object=wal and context=normal are the
same values.

6. Same argument with the fsync counters pg_stat_wal.wal_sync and
pg_stat_io.fsyncs.

7. This will encourage monitoring pull to move to pg_stat_io, where there is much
more context and granularity of the stats data. 

8. That will simplify the work done for the per backend WAL statistics ([1])

=== Remarks

R1. The "bytes" fields differ (as pg_stat_wal.wal_bytes somehow "focus" on the wal
records size while the pg_stat_io's unit is the wal_block_size) so we keep them
in both places.

R2. track_wal_io_timing becomes useless once those fields are gone from pg_stat_wal

R3. PendingWalStats becomes empty, so removes it. 

=== Patch structure

PFA 3 sub patches:

- 0001: Add details in the pg_stat_io doc about the wal object. That's basically
more or less a copy/paste from the pg_stat_wal fields description that will be
removed in 0002. As we already track them in pg_stat_io, it's done in a dedicated
patch.

- 0002: Remove wal_[sync|write][_time] from pg_stat_wal, PendingWalStats and 
track_wal_io_timing. That does not make sense to split this work in sub-patches
so that all of this in done in 0002.

- 0003: remove the pgstat_prepare_io_time() parameter. Now that track_wal_io_timing
is gone there is no need for pgstat_prepare_io_time() to get a parameter.

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

Looking forward to your feedback,

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
>From bc9dec707e2b12a6c6e7d1f654fbe41c8e5bbb65 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Tue, 18 Feb 2025 07:14:05 +0000
Subject: [PATCH v1 1/3] Add details in the pg_stat_io doc about the wal object

Adding details about writes, fsyncs, write_time and sync_time when linked
to the wal object. Same level of details as for wal_write, wal_sync, wal_write_time
and wal_sync_time in pg_stat_wal.
---
 doc/src/sgml/monitoring.sgml | 42 ++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 100.0% doc/src/sgml/

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 928a6eb64b0..2c61c0d1e50 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2930,6 +2930,48 @@ description | Waiting for a newly initialized WAL file to reach durable storage
    writer</literal>.
   </para>
 
+  <para>
+   For the <literal>wal</literal> <structfield>object</structfield>:
+   <itemizedlist>
+    <listitem>
+     <para>
+      <structfield>writes</structfield> is the number of times WAL buffers were
+       written out to disk via <function>XLogWrite</function> request.
+       See <xref linkend="wal-configuration"/> for more information about the internal
+       WAL function <function>XLogWrite</function>.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      <structfield>fsyncs</structfield> is the number of times WAL files were
+       synced to disk via <function>issue_xlog_fsync</function> request (if <xref linkend="guc-fsync"/>
+       is <literal>on</literal> and <xref linkend="guc-wal-sync-method"/> is either
+       <literal>fdatasync</literal>, <literal>fsync</literal> or <literal>fsync_writethrough</literal>,
+       otherwise zero). See <xref linkend="wal-configuration"/> for more information
+       about the internal WAL function <function>issue_xlog_fsync</function>.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      <structfield>write_time</structfield> is the total amount of time spent writing
+       WAL buffers to disk via <function>XLogWrite</function> request.
+       This includes the sync time when <varname>wal_sync_method</varname>
+       is either <literal>open_datasync</literal> or <literal>open_sync</literal>.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      <structfield>sync_time</structfield> is the total amount of time spent syncing
+       WAL files to disk via <function>issue_xlog_fsync</function> request (if
+       <varname>fsync</varname> is <literal>on</literal>,
+       and <varname>wal_sync_method</varname> is either <literal>fdatasync</literal>,
+       <literal>fsync</literal> or <literal>fsync_writethrough</literal>,
+       otherwise zero).
+     </para>
+    </listitem>
+   </itemizedlist>
+  </para>
+
   <para>
    <structname>pg_stat_io</structname> can be used to inform database tuning.
    For example:
-- 
2.34.1

>From 42b2d46d80f082829c91f9c91f844ab750f2cafd Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Tue, 18 Feb 2025 07:58:24 +0000
Subject: [PATCH v1 2/3] Remove wal_[sync|write][_time] from pg_stat_wal and
 track_wal_io_timing

a051e71e28a added this information into pg_stat_io (with more details and
granularity), so there is no need to keep it in pg_stat_wal. This also
allows to remove PendingWalStats and simplifies up coming commits related
to per backend WAL statistics. Once done, track_wal_io_timing becomes useless
so it is also removed.
---
 doc/src/sgml/config.sgml                      | 22 -------
 doc/src/sgml/monitoring.sgml                  | 62 -------------------
 doc/src/sgml/wal.sgml                         | 12 ++--
 src/backend/access/transam/xlog.c             | 38 ++----------
 src/backend/catalog/system_views.sql          |  4 --
 src/backend/utils/activity/pgstat_wal.c       | 24 +------
 src/backend/utils/adt/pgstatfuncs.c           | 20 +-----
 src/backend/utils/misc/guc_tables.c           |  9 ---
 src/backend/utils/misc/postgresql.conf.sample |  1 -
 src/include/access/xlog.h                     |  1 -
 src/include/catalog/pg_proc.dat               |  6 +-
 src/include/pgstat.h                          | 28 ---------
 src/test/regress/expected/rules.out           |  6 +-
 src/tools/pgindent/typedefs.list              |  1 -
 14 files changed, 19 insertions(+), 215 deletions(-)
  48.6% doc/src/sgml/
  12.7% src/backend/access/transam/
  11.1% src/backend/utils/activity/
  10.6% src/backend/utils/adt/
   3.4% src/backend/
   3.2% src/include/catalog/
   7.9% src/include/

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 336630ce417..05760241174 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8327,28 +8327,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-track-wal-io-timing" xreflabel="track_wal_io_timing">
-      <term><varname>track_wal_io_timing</varname> (<type>boolean</type>)
-      <indexterm>
-       <primary><varname>track_wal_io_timing</varname> configuration parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Enables timing of WAL I/O calls. This parameter is off by default,
-        as it will repeatedly query the operating system for the current time,
-        which may cause significant overhead on some platforms.
-        You can use the <application>pg_test_timing</application> tool to
-        measure the overhead of timing on your system.
-        I/O timing information is
-        displayed in <link linkend="monitoring-pg-stat-wal-view">
-        <structname>pg_stat_wal</structname></link>.
-        Only superusers and users with the appropriate <literal>SET</literal>
-        privilege can change this setting.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-track-functions" xreflabel="track_functions">
       <term><varname>track_functions</varname> (<type>enum</type>)
       <indexterm>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 2c61c0d1e50..8cbc89aa797 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -183,11 +183,6 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
    of block read, write, extend, and fsync times.
   </para>
 
-  <para>
-   The parameter <xref linkend="guc-track-wal-io-timing"/> enables monitoring
-   of WAL write and fsync times.
-  </para>
-
   <para>
    Normally these parameters are set in <filename>postgresql.conf</filename> so
    that they apply to all server processes, but it is possible to turn
@@ -3297,63 +3292,6 @@ description | Waiting for a newly initialized WAL file to reach durable storage
       </para></entry>
      </row>
 
-     <row>
-      <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>wal_write</structfield> <type>bigint</type>
-      </para>
-      <para>
-       Number of times WAL buffers were written out to disk via
-       <function>XLogWrite</function> request.
-       See <xref linkend="wal-configuration"/> for more information about
-       the internal WAL function <function>XLogWrite</function>.
-      </para></entry>
-     </row>
-
-     <row>
-      <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>wal_sync</structfield> <type>bigint</type>
-      </para>
-      <para>
-       Number of times WAL files were synced to disk via
-       <function>issue_xlog_fsync</function> request
-       (if <xref linkend="guc-fsync"/> is <literal>on</literal> and
-       <xref linkend="guc-wal-sync-method"/> is either
-       <literal>fdatasync</literal>, <literal>fsync</literal> or
-       <literal>fsync_writethrough</literal>, otherwise zero).
-       See <xref linkend="wal-configuration"/> for more information about
-       the internal WAL function <function>issue_xlog_fsync</function>.
-      </para></entry>
-     </row>
-
-     <row>
-      <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>wal_write_time</structfield> <type>double precision</type>
-      </para>
-      <para>
-       Total amount of time spent writing WAL buffers to disk via
-       <function>XLogWrite</function> request, in milliseconds
-       (if <xref linkend="guc-track-wal-io-timing"/> is enabled,
-       otherwise zero).  This includes the sync time when
-       <varname>wal_sync_method</varname> is either
-       <literal>open_datasync</literal> or <literal>open_sync</literal>.
-      </para></entry>
-     </row>
-
-     <row>
-      <entry role="catalog_table_entry"><para role="column_definition">
-       <structfield>wal_sync_time</structfield> <type>double precision</type>
-      </para>
-      <para>
-       Total amount of time spent syncing WAL files to disk via
-       <function>issue_xlog_fsync</function> request, in milliseconds
-       (if <varname>track_wal_io_timing</varname> is enabled,
-       <varname>fsync</varname> is <literal>on</literal>, and
-       <varname>wal_sync_method</varname> is either
-       <literal>fdatasync</literal>, <literal>fsync</literal> or
-       <literal>fsync_writethrough</literal>, otherwise zero).
-      </para></entry>
-     </row>
-
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>stats_reset</structfield> <type>timestamp with time zone</type>
diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml
index b908720adea..8f3ac982b0e 100644
--- a/doc/src/sgml/wal.sgml
+++ b/doc/src/sgml/wal.sgml
@@ -810,11 +810,11 @@
   <para>
    There are two internal functions to write WAL data to disk:
    <function>XLogWrite</function> and <function>issue_xlog_fsync</function>.
-   When <xref linkend="guc-track-wal-io-timing"/> is enabled, the total
+   When <xref linkend="guc-track-io-timing"/> is enabled, the total
    amounts of time <function>XLogWrite</function> writes and
    <function>issue_xlog_fsync</function> syncs WAL data to disk are counted as
-   <literal>wal_write_time</literal> and <literal>wal_sync_time</literal> in
-   <xref linkend="pg-stat-wal-view"/>, respectively.
+   <literal>write_time</literal> and <literal>sync_time</literal> in
+   <xref linkend="pg-stat-io-view"/> for the wal <literal>object</literal>, respectively.
    <function>XLogWrite</function> is normally called by
    <function>XLogInsertRecord</function> (when there is no space for the new
    record in WAL buffers), <function>XLogFlush</function> and the WAL writer,
@@ -829,11 +829,11 @@
    <literal>fsync</literal>, or <literal>fsync_writethrough</literal>,
    the write operation moves WAL buffers to kernel cache and
    <function>issue_xlog_fsync</function> syncs them to disk. Regardless
-   of the setting of <varname>track_wal_io_timing</varname>, the number
+   of the setting of <varname>track_io_timing</varname>, the number
    of times <function>XLogWrite</function> writes and
    <function>issue_xlog_fsync</function> syncs WAL data to disk are also
-   counted as <literal>wal_write</literal> and <literal>wal_sync</literal>
-   in <structname>pg_stat_wal</structname>, respectively.
+   counted as <literal>writes</literal> and <literal>fsyncs</literal>
+   in <structname>pg_stat_io</structname> for the wal <literal>object</literal>, respectively.
   </para>
 
   <para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 25a5c605404..5a1940a42f5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -134,7 +134,6 @@ int			CommitSiblings = 5; /* # concurrent xacts needed to sleep */
 int			wal_retrieve_retry_interval = 5000;
 int			max_slot_wal_keep_size_mb = -1;
 int			wal_decode_buffer_size = 512 * 1024;
-bool		track_wal_io_timing = false;
 
 #ifdef WAL_DEBUG
 bool		XLOG_DEBUG = false;
@@ -2436,10 +2435,9 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible)
 				errno = 0;
 
 				/*
-				 * Measure I/O timing to write WAL data, for pg_stat_io and/or
-				 * pg_stat_wal.
+				 * Measure I/O timing to write WAL data, for pg_stat_io.
 				 */
-				start = pgstat_prepare_io_time(track_io_timing || track_wal_io_timing);
+				start = pgstat_prepare_io_time(track_io_timing);
 
 				pgstat_report_wait_start(WAIT_EVENT_WAL_WRITE);
 				written = pg_pwrite(openLogFile, from, nleft, startoffset);
@@ -2448,20 +2446,6 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible)
 				pgstat_count_io_op_time(IOOBJECT_WAL, IOCONTEXT_NORMAL,
 										IOOP_WRITE, start, 1, written);
 
-				/*
-				 * Increment the I/O timing and the number of times WAL data
-				 * were written out to disk.
-				 */
-				if (track_wal_io_timing)
-				{
-					instr_time	end;
-
-					INSTR_TIME_SET_CURRENT(end);
-					INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_write_time, end, start);
-				}
-
-				PendingWalStats.wal_write++;
-
 				if (written <= 0)
 				{
 					char		xlogfname[MAXFNAMELEN];
@@ -8718,10 +8702,9 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
 		return;
 
 	/*
-	 * Measure I/O timing to sync the WAL file for pg_stat_io and/or
-	 * pg_stat_wal.
+	 * Measure I/O timing to sync the WAL file for pg_stat_io.
 	 */
-	start = pgstat_prepare_io_time(track_io_timing || track_wal_io_timing);
+	start = pgstat_prepare_io_time(track_io_timing);
 
 	pgstat_report_wait_start(WAIT_EVENT_WAL_SYNC);
 	switch (wal_sync_method)
@@ -8767,21 +8750,8 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
 
 	pgstat_report_wait_end();
 
-	/*
-	 * Increment the I/O timing and the number of times WAL files were synced.
-	 */
-	if (track_wal_io_timing)
-	{
-		instr_time	end;
-
-		INSTR_TIME_SET_CURRENT(end);
-		INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, end, start);
-	}
-
 	pgstat_count_io_op_time(IOOBJECT_WAL, IOCONTEXT_NORMAL, IOOP_FSYNC,
 							start, 1, 0);
-
-	PendingWalStats.wal_sync++;
 }
 
 /*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index eff0990957e..a4d2cfdcaf5 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1189,10 +1189,6 @@ CREATE VIEW pg_stat_wal AS
         w.wal_fpi,
         w.wal_bytes,
         w.wal_buffers_full,
-        w.wal_write,
-        w.wal_sync,
-        w.wal_write_time,
-        w.wal_sync_time,
         w.stats_reset
     FROM pg_stat_get_wal() w;
 
diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c
index c1ca65eb991..4dc41a4a590 100644
--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -21,8 +21,6 @@
 #include "utils/pgstat_internal.h"

-PgStat_PendingWalStats PendingWalStats = {0};
-
 /*
  * WAL usage counters saved from pgWalUsage at the previous call to
  * pgstat_report_wal(). This is used to calculate how much WAL usage
@@ -118,17 +116,10 @@ pgstat_wal_flush_cb(bool nowait)
 
 #define WALSTAT_ACC(fld, var_to_add) \
 	(stats_shmem->stats.fld += var_to_add.fld)
-#define WALSTAT_ACC_INSTR_TIME(fld) \
-	(stats_shmem->stats.fld += INSTR_TIME_GET_MICROSEC(PendingWalStats.fld))
 	WALSTAT_ACC(wal_records, wal_usage_diff);
 	WALSTAT_ACC(wal_fpi, wal_usage_diff);
 	WALSTAT_ACC(wal_bytes, wal_usage_diff);
 	WALSTAT_ACC(wal_buffers_full, wal_usage_diff);
-	WALSTAT_ACC(wal_write, PendingWalStats);
-	WALSTAT_ACC(wal_sync, PendingWalStats);
-	WALSTAT_ACC_INSTR_TIME(wal_write_time);
-	WALSTAT_ACC_INSTR_TIME(wal_sync_time);
-#undef WALSTAT_ACC_INSTR_TIME
 #undef WALSTAT_ACC
 
 	LWLockRelease(&stats_shmem->lock);
@@ -138,11 +129,6 @@ pgstat_wal_flush_cb(bool nowait)
 	 */
 	prevWalUsage = pgWalUsage;
 
-	/*
-	 * Clear out the statistics buffer, so it can be re-used.
-	 */
-	MemSet(&PendingWalStats, 0, sizeof(PendingWalStats));
-
 	return false;
 }
 
@@ -158,18 +144,12 @@ pgstat_wal_init_backend_cb(void)
 }
 
 /*
- * To determine whether any WAL activity has occurred since last time, not
- * only the number of generated WAL records but also the numbers of WAL
- * writes and syncs need to be checked. Because even transaction that
- * generates no WAL records can write or sync WAL data when flushing the
- * data pages.
+ * To determine whether WAL usage happened.
  */
 bool
 pgstat_wal_have_pending_cb(void)
 {
-	return pgWalUsage.wal_records != prevWalUsage.wal_records ||
-		PendingWalStats.wal_write != 0 ||
-		PendingWalStats.wal_sync != 0;
+	return pgWalUsage.wal_records != prevWalUsage.wal_records;
 }
 
 void
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index e9096a88492..68e16e52ab6 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1637,7 +1637,7 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS)
 Datum
 pg_stat_get_wal(PG_FUNCTION_ARGS)
 {
-#define PG_STAT_GET_WAL_COLS	9
+#define PG_STAT_GET_WAL_COLS	5
 	TupleDesc	tupdesc;
 	Datum		values[PG_STAT_GET_WAL_COLS] = {0};
 	bool		nulls[PG_STAT_GET_WAL_COLS] = {0};
@@ -1654,15 +1654,7 @@ pg_stat_get_wal(PG_FUNCTION_ARGS)
 					   NUMERICOID, -1, 0);
 	TupleDescInitEntry(tupdesc, (AttrNumber) 4, "wal_buffers_full",
 					   INT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 5, "wal_write",
-					   INT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 6, "wal_sync",
-					   INT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 7, "wal_write_time",
-					   FLOAT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 8, "wal_sync_time",
-					   FLOAT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 9, "stats_reset",
+	TupleDescInitEntry(tupdesc, (AttrNumber) 5, "stats_reset",
 					   TIMESTAMPTZOID, -1, 0);
 
 	BlessTupleDesc(tupdesc);
@@ -1682,14 +1674,8 @@ pg_stat_get_wal(PG_FUNCTION_ARGS)
 									Int32GetDatum(-1));
 
 	values[3] = Int64GetDatum(wal_stats->wal_buffers_full);
-	values[4] = Int64GetDatum(wal_stats->wal_write);
-	values[5] = Int64GetDatum(wal_stats->wal_sync);
-
-	/* Convert counters from microsec to millisec for display */
-	values[6] = Float8GetDatum(((double) wal_stats->wal_write_time) / 1000.0);
-	values[7] = Float8GetDatum(((double) wal_stats->wal_sync_time) / 1000.0);
 
-	values[8] = TimestampTzGetDatum(wal_stats->stat_reset_timestamp);
+	values[4] = TimestampTzGetDatum(wal_stats->stat_reset_timestamp);
 
 	/* Returns the record as Datum */
 	PG_RETURN_DATUM(HeapTupleGetDatum(heap_form_tuple(tupdesc, values, nulls)));
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index cce73314609..6d4cf7609a7 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1499,15 +1499,6 @@ struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
-	{
-		{"track_wal_io_timing", PGC_SUSET, STATS_CUMULATIVE,
-			gettext_noop("Collects timing statistics for WAL I/O activity."),
-			NULL
-		},
-		&track_wal_io_timing,
-		false,
-		NULL, NULL, NULL
-	},
 
 	{
 		{"update_process_title", PGC_SUSET, PROCESS_TITLE,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index d472987ed46..c64b3e8a0e6 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -637,7 +637,6 @@
 #track_counts = on
 #track_cost_delay_timing = off
 #track_io_timing = off
-#track_wal_io_timing = off
 #track_functions = none			# none, pl, all
 #stats_fetch_consistency = cache	# cache, none, snapshot
 
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 4411c1468ac..bd18d95a6cf 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -54,7 +54,6 @@ extern PGDLLIMPORT char *wal_consistency_checking_string;
 extern PGDLLIMPORT bool log_checkpoints;
 extern PGDLLIMPORT int CommitDelay;
 extern PGDLLIMPORT int CommitSiblings;
-extern PGDLLIMPORT bool track_wal_io_timing;
 extern PGDLLIMPORT int wal_decode_buffer_size;
 
 extern PGDLLIMPORT int CheckPointSegments;
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9e803d610d7..1e1626964e3 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5950,9 +5950,9 @@
 { oid => '1136', descr => 'statistics: information about WAL activity',
   proname => 'pg_stat_get_wal', proisstrict => 'f', provolatile => 's',
   proparallel => 'r', prorettype => 'record', proargtypes => '',
-  proallargtypes => '{int8,int8,numeric,int8,int8,int8,float8,float8,timestamptz}',
-  proargmodes => '{o,o,o,o,o,o,o,o,o}',
-  proargnames => '{wal_records,wal_fpi,wal_bytes,wal_buffers_full,wal_write,wal_sync,wal_write_time,wal_sync_time,stats_reset}',
+  proallargtypes => '{int8,int8,numeric,int8,timestamptz}',
+  proargmodes => '{o,o,o,o,o}',
+  proargnames => '{wal_records,wal_fpi,wal_bytes,wal_buffers_full,stats_reset}',
   prosrc => 'pg_stat_get_wal' },
 { oid => '6248', descr => 'statistics: information about WAL prefetching',
   proname => 'pg_stat_get_recovery_prefetch', prorows => '1', proretset => 't',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 53f2a8458e6..a3a341cc604 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -480,28 +480,9 @@ typedef struct PgStat_WalStats
 	PgStat_Counter wal_fpi;
 	uint64		wal_bytes;
 	PgStat_Counter wal_buffers_full;
-	PgStat_Counter wal_write;
-	PgStat_Counter wal_sync;
-	PgStat_Counter wal_write_time;
-	PgStat_Counter wal_sync_time;
 	TimestampTz stat_reset_timestamp;
 } PgStat_WalStats;
 
-/*
- * This struct stores wal-related durations as instr_time, which makes it
- * cheaper and easier to accumulate them, by not requiring type
- * conversions. During stats flush instr_time will be converted into
- * microseconds.
- */
-typedef struct PgStat_PendingWalStats
-{
-	PgStat_Counter wal_write;
-	PgStat_Counter wal_sync;
-	instr_time	wal_write_time;
-	instr_time	wal_sync_time;
-} PgStat_PendingWalStats;
-
-
 /*
  * Functions in pgstat.c
  */
@@ -834,13 +815,4 @@ extern PGDLLIMPORT PgStat_Counter pgStatTransactionIdleTime;
 /* updated by the traffic cop and in errfinish() */
 extern PGDLLIMPORT SessionEndType pgStatSessionEndCause;
 
-
-/*
- * Variables in pgstat_wal.c
- */
-
-/* updated directly by backends and background processes */
-extern PGDLLIMPORT PgStat_PendingWalStats PendingWalStats;
-
-
 #endif							/* PGSTAT_H */
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 5baba8d39ff..62f69ac20b2 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2259,12 +2259,8 @@ pg_stat_wal| SELECT wal_records,
     wal_fpi,
     wal_bytes,
     wal_buffers_full,
-    wal_write,
-    wal_sync,
-    wal_write_time,
-    wal_sync_time,
     stats_reset
-   FROM pg_stat_get_wal() w(wal_records, wal_fpi, wal_bytes, wal_buffers_full, wal_write, wal_sync, wal_write_time, wal_sync_time, stats_reset);
+   FROM pg_stat_get_wal() w(wal_records, wal_fpi, wal_bytes, wal_buffers_full, stats_reset);
 pg_stat_wal_receiver| SELECT pid,
     status,
     receive_start_lsn,
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index bce4214503d..740777127e9 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2163,7 +2163,6 @@ PgStat_KindInfo
 PgStat_LocalState
 PgStat_PendingDroppedStatsItem
 PgStat_PendingIO
-PgStat_PendingWalStats
 PgStat_SLRUStats
 PgStat_ShmemControl
 PgStat_Snapshot
-- 
2.34.1

>From 6f878f1bbfb119b1dbe95b4cb26b30c97b7a047e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email-masked]>
Date: Tue, 18 Feb 2025 08:52:41 +0000
Subject: [PATCH v1 3/3] remove the pgstat_prepare_io_time() parameter

Now that track_wal_io_timing is gone there is no need for pgstat_prepare_io_time()
to get a parameter.
---
 src/backend/access/transam/xlog.c         |  8 ++++----
 src/backend/access/transam/xlogreader.c   |  2 +-
 src/backend/access/transam/xlogrecovery.c |  2 +-
 src/backend/storage/buffer/bufmgr.c       | 10 +++++-----
 src/backend/storage/buffer/localbuf.c     |  4 ++--
 src/backend/storage/smgr/md.c             |  4 ++--
 src/backend/utils/activity/pgstat_io.c    |  7 +++----
 src/include/pgstat.h                      |  2 +-
 8 files changed, 19 insertions(+), 20 deletions(-)
  31.1% src/backend/access/transam/
  37.2% src/backend/storage/buffer/
  10.5% src/backend/storage/smgr/
  14.7% src/backend/utils/activity/
   6.2% src/include/

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5a1940a42f5..24624c84748 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2437,7 +2437,7 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible)
 				/*
 				 * Measure I/O timing to write WAL data, for pg_stat_io.
 				 */
-				start = pgstat_prepare_io_time(track_io_timing);
+				start = pgstat_prepare_io_time();
 
 				pgstat_report_wait_start(WAIT_EVENT_WAL_WRITE);
 				written = pg_pwrite(openLogFile, from, nleft, startoffset);
@@ -3248,7 +3248,7 @@ XLogFileInitInternal(XLogSegNo logsegno, TimeLineID logtli,
 				 errmsg("could not create file \"%s\": %m", tmppath)));
 
 	/* Measure I/O timing when initializing segment */
-	io_start = pgstat_prepare_io_time(track_io_timing);
+	io_start = pgstat_prepare_io_time();
 
 	pgstat_report_wait_start(WAIT_EVENT_WAL_INIT_WRITE);
 	save_errno = 0;
@@ -3310,7 +3310,7 @@ XLogFileInitInternal(XLogSegNo logsegno, TimeLineID logtli,
 	}
 
 	/* Measure I/O timing when flushing segment */
-	io_start = pgstat_prepare_io_time(track_io_timing);
+	io_start = pgstat_prepare_io_time();
 
 	pgstat_report_wait_start(WAIT_EVENT_WAL_INIT_SYNC);
 	if (pg_fsync(fd) != 0)
@@ -8704,7 +8704,7 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
 	/*
 	 * Measure I/O timing to sync the WAL file for pg_stat_io.
 	 */
-	start = pgstat_prepare_io_time(track_io_timing);
+	start = pgstat_prepare_io_time();
 
 	pgstat_report_wait_start(WAIT_EVENT_WAL_SYNC);
 	switch (wal_sync_method)
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 31bffc6f501..995e4f7c26f 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1557,7 +1557,7 @@ WALRead(XLogReaderState *state,
 
 #ifndef FRONTEND
 		/* Measure I/O timing when reading segment */
-		io_start = pgstat_prepare_io_time(track_io_timing);
+		io_start = pgstat_prepare_io_time();
 
 		pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
 #endif
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 473de6710d7..ac70b73749c 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -3401,7 +3401,7 @@ retry:
 	readOff = targetPageOff;
 
 	/* Measure I/O timing when reading segment */
-	io_start = pgstat_prepare_io_time(track_io_timing);
+	io_start = pgstat_prepare_io_time();
 
 	pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
 	r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 80b0d0c5ded..e880bed4627 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1509,7 +1509,7 @@ WaitReadBuffers(ReadBuffersOperation *operation)
 			io_pages[io_buffers_len++] = BufferGetBlock(buffers[i]);
 		}
 
-		io_start = pgstat_prepare_io_time(track_io_timing);
+		io_start = pgstat_prepare_io_time();
 		smgrreadv(operation->smgr, forknum, io_first_block, io_pages, io_buffers_len);
 		pgstat_count_io_op_time(io_object, io_context, IOOP_READ, io_start,
 								1, io_buffers_len * BLCKSZ);
@@ -2401,7 +2401,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr,
 		}
 	}
 
-	io_start = pgstat_prepare_io_time(track_io_timing);
+	io_start = pgstat_prepare_io_time();
 
 	/*
 	 * Note: if smgrzeroextend fails, we will end up with buffers that are
@@ -3858,7 +3858,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
 	 */
 	bufToWrite = PageSetChecksumCopy((Page) bufBlock, buf->tag.blockNum);
 
-	io_start = pgstat_prepare_io_time(track_io_timing);
+	io_start = pgstat_prepare_io_time();
 
 	/*
 	 * bufToWrite is either the shared buffer or a copy, as appropriate.
@@ -4459,7 +4459,7 @@ FlushRelationBuffers(Relation rel)
 
 				PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
 
-				io_start = pgstat_prepare_io_time(track_io_timing);
+				io_start = pgstat_prepare_io_time();
 
 				smgrwrite(srel,
 						  BufTagGetForkNum(&bufHdr->tag),
@@ -5916,7 +5916,7 @@ IssuePendingWritebacks(WritebackContext *wb_context, IOContext io_context)
 	sort_pending_writebacks(wb_context->pending_writebacks,
 							wb_context->nr_pending);
 
-	io_start = pgstat_prepare_io_time(track_io_timing);
+	io_start = pgstat_prepare_io_time();
 
 	/*
 	 * Coalesce neighbouring writes, but nothing else. For that we iterate
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 64931efaa75..78b65fb160a 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -244,7 +244,7 @@ GetLocalVictimBuffer(void)
 
 		PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
 
-		io_start = pgstat_prepare_io_time(track_io_timing);
+		io_start = pgstat_prepare_io_time();
 
 		/* And write... */
 		smgrwrite(oreln,
@@ -414,7 +414,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr,
 		}
 	}
 
-	io_start = pgstat_prepare_io_time(track_io_timing);
+	io_start = pgstat_prepare_io_time();
 
 	/* actually extend relation */
 	smgrzeroextend(bmr.smgr, fork, first_block, extend_by, false);
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 7bf0b45e2c3..298754d1b64 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -1381,7 +1381,7 @@ register_dirty_segment(SMgrRelation reln, ForkNumber forknum, MdfdVec *seg)
 		ereport(DEBUG1,
 				(errmsg_internal("could not forward fsync request because request queue is full")));
 
-		io_start = pgstat_prepare_io_time(track_io_timing);
+		io_start = pgstat_prepare_io_time();
 
 		if (FileSync(seg->mdfd_vfd, WAIT_EVENT_DATA_FILE_SYNC) < 0)
 			ereport(data_sync_elevel(ERROR),
@@ -1786,7 +1786,7 @@ mdsyncfiletag(const FileTag *ftag, char *path)
 		need_to_close = true;
 	}
 
-	io_start = pgstat_prepare_io_time(track_io_timing);
+	io_start = pgstat_prepare_io_time();
 
 	/* Sync the file. */
 	result = FileSync(file, WAIT_EVENT_DATA_FILE_SYNC);
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index 28a431084b8..e7362b52a37 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -83,15 +83,14 @@ pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op,
 }
 
 /*
- * Initialize the internal timing for an IO operation, depending on an
- * IO timing GUC.
+ * Initialize the internal timing for an IO operation.
  */
 instr_time
-pgstat_prepare_io_time(bool track_io_guc)
+pgstat_prepare_io_time(void)
 {
 	instr_time	io_start;
 
-	if (track_io_guc)
+	if (track_io_timing)
 		INSTR_TIME_SET_CURRENT(io_start);
 	else
 	{
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index a3a341cc604..030b101028f 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -564,7 +564,7 @@ extern bool pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io,
 										 BackendType bktype);
 extern void pgstat_count_io_op(IOObject io_object, IOContext io_context,
 							   IOOp io_op, uint32 cnt, uint64 bytes);
-extern instr_time pgstat_prepare_io_time(bool track_io_guc);
+extern instr_time pgstat_prepare_io_time(void);
 extern void pgstat_count_io_op_time(IOObject io_object, IOContext io_context,
 									IOOp io_op, instr_time start_time,
 									uint32 cnt, uint64 bytes);
-- 
2.34.1

---

From: Michael Paquier <[email-masked]>
Date: Wed, 19 Feb 2025 13:32:33 +0900

> === Remarks

As mentioned on a previous thread, I am siding with the removal of
these fields.  There are more benefits on terms of granularity of this
data to encourage users to move away from pg_stat_wal and stick to
pg_stat_io.  Reasons are posted here:
https://www.postgresql.org/message-id/[email-masked]

> records size while the pg_stat_io's unit is the wal_block_size) so we keep them

Yes, I don't think we should drop that anyway.  There is no equivalent
mapping and pg_stat_wal.bytes is not I/O.  This is also useful in the
WAL stats for the backends.

> R3. PendingWalStats becomes empty, so removes it. 

Sounds fine to me.  The simplifications are in pgstat_wal.c, where we
can now depend only on the diffs in the WalUsage, which we rely on for
EXPLAIN, VACUUM/ANALYZE and pgss.  This makes the implementation of
the backend WAL stats much simpler.

> removed in 0002. As we already track them in pg_stat_io, it's done in a dedicated

These additions feel unbalanced with the existing contents, and
overlap with the follow-up paragraph about the tuning that can be
guessed from the contents of pg_stat_io because the new content refers
twice to the section "WAL Configuration".  Perhaps this could be
simpler, with one sentence in the tuning part?  I would propose
something like:
For the WAL object, "fsyncs" and "sync_time" track the sync activity
of WAL files via issue_xlog_fsync().  "writes" and "write_time" track
the write activity of WAL files via XLogWrite().  See <xref
linkend="wal-configuration"/> for more information.

My point is that the "WAL configuration" section already provides all
the details that were in pg_stat_wal removed in 0002 and added in
0001, leading to duplicated contents.

> is gone there is no need for pgstat_prepare_io_time() to get a parameter.

0002 and 0003 can be grouped in the same commit, IMO.

Okay to update these are part of 0002 that removes the fields, but
there is also an argument about updating that on its own because this
is actually the case on HEAD?  (No need to post an updated patch for
this remark.)

The removal with PgStat_PendingWalStats and the GUC gone looks clean
as presented.
--
Michael