From f17300193ab350c20100c8101bc2ca5f74b5c109 Mon Sep 17 00:00:00 2001
From: Nitin Jadhav <nitinjadhav@microsoft.com>
Date: Tue, 16 Jul 2024 08:32:00 +0000
Subject: [PATCH] Fix inconsistency in reporting checkpointer stats.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The information about buffers written, which is revealed through the
pg_stat_checkpointer view, has been found to be inconsistent with the
data in the checkpoint complete log message. To address this, a new
column named ‘slru_written’ has been added to the pg_stat_checkpointer
view. This column displays the SLRU buffers written during checkpoints.
The existing ‘buffers_written’ column continues to represent the main
buffers written during checkpoints. Furthermore, the information about
buffers written, which is logged in the checkpoint complete log message,
includes both the main buffer count and the SLRU buffer count. This
patch distinguishes between these two types of information.
---
 doc/src/sgml/monitoring.sgml                  |  9 ++++++
 src/backend/access/transam/slru.c             |  5 +++-
 src/backend/access/transam/xlog.c             | 28 +++++++++++--------
 src/backend/catalog/system_views.sql          |  1 +
 .../utils/activity/pgstat_checkpointer.c      |  1 +
 src/backend/utils/adt/pgstatfuncs.c           |  6 ++++
 src/include/access/xlog.h                     |  1 +
 src/include/catalog/pg_proc.dat               |  5 ++++
 src/include/pgstat.h                          |  1 +
 src/test/regress/expected/rules.out           |  1 +
 10 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 55417a6fa9..b2fe425268 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3049,6 +3049,15 @@ 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>slru_written</structfield> <type>bigint</type>
+      </para>
+      <para>
+        Number of SLRU buffers written during checkpoints and restartpoints
+      </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/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 77b05cc0a7..de59bcdf1c 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -703,7 +703,10 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata)
 
 	/* If part of a checkpoint, count this as a buffer written. */
 	if (fdata)
