v3-0001-Introduce-num_done-counter-in-the-pg_stat_checkpoint.patch

text/x-patch

Filename: v3-0001-Introduce-num_done-counter-in-the-pg_stat_checkpoint.patch
Type: text/x-patch
Part: 0
Message: Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.

Patch

Format: format-patch
Series: patch v3-0001
Subject: Introduce num_done counter in the pg_stat_checkpointer view that reflects number of really performed checkpoints.
File+
doc/src/sgml/monitoring.sgml 10 1
src/backend/access/transam/xlog.c 7 2
src/backend/catalog/system_views.sql 1 0
src/backend/postmaster/checkpointer.c 29 16
src/backend/utils/activity/pgstat_checkpointer.c 2 0
src/backend/utils/adt/pgstatfuncs.c 6 0
src/include/access/xlog.h 1 1
src/include/catalog/pg_proc.dat 16 10
src/include/pgstat.h 1 0
src/test/regress/expected/rules.out 1 0
From 5832b1beb453b96a6ccbe72c404f2ad5373d0497 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <a.melnikov@postgrespro.ru>
Date: Thu, 19 Sep 2024 12:51:17 +0300
Subject: [PATCH] Introduce num_done counter in the pg_stat_checkpointer view
 that reflects number of really performed checkpoints.

---
 doc/src/sgml/monitoring.sgml                  | 11 ++++-
 src/backend/access/transam/xlog.c             |  9 +++-
 src/backend/catalog/system_views.sql          |  1 +
 src/backend/postmaster/checkpointer.c         | 45 ++++++++++++-------
 .../utils/activity/pgstat_checkpointer.c      |  2 +
 src/backend/utils/adt/pgstatfuncs.c           |  6 +++
 src/include/access/xlog.h                     |  2 +-
 src/include/catalog/pg_proc.dat               | 26 ++++++-----
 src/include/pgstat.h                          |  1 +
 src/test/regress/expected/rules.out           |  1 +
 10 files changed, 74 insertions(+), 30 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a2fda4677d7..19bf0164f1c 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3063,7 +3063,16 @@ description | Waiting for a newly initialized WAL file to reach durable storage
        <structfield>num_requested</structfield> <type>bigint</type>
       </para>
       <para>
-       Number of requested checkpoints that have been performed
+       Number of backend requested checkpoints
+      </para></entry>
+     </row>
+
+      <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>num_done</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of checkpoints that have been performed
       </para></entry>
      </row>
 
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 853ab06812b..c9d37cba453 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6878,8 +6878,11 @@ update_checkpoint_display(int flags, bool restartpoint, bool reset)
  * In this case, we only insert an XLOG_CHECKPOINT_SHUTDOWN record, and it's
  * both the record marking the completion of the checkpoint and the location
  * from which WAL replay would begin if needed.
+ *
+ * Returns true if a new checkpoint was performed or false if checkpoint
+ * was skipped because system is idle.
  */
-void
+bool
 CreateCheckPoint(int flags)
 {
 	bool		shutdown;
@@ -6971,7 +6974,7 @@ CreateCheckPoint(int flags)
 			END_CRIT_SECTION();
 			ereport(DEBUG1,
 					(errmsg_internal("checkpoint skipped because system is idle")));
-			return;
+			return false;
 		}
 	}
 
@@ -7353,6 +7356,8 @@ CreateCheckPoint(int flags)
 									 CheckpointStats.ckpt_segs_added,
 									 CheckpointStats.ckpt_segs_removed,
 									 CheckpointStats.ckpt_segs_recycled);
+
+	return true;
 }
 
 /*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7fd5d256a18..49109dbdc86 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1138,6 +1138,7 @@ CREATE VIEW pg_stat_checkpointer AS
     SELECT
         pg_stat_get_checkpointer_num_timed() AS num_timed,
         pg_stat_get_checkpointer_num_requested() AS num_requested,
+        pg_stat_get_checkpointer_num_performed() AS num_done,
         pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
         pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
         pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index eeb73c85726..8d610a3f1a7 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -461,8 +461,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
 			 */
 			if (!do_restartpoint)
 			{
-				CreateCheckPoint(flags);
-				ckpt_performed = true;
+				ckpt_performed = CreateCheckPoint(flags);
 			}
 			else
 				ckpt_performed = CreateRestartPoint(flags);
