public inbox for [email protected]  
help / color / mirror / Atom feed
From: Anton A. Melnikov <[email protected]>
To: Alexander Korotkov <[email protected]>
To: Anton A. Melnikov <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Subject: Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
Date: Mon, 4 Dec 2023 04:49:59 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAPpHfdtZvVRpQErb_KpbrieROPpo5o2wemAp-jJOVdww2AdQqg@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAPpHfdtZvVRpQErb_KpbrieROPpo5o2wemAp-jJOVdww2AdQqg@mail.gmail.com>

Thanks for remarks!

On 28.11.2023 21:34, Alexander Korotkov wrote:
> After examining the second patch
> ("v2-0001-Add-restartpoint-stats.patch"), it appears that adding
> additional statistics as outlined in the patch is the most suitable
> approach to address the concerns raised. This solution provides more
> visibility into the system's behavior without altering its core
> mechanics.

Agreed. I left only this variant of the patch and rework it due to commit 96f05261.
So the new counters is in the pg_stat_checkpointer view now.
Please see the v3-0001-add-restartpoints-stats.patch attached.


> However, it's essential that this additional functionality
> is accompanied by comprehensive documentation to ensure clear
> understanding and ease of use by the PostgreSQL community.
> 
> Please consider expanding the documentation to include detailed
> explanations of the new statistics and their implications in various
> scenarios.

In the separate v3-0002-doc-for-restartpoints-stats.patch i added the definitions
of the new counters into the "28.2.15. pg_stat_checkpointer" section
and explanation of them with examples into the "30.5.WAL Configuration" one.

Would be glad for any comments and and concerns.

With the best wishes,

-- 
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

  [text/x-patch] v3-0001-add-restartpoints-stats.patch (7.8K, ../[email protected]/2-v3-0001-add-restartpoints-stats.patch)
  download | inline diff:
commit 5711c09dbfbe02586a247a98e0ae41cd71a221a3
Author: Anton A. Melnikov <[email protected]>
Date:   Sun Dec 3 12:49:11 2023 +0300

    Add statistic about restartpoints into pg_stat_checkpointer

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 11d18ed9dd..058fc47c91 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1141,6 +1141,9 @@ 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_restartpoints_timed() AS restartpoints_timed,
+        pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
+        pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
         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,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index dc2da5a2cd..67ecb177e7 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -340,6 +340,8 @@ CheckpointerMain(void)
 		pg_time_t	now;
 		int			elapsed_secs;
 		int			cur_timeout;
+		bool		chkpt_or_rstpt_requested = false;
+		bool		chkpt_or_rstpt_timed = false;
 
 		/* Clear any already-pending wakeups */
 		ResetLatch(MyLatch);
@@ -358,7 +360,7 @@ CheckpointerMain(void)
 		if (((volatile CheckpointerShmemStruct *) CheckpointerShmem)->ckpt_flags)
 		{
 			do_checkpoint = true;
-			PendingCheckpointerStats.num_requested++;
+			chkpt_or_rstpt_requested = true;
 		}
 
 		/*
@@ -372,7 +374,7 @@ CheckpointerMain(void)
 		if (elapsed_secs >= CheckPointTimeout)
 		{
 			if (!do_checkpoint)
-				PendingCheckpointerStats.num_timed++;
+				chkpt_or_rstpt_timed = true;
 			do_checkpoint = true;
 			flags |= CHECKPOINT_CAUSE_TIME;
 		}
@@ -408,6 +410,24 @@ CheckpointerMain(void)
 			if (flags & CHECKPOINT_END_OF_RECOVERY)
 				do_restartpoint = false;
 
+			if (chkpt_or_rstpt_timed)
+			{
+				chkpt_or_rstpt_timed = false;
+				if (do_restartpoint)
+					PendingCheckpointerStats.restartpoints_timed++;
+				else
+					PendingCheckpointerStats.num_timed++;
+			}
+
+			if (chkpt_or_rstpt_requested)
+			{
+				chkpt_or_rstpt_requested = false;
+				if (do_restartpoint)
+					PendingCheckpointerStats.restartpoints_requested++;
+				else
+					PendingCheckpointerStats.num_requested++;
+			}
+
 			/*
 			 * We will warn if (a) too soon since last checkpoint (whatever
 			 * caused it) and (b) somebody set the CHECKPOINT_CAUSE_XLOG flag
@@ -471,6 +491,9 @@ CheckpointerMain(void)
 				 * checkpoints happen at a predictable spacing.
 				 */
 				last_checkpoint_time = now;
