Improve the checkpoint signaling mechanism so that the bgwriter can tell

the difference between checkpoints forced due to WAL segment consumption
and checkpoints forced for other reasons (such as CREATE DATABASE).  Avoid
generating 'checkpoints are occurring too frequently' messages when the
checkpoint wasn't caused by WAL segment consumption.  Per gripe from
Chris K-L.

Author: tglsfdc

src/backend/access/transam/clog.c

@@ -24,7 +24,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/clog.c,v 1.30 2005/06/06 20:22:57 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/clog.c,v 1.31 2005/06/30 00:00:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -336,7 +336,7 @@ TruncateCLOG(TransactionId oldestXact)
 		return;					/* nothing to remove */
 
 	/* Perform a CHECKPOINT */
-	RequestCheckpoint(true);
+	RequestCheckpoint(true, false);
 
 	/* Now we can remove the old CLOG segment(s) */
 	SimpleLruTruncate(ClogCtl, cutoffPage);

src/backend/access/transam/xlog.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.204 2005/06/29 22:51:53 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.205 2005/06/30 00:00:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1337,7 +1337,7 @@ XLogWrite(XLogwrtRqst WriteRqst)
 						if (XLOG_DEBUG)
 							elog(LOG, "time for a checkpoint, signaling bgwriter");
 #endif
-						RequestCheckpoint(false);
+						RequestCheckpoint(false, true);
 					}
 				}
 			}
@@ -5496,7 +5496,7 @@ pg_start_backup(PG_FUNCTION_ARGS)
 	 * will have different checkpoint positions and hence different
 	 * history file names, even if nothing happened in between.
 	 */
-	RequestCheckpoint(true);
+	RequestCheckpoint(true, false);
 
 	/*
 	 * Now we need to fetch the checkpoint record location, and also its

src/backend/commands/dbcommands.c

@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.163 2005/06/29 20:34:13 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.164 2005/06/30 00:00:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -542,7 +542,7 @@ createdb(const CreatedbStmt *stmt)
 	 * Perhaps if we ever implement CREATE DATABASE in a less cheesy
 	 * way, we can avoid this.
 	 */
-	RequestCheckpoint(true);
+	RequestCheckpoint(true, false);
 
 	/*
 	 * Set flag to update flat database file at commit.
@@ -668,7 +668,7 @@ dropdb(const char *dbname)
 	 * open files, which would cause rmdir() to fail.
 	 */
 #ifdef WIN32
-	RequestCheckpoint(true);
+	RequestCheckpoint(true, false);
 #endif
 
 	/*

src/backend/postmaster/bgwriter.c

@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/postmaster/bgwriter.c,v 1.16 2005/05/28 17:21:32 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/postmaster/bgwriter.c,v 1.17 2005/06/30 00:00:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -86,8 +86,14 @@
  *	6. If ckpt_failed is different from the originally saved value,
  *	   assume request failed; otherwise it was definitely successful.
  *
+ * An additional field is ckpt_time_warn; this is also sig_atomic_t for
+ * simplicity, but is only used as a boolean.  If a backend is requesting
+ * a checkpoint for which a checkpoints-too-close-together warning is
+ * reasonable, it should set this field TRUE just before sending the signal.
+ *
  * The requests array holds fsync requests sent by backends and not yet
- * absorbed by the bgwriter.
+ * absorbed by the bgwriter.  Unlike the checkpoint fields, the requests
+ * fields are protected by BgWriterCommLock.
  *----------
  */
 typedef struct
@@ -105,6 +111,8 @@ typedef struct
 	sig_atomic_t ckpt_done;		/* advances when checkpoint done */
 	sig_atomic_t ckpt_failed;	/* advances when checkpoint fails */
 
+	sig_atomic_t ckpt_time_warn;	/* warn if too soon since last ckpt? */
+
 	int			num_requests;	/* current # of requests */
 	int			max_requests;	/* allocated array size */
 	BgWriterRequest requests[1];	/* VARIABLE LENGTH ARRAY */
@@ -319,20 +327,20 @@ BackgroundWriterMain(void)
 		 */
 		if (do_checkpoint)
 		{
-			if (CheckPointWarning != 0)
-			{
-				/*
-				 * Ideally we should only warn if this checkpoint was
-				 * requested due to running out of segment files, and not
-				 * if it was manually requested.  However we can't tell
-				 * the difference with the current signalling mechanism.
-				 */
-				if (elapsed_secs < CheckPointWarning)
-					ereport(LOG,
-							(errmsg("checkpoints are occurring too frequently (%d seconds apart)",
-									elapsed_secs),
-							 errhint("Consider increasing the configuration parameter \"checkpoint_segments\".")));
-			}
+			/*
+			 * We will warn if (a) too soon since last checkpoint (whatever
+			 * caused it) and (b) somebody has set the ckpt_time_warn flag
+			 * since the last checkpoint start.  Note in particular that
+			 * this implementation will not generate warnings caused by
+			 * CheckPointTimeout < CheckPointWarning.
+			 */
+			if (BgWriterShmem->ckpt_time_warn &&
+				elapsed_secs < CheckPointWarning)
+				ereport(LOG,
+						(errmsg("checkpoints are occurring too frequently (%d seconds apart)",
+								elapsed_secs),
+						 errhint("Consider increasing the configuration parameter \"checkpoint_segments\".")));
+			BgWriterShmem->ckpt_time_warn = false;
 
 			/*
 			 * Indicate checkpoint start to any waiting backends.
@@ -497,9 +505,13 @@ BgWriterShmemInit(void)
  * If waitforit is true, wait until the checkpoint is completed
  * before returning; otherwise, just signal the request and return
  * immediately.
+ *
+ * If warnontime is true, and it's "too soon" since the last checkpoint,
+ * the bgwriter will log a warning.  This should be true only for checkpoints
+ * caused due to xlog filling, else the warning will be misleading.
  */
 void
-RequestCheckpoint(bool waitforit)
+RequestCheckpoint(bool waitforit, bool warnontime)
 {
 	/* use volatile pointer to prevent code rearrangement */
 	volatile BgWriterShmemStruct *bgs = BgWriterShmem;
@@ -523,6 +535,10 @@ RequestCheckpoint(bool waitforit)
 		return;
 	}
 
+	/* Set warning request flag if appropriate */
+	if (warnontime)
+		bgs->ckpt_time_warn = true;
+
 	/*
 	 * Send signal to request checkpoint.  When waitforit is false, we
 	 * consider failure to send the signal to be nonfatal.

src/backend/tcop/utility.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.239 2005/06/28 05:09:00 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.240 2005/06/30 00:00:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -989,7 +989,7 @@ ProcessUtility(Node *parsetree,
 				ereport(ERROR,
 						(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 						 errmsg("must be superuser to do CHECKPOINT")));
-			RequestCheckpoint(true);
+			RequestCheckpoint(true, false);
 			break;
 
 		case T_ReindexStmt:

src/include/postmaster/bgwriter.h

@@ -5,7 +5,7 @@
  *
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/include/postmaster/bgwriter.h,v 1.5 2005/03/04 20:21:06 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/postmaster/bgwriter.h,v 1.6 2005/06/30 00:00:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -23,7 +23,7 @@ extern int	CheckPointWarning;
 
 extern void BackgroundWriterMain(void);
 
-extern void RequestCheckpoint(bool waitforit);
+extern void RequestCheckpoint(bool waitforit, bool warnontime);
 
 extern bool ForwardFsyncRequest(RelFileNode rnode, BlockNumber segno);
 extern void AbsorbFsyncRequests(void);