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.

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 2da835dbbe5adb178710515d576fb7d11d5203a8..3768570ad3d5c6ca31d5ed4cd4314cacdcf755bd 100644 (file)
--- a/src/backend/access/transam/clog.c
+++ b/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);

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 93f8d75e6cc05319153aaa0e4dcd9045f7343cff..4c2f6a69a709e1d12e5ccf5b06213068d1155ee5 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/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

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 96f964fb95fc75cf17d70cb532d0ef5d752371b0..ba5ce9200b45ea52590470129594b3c8fcff4c9b 100644 (file)
--- a/src/backend/commands/dbcommands.c
+++ b/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
 
        /*

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 0cd694ef52989c6b1ac1b6ddbb51e35ba967bfc3..d826a6190de11ae2bee605d23c91ffaa1befac01 100644 (file)
--- a/src/backend/postmaster/bgwriter.c
+++ b/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.

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 40d5e7d7ce6cf9b20b2ca889203c0dc544b25924..becd2793384b26f8694744587832c8b5885c0c5f 100644 (file)
--- a/src/backend/tcop/utility.c
+++ b/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:

diff --git a/src/include/postmaster/bgwriter.h b/src/include/postmaster/bgwriter.h
index dc1260de182c1fa8a7a8273328e9e105cf2a06f8..3cdfef1750fd296ca217daa12c0dce6ae1cc4224 100644 (file)
--- a/src/include/postmaster/bgwriter.h
+++ b/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);