+
+				if (do_restartpoint)
+					PendingCheckpointerStats.restartpoints_performed++;
 			}
 			else
 			{
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index 301a0bc7bd..6ee258f240 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -49,6 +49,9 @@ pgstat_report_checkpointer(void)
 #define CHECKPOINTER_ACC(fld) stats_shmem->stats.fld += PendingCheckpointerStats.fld
 	CHECKPOINTER_ACC(num_timed);
 	CHECKPOINTER_ACC(num_requested);
+	CHECKPOINTER_ACC(restartpoints_timed);
+	CHECKPOINTER_ACC(restartpoints_requested);
+	CHECKPOINTER_ACC(restartpoints_performed);
 	CHECKPOINTER_ACC(write_time);
 	CHECKPOINTER_ACC(sync_time);
 	CHECKPOINTER_ACC(buffers_written);
@@ -116,6 +119,9 @@ 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(restartpoints_timed);
+	CHECKPOINTER_COMP(restartpoints_requested);
+	CHECKPOINTER_COMP(restartpoints_performed);
 	CHECKPOINTER_COMP(write_time);
 	CHECKPOINTER_COMP(sync_time);
 	CHECKPOINTER_COMP(buffers_written);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 0cea320c00..e65cbf41e9 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1193,6 +1193,24 @@ pg_stat_get_checkpointer_num_requested(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_requested);
 }
 
+Datum
+pg_stat_get_checkpointer_restartpoints_timed(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->restartpoints_timed);
+}
+
+Datum
+pg_stat_get_checkpointer_restartpoints_requested(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->restartpoints_requested);
+}
+
+Datum
+pg_stat_get_checkpointer_restartpoints_performed(PG_FUNCTION_ARGS)
+{
+	PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->restartpoints_performed);
+}
+
 Datum
 pg_stat_get_checkpointer_buffers_written(PG_FUNCTION_ARGS)
 {
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index fb58dee3bc..f6926c864d 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5721,6 +5721,21 @@
   proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
   proparallel => 'r', prorettype => 'int8', proargtypes => '',
   prosrc => 'pg_stat_get_checkpointer_num_requested' },
