Flush unlogged table's buffers when copying or moving databases.

CREATE DATABASE and ALTER DATABASE .. SET TABLESPACE copy the source
database directory on the filesystem level. To ensure the on disk
state is consistent they block out users of the affected database and
force a checkpoint to flush out all data to disk. Unfortunately, up to
now, that checkpoint didn't flush out dirty buffers from unlogged
relations.

That bug means there could be leftover dirty buffers in either the
template database, or the database in its old location. Leading to
problems when accessing relations in an inconsistent state; and to
possible problems during shutdown in the SET TABLESPACE case because
buffers belonging files that don't exist anymore are flushed.

This was reported in bug #10675 by Maxim Boguk.

Fix by Pavan Deolasee, modified somewhat by me. Reviewed by MauMau and
Fujii Masao.

Backpatch to 9.1 where unlogged tables were introduced.

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index fde5da00bfb56ac12ab38581592da259b6843282..b8ba63d005aa555a127174af46a7c2d3e1d2d5ab 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7865,9 +7865,9 @@ LogCheckpointStart(int flags, bool restartpoint)
         * the main message, but what about all the flags?
         */
        if (restartpoint)
-               msg = "restartpoint starting:%s%s%s%s%s%s%s";
+               msg = "restartpoint starting:%s%s%s%s%s%s%s%s";
        else
-               msg = "checkpoint starting:%s%s%s%s%s%s%s";
+               msg = "checkpoint starting:%s%s%s%s%s%s%s%s";
 
        elog(LOG, msg,
                 (flags & CHECKPOINT_IS_SHUTDOWN) ? " shutdown" : "",
@@ -7876,7 +7876,8 @@ LogCheckpointStart(int flags, bool restartpoint)
                 (flags & CHECKPOINT_FORCE) ? " force" : "",
                 (flags & CHECKPOINT_WAIT) ? " wait" : "",
                 (flags & CHECKPOINT_CAUSE_XLOG) ? " xlog" : "",
-                (flags & CHECKPOINT_CAUSE_TIME) ? " time" : "");
+                (flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
+                (flags & CHECKPOINT_FLUSH_ALL) ? " flush-all" :"");
 }
 
 /*

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5f85022baba3d4594d36d3530de180ae70e429c7..5cad82412daa7c7e326859efd0d27a8cf4901066 100644 (file)
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -523,15 +523,17 @@ createdb(const CreatedbStmt *stmt)
                                                   DatabaseRelationId, dboid, 0, NULL);
 
        /*
-        * Force a checkpoint before starting the copy. This will force dirty
-        * buffers out to disk, to ensure source database is up-to-date on disk
-        * for the copy. FlushDatabaseBuffers() would suffice for that, but we
-        * also want to process any pending unlink requests. Otherwise, if a
-        * checkpoint happened while we're copying files, a file might be deleted
-        * just when we're about to copy it, causing the lstat() call in copydir()
-        * to fail with ENOENT.
+        * Force a checkpoint before starting the copy. This will force all dirty
+        * buffers, including those of unlogged tables, out to disk, to ensure
+        * source database is up-to-date on disk for the copy.
+        * FlushDatabaseBuffers() would suffice for that, but we also want
+        * to process any pending unlink requests. Otherwise, if a checkpoint
+        * happened while we're copying files, a file might be deleted just when
+        * we're about to copy it, causing the lstat() call in copydir() to fail
+        * with ENOENT.
         */
-       RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT);
+       RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT
+                                         | CHECKPOINT_FLUSH_ALL);
 
        /*
         * Take an MVCC snapshot to use while scanning through pg_tablespace.  For
@@ -1122,8 +1124,9 @@ movedb(const char *dbname, const char *tblspcname)
        dst_dbpath = GetDatabasePath(db_id, dst_tblspcoid);
 
        /*
-        * Force a checkpoint before proceeding. This will force dirty buffers out
-        * to disk, to ensure source database is up-to-date on disk for the copy.
+        * Force a checkpoint before proceeding. This will force all dirty
+        * buffers, including those of unlogged tables, out to disk, to ensure
+        * source database is up-to-date on disk for the copy.
         * FlushDatabaseBuffers() would suffice for that, but we also want to
         * process any pending unlink requests. Otherwise, the check for existing
         * files in the target directory might fail unnecessarily, not to mention
@@ -1131,7 +1134,8 @@ movedb(const char *dbname, const char *tblspcname)
         * On Windows, this also ensures that background procs don't hold any open
         * files, which would cause rmdir() to fail.
         */
-       RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT);
+       RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT
+                                         | CHECKPOINT_FLUSH_ALL);
 
        /*
         * Check for existence of files in the target directory, i.e., objects of

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 101f1a3ef47856e5d4683c5d2e4d755f0c20f262..34f36b4e8dbb8efda804a3bf1a5a9acce38c43d8 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1191,9 +1191,10 @@ UnpinBuffer(volatile BufferDesc *buf, bool fixOwner)
  *
  * This is called at checkpoint time to write out all dirty shared buffers.
  * The checkpoint request flags should be passed in.  If CHECKPOINT_IMMEDIATE
- * is set, we disable delays between writes; if CHECKPOINT_IS_SHUTDOWN is
- * set, we write even unlogged buffers, which are otherwise skipped.  The
- * remaining flags currently have no effect here.
+ * is set, we disable delays between writes; if CHECKPOINT_IS_SHUTDOWN,
+ * CHECKPOINT_END_OF_RECOVERY or CHECKPOINT_FLUSH_ALL is set, we write even
+ * unlogged buffers, which are otherwise skipped.  The remaining flags
+ * currently have no effect here.
  */
 static void
 BufferSync(int flags)
@@ -1208,10 +1209,12 @@ BufferSync(int flags)
        ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
 
        /*
-        * Unless this is a shutdown checkpoint, we write only permanent, dirty
-        * buffers.  But at shutdown or end of recovery, we write all dirty buffers.
+        * Unless this is a shutdown checkpoint or we have been explicitly told,
+        * we write only permanent, dirty buffers.  But at shutdown or end of
+        * recovery, we write all dirty buffers.
         */
-       if (!((flags & CHECKPOINT_IS_SHUTDOWN) || (flags & CHECKPOINT_END_OF_RECOVERY)))
+       if (!((flags & (CHECKPOINT_IS_SHUTDOWN | CHECKPOINT_END_OF_RECOVERY |
+                                       CHECKPOINT_FLUSH_ALL))))
                mask |= BM_PERMANENT;
 
        /*

diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index cc21d98a9a1778aced69ffe5e0c673da3e9346ac..e35aecc3495e12e298e1d997f4f62a97cf5aa2a1 100644 (file)
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -240,6 +240,8 @@ extern bool XLOG_DEBUG;
 /* These indicate the cause of a checkpoint request */
 #define CHECKPOINT_CAUSE_XLOG  0x0020  /* XLOG consumption */
 #define CHECKPOINT_CAUSE_TIME  0x0040  /* Elapsed time */
+#define CHECKPOINT_FLUSH_ALL   0x0080  /* Flush all pages, including those
+                                                                                * belonging to unlogged tables */
 
 /* Checkpoint statistics */
 typedef struct CheckpointStatsData