8000 Avoid unlikely data-loss scenarios due to rename() without fsync. · prmdeveloper/postgres@4f37d09 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4f37d09

Browse files
anarazelanarazel
committed
Avoid unlikely data-loss scenarios due to rename() without fsync.
Renaming a file using rename(2) is not guaranteed to be durable in face of crashes. Use the previously added durable_rename()/durable_link_or_rename() in various places where we previously just renamed files. Most of the changed call sites are arguably not critical, but it seems better to err on the side of too much durability. The most prominent known case where the previously missing fsyncs could cause data loss is crashes at the end of a checkpoint. After the actual checkpoint has been performed, old WAL files are recycled. When they're filled, their contents are fdatasynced, but we did not fsync the containing directory. An OS/hardware crash in an unfortunate moment could then end up leaving that file with its old name, but new content; WAL replay would thus not replay it. Reported-By: Tomas Vondra Author: Michael Paquier, Tomas Vondra, Andres Freund Discussion: 56583BDD.9060302@2ndquadrant.com Backpatch: All supported branches
1 parent 43b491a commit 4f37d09

File tree

6 files changed

+18
-93
lines changed

6 files changed

+18
-93
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -735,11 +735,7 @@ pgss_shmem_shutdown(int code, Datum arg)
735735
/*
736736
* Rename file into place, so we atomically replace any old one.
737737
*/
738-
if (rename(PGSS_DUMP_FILE ".tmp", PGSS_DUMP_FILE) != 0)
739-
ereport(LOG,
740-
(errcode_for_file_access(),
741-
errmsg("could not rename pg_stat_statement file \"%s\": %m",
742-
PGSS_DUMP_FILE ".tmp")));
738+
(void) durable_rename(PGSS_DUMP_FILE ".tmp", PGSS_DUMP_FILE, LOG);
743739

744740
/* Unlink query-texts file; it's not needed while shutdown */
745741
unlink(PGSS_TEXT_FILE);

‎src/backend/access/transam/timeline.c

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -418,24 +418,10 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
418418
TLHistoryFilePath(path, newTLI);
419419

420420
/*
421-
* Prefer link() to rename() here just to be really sure that we don't
422-
* overwrite an existing file. However, there shouldn't be one, so
423-
* rename() is an acceptable substitute except for the truly paranoid.
421+
* Perform the rename using link if available, paranoidly trying to avoid
422+
* overwriting an existing file (there shouldn't be one).
424423
*/
425-
#if HAVE_WORKING_LINK
426-
if (link(tmppath, path) < 0)
427-
ereport(ERROR,
428-
(errcode_for_file_access(),
429-
errmsg("could not link file \"%s\" to \"%s\": %m",
430-
tmppath, path)));
431-
unlink(tmppath);
432-
#else
433-
if (rename(tmppath, path) < 0)
434-
ereport(ERROR,
435-
(errcode_for_file_access(),
436-
errmsg("could not rename file \"%s\" to \"%s\": %m",
437-
tmppath, path)));
438-
#endif
424+
durable_link_or_rename(tmppath, path, ERROR);
439425

440426
/* The history file can be archived immediately. */
441427
if (XLogArchivingActive())
@@ -508,24 +494,10 @@ writeTimeLineHistoryFile(TimeLineID tli, char *content, int size)
508494
TLHistoryFilePath(path, tli);
509495

510496
/*
511-
* Prefer link() to rename() here just to be really sure that we don't
512-
* overwrite an existing logfile. However, there shouldn't be one, so
513-
* rename() is an acceptable substitute except for the truly paranoid.
497+
* Perform the rename using link if available, paranoidly trying to avoid
498+
* overwriting an existing file (there shouldn't be one).
514499
*/
515-
#if HAVE_WORKING_LINK
516-
if (link(tmppath, path) < 0)
517-
ereport(ERROR,
518-
(errcode_for_file_access(),
519-
errmsg("could not link file \"%s\" to \"%s\": %m",
520-
tmppath, path)));
521-
unlink(tmppath);
522-
#else
523-
if (rename(tmppath, path) < 0)
524-
ereport(ERROR,
525-
(errcode_for_file_access(),
526-
errmsg("could not rename file \"%s\" to \"%s\": %m",
527-
tmppath, path)));
528-
#endif
500+
durable_link_or_rename(tmppath, path, ERROR);
529501
}
530502

531503
/*

‎src/backend/access/transam/xlog.c

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3420,34 +3420,16 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
34203420
}
34213421

34223422
/*
3423-
* Prefer link() to rename() here just to be really sure that we don't
3424-
* overwrite an existing logfile. However, there shouldn't be one, so
3425-
* rename() is an acceptable substitute except for the truly paranoid.
3423+
* Perform the rename using link if available, paranoidly trying to avoid
3424+
* overwriting an existing file (there shouldn't be one).
34263425
*/
3427-
#if HAVE_WORKING_LINK
3428-
if (link(tmppath, path) < 0)
3426+
if (durable_link_or_rename(tmppath, path, LOG) != 0)
34293427
{
34303428
if (use_lock)
34313429
LWLockRelease(ControlFileLock);
3432-
ereport(LOG,
3433-
(errcode_for_file_access(),
3434-
errmsg("could not link file \"%s\" to \"%s\" (initialization of log file): %m",
3435-
tmppath, path)));
3436-
return false;
3437-
}
3438-
unlink(tmppath);
3439-
#else
3440-
if (rename(tmppath, path) < 0)
3441-
{
3442-
if (use_lock)
3443-
LWLockRelease(ControlFileLock);
3444-
ereport(LOG,
3445-
(errcode_for_file_access(),
3446-
errmsg("could not rename file \"%s\" to \"%s\" (initialization of log file): %m",
3447-
tmppath, path)));
3430+
/* durable_link_or_rename already emitted log message */
34483431
return false;
34493432
}
3450-
#endif
34513433