+{ oid => '8743',
+  descr => 'statistics: number of timed restartpoints started by the checkpointer',
+  proname => 'pg_stat_get_checkpointer_restartpoints_timed', provolatile => 's',
+  proparallel => 'r', prorettype => 'int8', proargtypes => '',
+  prosrc => 'pg_stat_get_checkpointer_restartpoints_timed' },
+{ oid => '8744',
+  descr => 'statistics: number of backend requested restartpoints started by the checkpointer',
+  proname => 'pg_stat_get_checkpointer_restartpoints_requested', provolatile => 's',
+  proparallel => 'r', prorettype => 'int8', proargtypes => '',
+  prosrc => 'pg_stat_get_checkpointer_restartpoints_requested' },
+{ oid => '8745',
+  descr => 'statistics: number of backend performed restartpoints',
+  proname => 'pg_stat_get_checkpointer_restartpoints_performed', provolatile => 's',
+  proparallel => 'r', prorettype => 'int8', proargtypes => '',
+  prosrc => 'pg_stat_get_checkpointer_restartpoints_performed' },
 { oid => '2771',
   descr => 'statistics: number of buffers written by the checkpointer',
   proname => 'pg_stat_get_checkpointer_buffers_written', provolatile => 's',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index f95d8db0c4..3cc2762ef5 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -262,6 +262,9 @@ typedef struct PgStat_CheckpointerStats
 {
 	PgStat_Counter num_timed;
 	PgStat_Counter num_requested;
+	PgStat_Counter restartpoints_timed;
+	PgStat_Counter restartpoints_requested;
+	PgStat_Counter restartpoints_performed;
 	PgStat_Counter write_time;	/* times in milliseconds */
 	PgStat_Counter sync_time;
 	PgStat_Counter buffers_written;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 05070393b9..f645e8486b 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1822,6 +1822,9 @@ 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_restartpoints_timed() AS restartpoints_timed,
+    pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
+    pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
     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,


  [text/x-patch] v3-0002-doc-for-restartpoints-stats.patch (4.5K, ../[email protected]/3-v3-0002-doc-for-restartpoints-stats.patch)
  download | inline diff:
commit 4acf7f57f1476611b70f027b4fddd7cc276204af
Author: Anton A. Melnikov <[email protected]>
Date:   Mon Dec 4 04:19:47 2023 +0300

    Add docs about restartpoints related counters in the pg_stat_checkpointer view.

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 42509042ad..45cc091ea7 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -2982,6 +2982,33 @@ 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>restartpoints_timed</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of scheduled restartpoints due to timeout or after failed attempt to perform it
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>restartpoints_req</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of requested restartpoints
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>restartpoints_done</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of restartpoints that have been performed
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>write_time</structfield> <type>double precision</type>
diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml
index 2ed4eb659d..678b0489d3 100644
--- a/doc/src/sgml/wal.sgml
+++ b/doc/src/sgml/wal.sgml
@@ -655,14 +655,41 @@
    directory.
    Restartpoints can't be performed more frequently than checkpoints on the
    primary because restartpoints can only be performed at checkpoint records.
-   A restartpoint is triggered when a checkpoint record is reached if at
-   least <varname>checkpoint_timeout</varname> seconds have passed since the last
-   restartpoint, or if WAL size is about to exceed
-   <varname>max_wal_size</varname>. However, because of limitations on when a
-   restartpoint can be performed, <varname>max_wal_size</varname> is often exceeded
-   during recovery, by up to one checkpoint cycle's worth of WAL.
+   A restartpoint can be demanded by a shedule or by an external request.
+   The <structfield>restartpoints_timed</structfield> counter in the
+   <link linkend="monitoring-pg-stat-checkpointer-view"><structname>pg_stat_checkpointer</structname></link>
+   view counts the first ones while the <structfield>restartpoints_req</structfield>
+   the second.
+   A restartpoint is triggered by shedule when a checkpoint record is reached
+   if at least <xref linkend="guc-checkpoint-timeout"/> seconds have passed since
+   the last performed restartpoint or when the previous attempt to perform
+   the restartpoint have been failed. In the last case the next restartpoint
+   will be scheduled in 15s.
+   A restartpoint is triggered by request due to similar reasons like checkpoint
+   but mostly if WAL size is about to exceed <xref linkend="guc-max-wal-size"/>
+   However, because of limitations on when a restartpoint can be performed,
+   <varname>max_wal_size</varname> is often exceeded during recovery,
+   by up to one checkpoint cycle's worth of WAL.
    (<varname>max_wal_size</varname> is never a hard limit anyway, so you should
    always leave plenty of headroom to avoid running out of disk space.)
+   The <structfield>restartpoints_done</structfield> counter in the
+   <link linkend="monitoring-pg-stat-checkpointer-view"><structname>pg_stat_checkpointer</structname></link>
+   view counts the restartpoints that have really been performed.
+  </para>
+
+  <para>
+   In some cases, when the WAL size on the primary increases quickly,
+   for instance during massive INSERT,
+   the <structfield>restartpoints_req</structfield> counter on the standby
+   may demonstarte a spike growth.
+   This occurs since requests to create a new restartpoint due to increased
+   XLOG consumption cannot be performed because the safe checkpoint record
+   since last restartpoint has not yet been replayed on the standby.
+   This behavior is normal does not lead to increase in system resources
+   consumption.
+   Only the <structfield>restartpoints_done</structfield>
+   counter among the restartpoint related ones indicates that noticable system
+   resources have been spent.
   </para>
 
   <para>


view thread (13+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox