Greetings,

* Laurenz Albe (laurenz.a...@cybertec.at) wrote:
> On Tue, 2020-12-08 at 17:29 +0000, Bossart, Nathan wrote:
> > +1 to setting checkpoint_completion_target to 0.9 by default.
> 
> +1 for changing the default or getting rid of it, as Tom suggested.

Attached is a patch to change it from a GUC to a compile-time #define
which is set to 0.9, with accompanying documentation updates.

> While we are at it, could we change the default of "log_lock_waits" to "on"?

While I agree that it'd be good to change quite a few of the log_X items
to be 'on' by default, I'm not planning to work on this.

Thanks,

Stephen
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 42a8ed328d..ed2b27164b 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -857,10 +857,9 @@ SELECT pg_start_backup('label', false, false);
     <para>
      By default, <function>pg_start_backup</function> can take a long time to finish.
      This is because it performs a checkpoint, and the I/O
-     required for the checkpoint will be spread out over a significant
-     period of time, by default half your inter-checkpoint interval
-     (see the configuration parameter
-     <xref linkend="guc-checkpoint-completion-target"/>).  This is
+     required for the checkpoint will be spread out over the inter-checkpoint
+     interval (see the configuration parameter
+     <xref linkend="guc-checkpoint-timeout"/>).  This is
      usually what you want, because it minimizes the impact on query
      processing.  If you want to start the backup as soon as
      possible, change the second parameter to <literal>true</literal>, which will
@@ -1000,10 +999,9 @@ SELECT pg_start_backup('label');
     <para>
      By default, <function>pg_start_backup</function> can take a long time to finish.
      This is because it performs a checkpoint, and the I/O
-     required for the checkpoint will be spread out over a significant
-     period of time, by default half your inter-checkpoint interval
-     (see the configuration parameter
-     <xref linkend="guc-checkpoint-completion-target"/>).  This is
+     required for the checkpoint will be spread out over the inter-checkpoint
+     interval (see the configuration parameter
+     <xref linkend="guc-checkpoint-timeout"/>).  This is
      usually what you want, because it minimizes the impact on query
      processing.  If you want to start the backup as soon as
      possible, use:
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8cd3d6901c..39f9701959 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3251,22 +3251,6 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-checkpoint-completion-target" xreflabel="checkpoint_completion_target">
-      <term><varname>checkpoint_completion_target</varname> (<type>floating point</type>)
-      <indexterm>
-       <primary><varname>checkpoint_completion_target</varname> configuration parameter</primary>
-      </indexterm>
-      </term>
-      <listitem>
-       <para>
-        Specifies the target of checkpoint completion, as a fraction of
-        total time between checkpoints. The default is 0.5.
-        This parameter can only be set in the <filename>postgresql.conf</filename>
-        file or on the server command line.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-checkpoint-flush-after" xreflabel="checkpoint_flush_after">
       <term><varname>checkpoint_flush_after</varname> (<type>integer</type>)
       <indexterm>
diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml
index d1c3893b14..735d0c0661 100644
--- a/doc/src/sgml/wal.sgml
+++ b/doc/src/sgml/wal.sgml
@@ -521,28 +521,19 @@
 
   <para>
    To avoid flooding the I/O system with a burst of page writes,
-   writing dirty buffers during a checkpoint is spread over a period of time.
-   That period is controlled by
-   <xref linkend="guc-checkpoint-completion-target"/>, which is
-   given as a fraction of the checkpoint interval.
-   The I/O rate is adjusted so that the checkpoint finishes when the
-   given fraction of
-   <varname>checkpoint_timeout</varname> seconds have elapsed, or before
+   writing dirty buffers during a checkpoint is spread out across the time between
+   when checkpoints are scheduled to begin, as configured by 
+   <xref linkend="guc-checkpoint-timeout"/>.
+   The I/O rate is adjusted so that the checkpoint finishes at approximately the
+   time when the next checkpoint is scheduled to begin, or before 
    <varname>max_wal_size</varname> is exceeded, whichever is sooner.
-   With the default value of 0.5,
-   <productname>PostgreSQL</productname> can be expected to complete each checkpoint
-   in about half the time before the next checkpoint starts.  On a system
-   that's very close to maximum I/O throughput during normal operation,
-   you might want to increase <varname>checkpoint_completion_target</varname>
-   to reduce the I/O load from checkpoints.  The disadvantage of this is that
-   prolonging checkpoints affects recovery time, because more WAL segments
-   will need to be kept around for possible use in recovery.  Although
-   <varname>checkpoint_completion_target</varname> can be set as high as 1.0,
-   it is best to keep it less than that (perhaps 0.9 at most) since
-   checkpoints include some other activities besides writing dirty buffers.
-   A setting of 1.0 is quite likely to result in checkpoints not being
-   completed on time, which would result in performance loss due to
-   unexpected variation in the number of WAL segments needed.
+   This spreads out the I/O as much as possible to have the I/O load be consistent
+   during the checkpoint and generally throughout the operation of the system.  The
+   disadvantage of this is that prolonging checkpoints affects recovery time,
+   because more WAL segments will need to be kept around for possible use in recovery.
+   A user concerned about the amount of time required to recover might wish to reduce
+   <varname>checkpoint_timeout</varname>, causing checkpoints to happen more
+   frequently.
   </para>
 
   <para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7e81ce4f17..f027cfe171 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2289,7 +2289,7 @@ AdvanceXLInsertBuffer(XLogRecPtr upto, bool opportunistic)
 
 /*
  * Calculate CheckPointSegments based on max_wal_size_mb and
- * checkpoint_completion_target.
+ * CheckPointCompletionTarget.
  */
 static void
 CalculateCheckpointSegments(void)
@@ -2306,7 +2306,7 @@ CalculateCheckpointSegments(void)
 	 *    only did this on the primary anyway, not on standby. Keeping just
 	 *    one checkpoint simplifies processing and reduces disk space in
 	 *    many smaller databases.)
-	 * b) during checkpoint, we consume checkpoint_completion_target *
+	 * b) during checkpoint, we consume CheckPointCompletionTarget *
 	 *	  number of segments consumed between checkpoints.
 	 *-------
 	 */
@@ -2327,13 +2327,6 @@ assign_max_wal_size(int newval, void *extra)
 	CalculateCheckpointSegments();
 }
 
-void
-assign_checkpoint_completion_target(double newval, void *extra)
-{
-	CheckPointCompletionTarget = newval;
-	CalculateCheckpointSegments();
-}
-
 /*
  * At a checkpoint, how many WAL segments to recycle as preallocated future
  * XLOG segments? Returns the highest segment that should be preallocated.
@@ -8694,7 +8687,7 @@ UpdateCheckPointDistanceEstimate(uint64 nbytes)
  *	CHECKPOINT_IS_SHUTDOWN: checkpoint is for database shutdown.
  *	CHECKPOINT_END_OF_RECOVERY: checkpoint is for end of WAL recovery.
  *	CHECKPOINT_IMMEDIATE: finish the checkpoint ASAP,
- *		ignoring checkpoint_completion_target parameter.
+ *		ignoring the CheckPointCompletionTarget.
  *	CHECKPOINT_FORCE: force a checkpoint even if no XLOG activity has occurred
  *		since the last one (implied by CHECKPOINT_IS_SHUTDOWN or
  *		CHECKPOINT_END_OF_RECOVERY).
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 429c8010ef..4f6e843146 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -145,7 +145,6 @@ static CheckpointerShmemStruct *CheckpointerShmem;
  */
 int			CheckPointTimeout = 300;
 int			CheckPointWarning = 30;
-double		CheckPointCompletionTarget = 0.5;
 
 /*
  * Private state
@@ -671,7 +670,7 @@ ImmediateCheckpointRequested(void)
  *
  * This function is called after each page write performed by BufferSync().
  * It is responsible for throttling BufferSync()'s write rate to hit
- * checkpoint_completion_target.
+ * CheckPointCompletionTarget.
  *
  * The checkpoint request flags should be passed in; currently the only one
  * examined is CHECKPOINT_IMMEDIATE, which disables delays between writes.
@@ -757,7 +756,7 @@ IsCheckpointOnSchedule(double progress)
 
 	Assert(ckpt_active);
 
-	/* Scale progress according to checkpoint_completion_target. */
+	/* Scale progress according to CheckPointCompletionTarget. */
 	progress *= CheckPointCompletionTarget;
 
 	/*
@@ -786,7 +785,7 @@ IsCheckpointOnSchedule(double progress)
 	 * be a large gap between a checkpoint's redo-pointer and the checkpoint
 	 * record itself, and we only start the restartpoint after we've seen the
 	 * checkpoint record. (The gap is typically up to CheckPointSegments *
-	 * checkpoint_completion_target where checkpoint_completion_target is the
+	 * CheckPointCompletionTarget where CheckPointCompletionTarget is the
 	 * value that was in effect when the WAL was generated).
 	 */
 	if (RecoveryInProgress())