34523434
if (use_lock)
34533435
LWLockRelease(ControlFileLock);
@@ -5428,11 +5410,7 @@ exitArchiveRecovery(TimeLineID endTLI, XLogSegNo endLogSegNo)
54285410
* re-enter archive recovery mode in a subsequent crash.
54295411
*/
54305412
unlink(RECOVERY_COMMAND_DONE);
5431-
if (rename(RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE) != 0)
5432-
ereport(FATAL,
5433-
(errcode_for_file_access(),
5434-
errmsg("could not rename file \"%s\" to \"%s\": %m",
5435-
RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE)));
5413+
durable_rename(RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE, FATAL);
54365414

54375415
ereport(LOG,
54385416
(errmsg("archive recovery complete")));
@@ -6619,11 +6597,7 @@ StartupXLOG(void)
66196597
if (haveBackupLabel)
66206598
{
66216599
unlink(BACKUP_LABEL_OLD);
6622-
if (rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD) != 0)
6623-
ereport(FATAL,
6624-
(errcode_for_file_access(),
6625-
errmsg("could not rename file \"%s\" to \"%s\": %m",
6626-
BACKUP_LABEL_FILE, BACKUP_LABEL_OLD)));
6600+
durable_rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD, FATAL);
66276601
}
66286602

66296603
/* Check that the GUCs used to generate the WAL allow recovery */
@@ -10705,7 +10679,7 @@ CancelBackup(void)
1070510679
/* remove leftover file from previously canceled backup if it exists */
1070610680
unlink(BACKUP_LABEL_OLD);
1070710681

10708-
if (rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD) == 0)
10682+
if (durable_rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD, DEBUG1) == 0)
1070910683
{
1071010684
ereport(LOG,
1071110685
(errmsg("online backup mode canceled"),

‎src/backend/access/transam/xlogarchive.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -469,11 +469,7 @@ KeepFileRestoredFromArchive(char *path, char *xlogfname)
469469
reload = true;
470470
}
471471

472-
if (rename(path, xlogfpath) < 0)
473-
ereport(ERROR,
474-
(errcode_for_file_access(),
475-
errmsg("could not rename file \"%s\" to \"%s\": %m",
476-
path, xlogfpath)));
472+
durable_rename(path, xlogfpath, ERROR);
477473

478474
/*
479475
* Create .done file forcibly to prevent the restored segment from being
@@ -576,12 +572,7 @@ XLogArchiveForceDone(const char *xlog)
576572
StatusFilePath(archiveReady, xlog, ".ready");
577573
if (stat(archiveReady, &stat_buf) == 0)
578574
{
579-
if (rename(archiveReady, archiveDone) < 0)
580-
ereport(WARNING,
581-
(errcode_for_file_access(),
582-
errmsg("could not rename file \"%s\" to \"%s\": %m",
583-
archiveReady, archiveDone)));
584-
575+
(void) durable_rename(archiveReady, archiveDone, WARNING);
585576
return;
586577
}
587578

‎src/backend/postmaster/pgarch.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -753,9 +753,5 @@ pgarch_archiveDone(char *xlog)
753753

754754
StatusFilePath(rlogready, xlog, ".ready");
755755
StatusFilePath(rlogdone, xlog, ".done");
756-
if (rename(rlogready, rlogdone) < 0)
757-
ereport(WARNING,
758-
(errcode_for_file_access(),
759-
errmsg("could not rename file \"%s\" to \"%s\": %m",
760-
rlogready, rlogdone)));
756+
(void) durable_rename(rlogready, rlogdone, WARNING);
761757
}

‎src/backend/utils/misc/guc.c

Lines changed: 1 addition & 5 deletions
6823
Original file line numberDiff line numberDiff line change
@@ -6817,11 +6817,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
68176817
* at worst it can lose the parameters set by last ALTER SYSTEM
68186818
* command.
68196819
*/
6820-
if (rename(AutoConfTmpFileName, AutoConfFileName) < 0)
6821-
ereport(ERROR,
6822-
(errcode_for_file_access(),
6823-
errmsg("could not rename file \"%s\" to \"%s\": %m",
6824-
AutoConfTmpFileName, AutoConfFileName)));
6820+
durable_rename(AutoConfTmpFileName, AutoConfFileName, ERROR);
68256821
}
68266822
PG_CATCH();
6827
{

0 commit comments

Comments
 (0)
0