@@ -484,27 +483,41 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
 
 			ConditionVariableBroadcast(&CheckpointerShmem->done_cv);
 
-			if (ckpt_performed)
+			if (!do_restartpoint)
 			{
 				/*
-				 * Note we record the checkpoint start time not end time as
-				 * last_checkpoint_time.  This is so that time-driven
-				 * checkpoints happen at a predictable spacing.
-				 */
+				* Note we record the checkpoint start time not end time as
+				* last_checkpoint_time.  This is so that time-driven
+				* checkpoints happen at a predictable spacing.
+				*/
 				last_checkpoint_time = now;
 
-				if (do_restartpoint)
-					PendingCheckpointerStats.restartpoints_performed++;
+				if(ckpt_performed)
+					PendingCheckpointerStats.num_performed++;
+
 			}
 			else
 			{
-				/*
-				 * We were not able to perform the restartpoint (checkpoints
-				 * throw an ERROR in case of error).  Most likely because we
-				 * have not received any new checkpoint WAL records since the
-				 * last restartpoint. Try again in 15 s.
-				 */
-				last_checkpoint_time = now - CheckPointTimeout + 15;
+				if (ckpt_performed)
+				{
+					/*
+					 * The same as for checkpoint.
+					 * Please see the corresponding comment.
+					 */
+					last_checkpoint_time = now;
+
+					PendingCheckpointerStats.restartpoints_performed++;
+				}
+				else
+				{
+					/*
+					* We were not able to perform the restartpoint (checkpoints
+					* throw an ERROR in case of error).  Most likely because we
+					* have not received any new checkpoint WAL records since the
+					* last restartpoint. Try again in 15 s.
+					*/
+					last_checkpoint_time = now - CheckPointTimeout + 15;
+				}
 			}
 
 			ckpt_active = false;
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index bbfc9c7e183..4a0a2d1493a 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -49,6 +49,7 @@ pgstat_report_checkpointer(void)
 #define CHECKPOINTER_ACC(fld) stats_shmem->stats.fld += PendingCheckpointerStats.fld
 	CHECKPOINTER_ACC(num_timed);
 	CHECKPOINTER_ACC(num_requested);
+	CHECKPOINTER_ACC(num_performed);
 	CHECKPOINTER_ACC(restartpoints_timed);
 	CHECKPOINTER_ACC(restartpoints_requested);
 	CHECKPOINTER_ACC(restartpoints_performed);
@@ -127,6 +128,7 @@ pgstat_checkpointer_snapshot_cb(void)
 #define CHECKPOINTER_COMP(fld) pgStatLocal.snapshot.checkpointer.fld -= reset.fld;
 	CHECKPOINTER_COMP(num_timed);
 	CHECKPOINTER_COMP(num_requested);
+	CHECKPOINTER_COMP(num_performed);
 	CHECKPOINTER_COMP(restartpoints_timed);
 	CHECKPOINTER_COMP(restartpoints_requested);
 	CHECKPOINTER_COMP(restartpoints_performed);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9c23ac7c8c8..17b0fc02ef0 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1191,6 +1191,12 @@ pg_stat_get_checkpointer_num_requested(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_requested);
 }
 
+Datum
+pg_stat_get_checkpointer_num_performed(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_performed);
+}
+
 Datum
 pg_stat_get_checkpointer_restartpoints_timed(PG_FUNCTION_ARGS)
 {
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 083810f5b4c..36f6e4e4b4e 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -239,7 +239,7 @@ extern void LocalProcessControlFile(bool reset);
 extern WalLevel GetActiveWalLevelOnStandby(void);
 extern void StartupXLOG(void);
 extern void ShutdownXLOG(int code, Datum arg);
-extern void CreateCheckPoint(int flags);
+extern bool CreateCheckPoint(int flags);
 extern bool CreateRestartPoint(int flags);
 extern WALAvailability GetWALAvailability(XLogRecPtr targetLSN);
 extern void XLogPutNextOid(Oid nextOid);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 43f608d7a0a..baa6120e664 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5817,6 +5817,12 @@
   proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
   proparallel => 'r', prorettype => 'int8', proargtypes => '',
   prosrc => 'pg_stat_get_checkpointer_num_requested' },
+{ oid => '2775',
+  descr => 'statistics: number of checkpoints performed by the checkpointer',
+  proname => 'pg_stat_get_checkpointer_num_performed',
+  provolatile => 's', proparallel => 'r', prorettype => 'int8',
+  proargtypes => '',
+  prosrc => 'pg_stat_get_checkpointer_num_performed' },
 { oid => '6327',
   descr => 'statistics: number of timed restartpoints started by the checkpointer',
   proname => 'pg_stat_get_checkpointer_restartpoints_timed', provolatile => 's',
@@ -5843,6 +5849,16 @@
   proname => 'pg_stat_get_checkpointer_stat_reset_time', provolatile => 's',
   proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
   prosrc => 'pg_stat_get_checkpointer_stat_reset_time' },
+  { oid => '3160',
+  descr => 'statistics: checkpoint/restartpoint time spent writing buffers to disk, in milliseconds',
+  proname => 'pg_stat_get_checkpointer_write_time', provolatile => 's',
+  proparallel => 'r', prorettype => 'float8', proargtypes => '',
+  prosrc => 'pg_stat_get_checkpointer_write_time' },
+{ oid => '3161',
+  descr => 'statistics: checkpoint/restartpoint time spent synchronizing buffers to disk, in milliseconds',
+  proname => 'pg_stat_get_checkpointer_sync_time', provolatile => 's',
+  proparallel => 'r', prorettype => 'float8', proargtypes => '',
+  prosrc => 'pg_stat_get_checkpointer_sync_time' },
 { oid => '2772',
   descr => 'statistics: number of buffers written by the bgwriter for cleaning dirty buffers',
   proname => 'pg_stat_get_bgwriter_buf_written_clean', provolatile => 's',
@@ -5857,16 +5873,6 @@
   proname => 'pg_stat_get_bgwriter_stat_reset_time', provolatile => 's',
   proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
   prosrc => 'pg_stat_get_bgwriter_stat_reset_time' },
-{ oid => '3160',
-  descr => 'statistics: checkpoint/restartpoint time spent writing buffers to disk, in milliseconds',
-  proname => 'pg_stat_get_checkpointer_write_time', provolatile => 's',
-  proparallel => 'r', prorettype => 'float8', proargtypes => '',
-  prosrc => 'pg_stat_get_checkpointer_write_time' },
-{ oid => '3161',
-  descr => 'statistics: checkpoint/restartpoint time spent synchronizing buffers to disk, in milliseconds',
-  proname => 'pg_stat_get_checkpointer_sync_time', provolatile => 's',
-  proparallel => 'r', prorettype => 'float8', proargtypes => '',
-  prosrc => 'pg_stat_get_checkpointer_sync_time' },
 { oid => '2859', descr => 'statistics: number of buffer allocations',
   proname => 'pg_stat_get_buf_alloc', provolatile => 's', proparallel => 'r',
   prorettype => 'int8', proargtypes => '', prosrc => 'pg_stat_get_buf_alloc' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 4752dfe7197..476acd680c0 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -294,6 +294,7 @@ typedef struct PgStat_CheckpointerStats
 {
 	PgStat_Counter num_timed;
 	PgStat_Counter num_requested;
+	PgStat_Counter num_performed;
 	PgStat_Counter restartpoints_timed;
 	PgStat_Counter restartpoints_requested;
 	PgStat_Counter restartpoints_performed;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index a1626f3fae9..f5434d8365c 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1824,6 +1824,7 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_buf_written_clean() AS buffers_cle
     pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
 pg_stat_checkpointer| SELECT pg_stat_get_checkpointer_num_timed() AS num_timed,
     pg_stat_get_checkpointer_num_requested() AS num_requested,
+    pg_stat_get_checkpointer_num_performed() AS num_done,
     pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
     pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
     pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
-- 
2.46.0