Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 7dbb606

Browse files
committed
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.
1 parent 83dc590 commit 7dbb606

File tree

4 files changed

+30
-21
lines changed

4 files changed

+30
-21
lines changed

src/backend/access/transam/xlog.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7771,9 +7771,9 @@ LogCheckpointStart(int flags, bool restartpoint)
77717771
* the main message, but what about all the flags?
77727772
*/
77737773
if (restartpoint)
7774-
msg = "restartpoint starting:%s%s%s%s%s%s%s";
7774+
msg = "restartpoint starting:%s%s%s%s%s%s%s%s";
77757775
else
7776-
msg = "checkpoint starting:%s%s%s%s%s%s%s";
7776+
msg = "checkpoint starting:%s%s%s%s%s%s%s%s";
77777777

77787778
elog(LOG, msg,
77797779
(flags & CHECKPOINT_IS_SHUTDOWN) ? " shutdown" : "",
@@ -7782,7 +7782,8 @@ LogCheckpointStart(int flags, bool restartpoint)
77827782
(flags & CHECKPOINT_FORCE) ? " force" : "",
77837783
(flags & CHECKPOINT_WAIT) ? " wait" : "",
77847784
(flags & CHECKPOINT_CAUSE_XLOG) ? " xlog" : "",
7785-
(flags & CHECKPOINT_CAUSE_TIME) ? " time" : "");
7785+
(flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
7786+
(flags & CHECKPOINT_FLUSH_ALL) ? " flush-all" :"");
77867787
}
77877788

77887789
/*

src/backend/commands/dbcommands.c

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -549,15 +549,17 @@ createdb(const CreatedbStmt *stmt)
549549
InvokeObjectPostCreateHook(DatabaseRelationId, dboid, 0);
550550

551551
/*
552-
* Force a checkpoint before starting the copy. This will force dirty
553-
* buffers out to disk, to ensure source database is up-to-date on disk
554-
* for the copy. FlushDatabaseBuffers() would suffice for that, but we
555-
* also want to process any pending unlink requests. Otherwise, if a
556-
* checkpoint happened while we're copying files, a file might be deleted
557-
* just when we're about to copy it, causing the lstat() call in copydir()
558-
* to fail with ENOENT.
552+
* Force a checkpoint before starting the copy. This will force all dirty
553+
* buffers, including those of unlogged tables, out to disk, to ensure
554+
* source database is up-to-date on disk for the copy.
555+
* FlushDatabaseBuffers() would suffice for that, but we also want
556+
* to process any pending unlink requests. Otherwise, if a checkpoint
557+
* happened while we're copying files, a file might be deleted just when
558+
* we're about to copy it, causing the lstat() call in copydir() to fail
559+
* with ENOENT.
559560
*/
560-
RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT);
561+
RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT
562+
| CHECKPOINT_FLUSH_ALL);
561563

562564
/*
563565
* Once we start copying subdirectories, we need to be able to clean 'em
@@ -1138,16 +1140,18 @@ movedb(const char *dbname, const char *tblspcname)
11381140
dst_dbpath = GetDatabasePath(db_id, dst_tblspcoid);
11391141

11401142
/*
1141-
* Force a checkpoint before proceeding. This will force dirty buffers out
1142-
* to disk, to ensure source database is up-to-date on disk for the copy.
1143+
* Force a checkpoint before proceeding. This will force all dirty
1144+
* buffers, including those of unlogged tables, out to disk, to ensure
1145+
* source database is up-to-date on disk for the copy.
11431146
* FlushDatabaseBuffers() would suffice for that, but we also want to
11441147
* process any pending unlink requests. Otherwise, the check for existing
11451148
* files in the target directory might fail unnecessarily, not to mention
11461149
* that the copy might fail due to source files getting deleted under it.
11471150
* On Windows, this also ensures that background procs don't hold any open
11481151
* files, which would cause rmdir() to fail.
11491152
*/
1150-
RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT);
1153+
RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT
1154+
| CHECKPOINT_FLUSH_ALL);
11511155

11521156
/*
11531157
* Check for existence of files in the target directory, i.e., objects of

src/backend/storage/buffer/bufmgr.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1493,9 +1493,10 @@ UnpinBuffer(volatile BufferDesc *buf, bool fixOwner)
14931493
*
14941494
* This is called at checkpoint time to write out all dirty shared buffers.
14951495
* The checkpoint request flags should be passed in. If CHECKPOINT_IMMEDIATE
1496-
* is set, we disable delays between writes; if CHECKPOINT_IS_SHUTDOWN is
1497-
* set, we write even unlogged buffers, which are otherwise skipped. The
1498-
* remaining flags currently have no effect here.
1496+
* is set, we disable delays between writes; if CHECKPOINT_IS_SHUTDOWN,
1497+
* CHECKPOINT_END_OF_RECOVERY or CHECKPOINT_FLUSH_ALL is set, we write even
1498+
* unlogged buffers, which are otherwise skipped. The remaining flags
1499+
* currently have no effect here.
14991500
*/
15001501
static void
15011502
BufferSync(int flags)
@@ -1510,11 +1511,12 @@ BufferSync(int flags)
15101511
ResourceOwnerEnlargeBuffers(CurrentResourceOwner);
15111512

15121513
/*
1513-
* Unless this is a shutdown checkpoint, we write only permanent, dirty
1514-
* buffers. But at shutdown or end of recovery, we write all dirty
1515-
* buffers.
1514+
* Unless this is a shutdown checkpoint or we have been explicitly told,
1515+
* we write only permanent, dirty buffers. But at shutdown or end of
1516+
* recovery, we write all dirty buffers.
15161517
*/
1517-
if (!((flags & CHECKPOINT_IS_SHUTDOWN) || (flags & CHECKPOINT_END_OF_RECOVERY)))
1518+
if (!((flags & (CHECKPOINT_IS_SHUTDOWN | CHECKPOINT_END_OF_RECOVERY |
1519+
CHECKPOINT_FLUSH_ALL))))
15181520
mask |= BM_PERMANENT;
15191521

15201522
/*

src/include/access/xlog.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,8 @@ extern bool XLOG_DEBUG;
253253
/* These indicate the cause of a checkpoint request */
254254
#define CHECKPOINT_CAUSE_XLOG 0x0020 /* XLOG consumption */
255255
#define CHECKPOINT_CAUSE_TIME 0x0040 /* Elapsed time */
256+
#define CHECKPOINT_FLUSH_ALL 0x0080 /* Flush all pages, including those
257+
* belonging to unlogged tables */
256258

257259
/* Checkpoint statistics */
258260
typedef struct CheckpointStatsData

0 commit comments

Comments
 (0)