-		CheckpointStats.ckpt_bufs_written++;
+	{
+		CheckpointStats.ckpt_slru_written++;
+		PendingCheckpointerStats.slru_written++;
+	}
 }
 
 /*
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 33e27a6e72..19d7fbf70a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6666,14 +6666,16 @@ LogCheckpointEnd(bool restartpoint)
 	 */
 	if (restartpoint)
 		ereport(LOG,
-				(errmsg("restartpoint complete: wrote %d buffers (%.1f%%); "
-						"%d WAL file(s) added, %d removed, %d recycled; "
-						"write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
-						"sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
-						"distance=%d kB, estimate=%d kB; "
-						"lsn=%X/%X, redo lsn=%X/%X",
+				(errmsg("restartpoint complete: wrote %d buffers (%.1f%%), "
+						"wrote %d slru buffers (%.1f%%); %d WAL file(s) added, "
+						"%d removed, %d recycled; write=%ld.%03d s, "
+						"sync=%ld.%03d s, total=%ld.%03d s; sync files=%d, "
+						"longest=%ld.%03d s, average=%ld.%03d s; distance=%d kB, "
+						"estimate=%d kB; lsn=%X/%X, redo lsn=%X/%X",
 						CheckpointStats.ckpt_bufs_written,
 						(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
+						CheckpointStats.ckpt_slru_written,
+						(double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
 						CheckpointStats.ckpt_segs_added,
 						CheckpointStats.ckpt_segs_removed,
 						CheckpointStats.ckpt_segs_recycled,
@@ -6689,14 +6691,16 @@ LogCheckpointEnd(bool restartpoint)
 						LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
 	else
 		ereport(LOG,
-				(errmsg("checkpoint complete: wrote %d buffers (%.1f%%); "
-						"%d WAL file(s) added, %d removed, %d recycled; "
-						"write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
-						"sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
-						"distance=%d kB, estimate=%d kB; "
-						"lsn=%X/%X, redo lsn=%X/%X",
+				(errmsg("checkpoint complete: wrote %d buffers (%.1f%%), "
+						"wrote %d slru buffers (%.1f%%); %d WAL file(s) added, "
+						"%d removed, %d recycled; write=%ld.%03d s, "
+						"sync=%ld.%03d s, total=%ld.%03d s; sync files=%d, "
+						"longest=%ld.%03d s, average=%ld.%03d s; distance=%d kB, "
+						"estimate=%d kB; lsn=%X/%X, redo lsn=%X/%X",
 						CheckpointStats.ckpt_bufs_written,
 						(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
+						CheckpointStats.ckpt_slru_written,
+						(double) CheckpointStats.ckpt_slru_written * 100 / NBuffers,
 						CheckpointStats.ckpt_segs_added,
 						CheckpointStats.ckpt_segs_removed,
 						CheckpointStats.ckpt_segs_recycled,
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 19cabc9a47..aaaa2b3474 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1144,6 +1144,7 @@ CREATE VIEW pg_stat_checkpointer AS
         pg_stat_get_checkpointer_write_time() AS write_time,
         pg_stat_get_checkpointer_sync_time() AS sync_time,
         pg_stat_get_checkpointer_buffers_written() AS buffers_written,
+        pg_stat_get_checkpointer_slru_written() AS slru_written,
         pg_stat_get_checkpointer_stat_reset_time() AS stats_reset;
 
 CREATE VIEW pg_stat_io AS
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index bbfc9c7e18..98d16c7868 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -55,6 +55,7 @@ pgstat_report_checkpointer(void)
 	CHECKPOINTER_ACC(write_time);
 	CHECKPOINTER_ACC(sync_time);
 	CHECKPOINTER_ACC(buffers_written);
+	CHECKPOINTER_ACC(slru_written);
 #undef CHECKPOINTER_ACC
 
 	pgstat_end_changecount_write(&stats_shmem->changecount);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 3876339ee1..f3ca746818 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1215,6 +1215,12 @@ pg_stat_get_checkpointer_buffers_written(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->buffers_written);
 }
 
+Datum
+pg_stat_get_checkpointer_slru_written(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->slru_written);
+}
+
 Datum
 pg_stat_get_bgwriter_buf_written_clean(PG_FUNCTION_ARGS)
 {
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 1a1f11a943..23a264ef23 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -165,6 +165,7 @@ typedef struct CheckpointStatsData
 	TimestampTz ckpt_end_t;		/* end of checkpoint */
 
 	int			ckpt_bufs_written;	/* # of buffers written */
+	int			ckpt_slru_written;	/* # of SLRU buffers written */
 
 	int			ckpt_segs_added;	/* # of new xlog segments created */
 	int			ckpt_segs_removed;	/* # of xlog segments deleted */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 73d9cf8582..6adb683553 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5757,6 +5757,11 @@
   proname => 'pg_stat_get_checkpointer_buffers_written', provolatile => 's',
   proparallel => 'r', prorettype => 'int8', proargtypes => '',
   prosrc => 'pg_stat_get_checkpointer_buffers_written' },
+{ oid => '3814',
+  descr => 'statistics: number of SLRU buffers written during checkpoints and restartpoints',
+  proname => 'pg_stat_get_checkpointer_slru_written', provolatile => 's',
+  proparallel => 'r', prorettype => 'int8', proargtypes => '',
+  prosrc => 'pg_stat_get_checkpointer_slru_written' },
 { oid => '6314', descr => 'statistics: last reset for the checkpointer',
   proname => 'pg_stat_get_checkpointer_stat_reset_time', provolatile => 's',
   proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 6b99bb8aad..eedcc7d4a2 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -268,6 +268,7 @@ typedef struct PgStat_CheckpointerStats
 	PgStat_Counter write_time;	/* times in milliseconds */
 	PgStat_Counter sync_time;
 	PgStat_Counter buffers_written;
+	PgStat_Counter slru_written;
 	TimestampTz stat_reset_timestamp;
 } PgStat_CheckpointerStats;
 
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 4c789279e5..4b1ff2d26e 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1830,6 +1830,7 @@ pg_stat_checkpointer| SELECT pg_stat_get_checkpointer_num_timed() AS num_timed,
     pg_stat_get_checkpointer_write_time() AS write_time,
     pg_stat_get_checkpointer_sync_time() AS sync_time,
     pg_stat_get_checkpointer_buffers_written() AS buffers_written,
+    pg_stat_get_checkpointer_slru_written() AS slru_written,
     pg_stat_get_checkpointer_stat_reset_time() AS stats_reset;
 pg_stat_database| SELECT oid AS datid,
     datname,
-- 
2.43.0