@@ -903,7 +902,7 @@ CheckpointerShmemInit(void)
  *	CHECKPOINT_IS_SHUTDOWN: checkpoint is for database shutdown.
  *	CHECKPOINT_END_OF_RECOVERY: checkpoint is for end of WAL recovery.
  *	CHECKPOINT_IMMEDIATE: finish the checkpoint ASAP,
- *		ignoring checkpoint_completion_target parameter.
+ *		ignoring the CheckPointCompletionTarget.
  *	CHECKPOINT_FORCE: force a checkpoint even if no XLOG activity has occurred
  *		since the last one (implied by CHECKPOINT_IS_SHUTDOWN or
  *		CHECKPOINT_END_OF_RECOVERY).
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 635d91d50a..e501e525eb 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3637,16 +3637,6 @@ static struct config_real ConfigureNamesReal[] =
 		NULL, NULL, NULL
 	},
 
-	{
-		{"checkpoint_completion_target", PGC_SIGHUP, WAL_CHECKPOINTS,
-			gettext_noop("Time spent flushing dirty buffers during checkpoint, as fraction of checkpoint interval."),
-			NULL
-		},
-		&CheckPointCompletionTarget,
-		0.5, 0.0, 1.0,
-		NULL, NULL, NULL
-	},
-
 	{
 		{"vacuum_cleanup_index_scale_factor", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Number of tuple inserts prior to index cleanup as a fraction of reltuples."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 9c9091e601..39049c0e48 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -230,7 +230,6 @@
 #checkpoint_timeout = 5min		# range 30s-1d
 #max_wal_size = 1GB
 #min_wal_size = 80MB
-#checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
 #checkpoint_flush_after = 0		# measured in pages, 0 disables
 #checkpoint_warning = 30s		# 0 disables
 
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 221af87e71..e7d93c27bd 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -350,7 +350,6 @@ extern void StartupRequestWalReceiverRestart(void);
 extern void XLogRequestWalReceiverReply(void);
 
 extern void assign_max_wal_size(int newval, void *extra);
-extern void assign_checkpoint_completion_target(double newval, void *extra);
 
 /*
  * Routines to start, stop, and get status of a base backup.
diff --git a/src/include/postmaster/bgwriter.h b/src/include/postmaster/bgwriter.h
index 0a5708b32e..5e13c3bd2a 100644
--- a/src/include/postmaster/bgwriter.h
+++ b/src/include/postmaster/bgwriter.h
@@ -20,12 +20,31 @@
 #include "storage/smgr.h"
 #include "storage/sync.h"
 
+/*
+ * CheckPointCompletionTarget is the percentage of time between when
+ * checkpoints are scheduled to start that we wish to spend writing
+ * out dirty buffers.  Our goal is to spread the I/O out over as much of the
+ * checkpoint interval as possible, while not finishing the checkpoint late,
+ * so that the amount of I/O is consistent.
+ *
+ * It might be tempting to set this to '1.0', but there are a few other
+ * things that happen during a checkpoint and we don't want to mistakenly
+ * end up not finishing the checkpoint on time as that could lead to
+ * unexpected variation in the number of WAL segments needed and reduced
+ * performance.
+ *
+ * CheckPointCompletionTarget used to be exposed as a GUC named
+ * checkpoint_completion_target, but there's little evidence to suggest that
+ * there's actually a case for it being a different value, so it's no longer
+ * exposed as a GUC to be configured.
+ */
+
+#define CheckPointCompletionTarget	0.9
 
 /* GUC options */
 extern int	BgWriterDelay;
 extern int	CheckPointTimeout;
 extern int	CheckPointWarning;
-extern double CheckPointCompletionTarget;
 
 extern void BackgroundWriterMain(void) pg_attribute_noreturn();
 extern void CheckpointerMain(void) pg_attribute_noreturn();
diff --git a/src/test/recovery/t/015_promotion_pages.pl b/src/test/recovery/t/015_promotion_pages.pl
index 6fb70b5001..25a9e4764a 100644
--- a/src/test/recovery/t/015_promotion_pages.pl
+++ b/src/test/recovery/t/015_promotion_pages.pl
@@ -26,7 +26,6 @@ my $bravo = get_new_node('bravo');
 $bravo->init_from_backup($alpha, 'bkp', has_streaming => 1);
 $bravo->append_conf('postgresql.conf', <<EOF);
 checkpoint_timeout=1h
-checkpoint_completion_target=0.9
 EOF
 $bravo->start;
 

Attachment: signature.asc
Description: PGP signature

Reply via email to