Skip to content

Commit 1d4a0ab

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 606e0f9 commit 1d4a0ab

File tree

7 files changed

+24
-126
lines changed

7 files changed

+24
-126
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -741,11 +741,7 @@ pgss_shmem_shutdown(int code, Datum arg)
741741
/*
742742
* Rename file into place, so we atomically replace any old one.
743743
*/
744-
if (rename(PGSS_DUMP_FILE ".tmp", PGSS_DUMP_FILE) != 0)
745-
ereport(LOG,
746-
(errcode_for_file_access(),
747-
errmsg("could not rename pg_stat_statement file \"%s\": %m",
748-
PGSS_DUMP_FILE ".tmp")));
744+
(void) durable_rename(PGSS_DUMP_FILE ".tmp", PGSS_DUMP_FILE, LOG);
749745

750746
/* Unlink query-texts file; it's not needed while shutdown */
751747
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: 11 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3299,34 +3299,16 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
32993299
}
33003300

33013301
/*
3302-
* Prefer link() to rename() here just to be really sure that we don't
3303-
* overwrite an existing logfile. However, there shouldn't be one, so
3304-
* rename() is an acceptable substitute except for the truly paranoid.
3302+
* Perform the rename using link if available, paranoidly trying to avoid
3303+
* overwriting an existing file (there shouldn't be one).
33053304
*/
3306-
#if HAVE_WORKING_LINK
3307-
if (link(tmppath, path) < 0)
3305+
if (durable_link_or_rename(tmppath, path, LOG) != 0)
33083306
{
33093307
if (use_lock)
33103308
LWLockRelease(ControlFileLock);
3311-
ereport(LOG,
3312-
(errcode_for_file_access(),
3313-
errmsg("could not link file \"%s\" to \"%s\" (initialization of log file): %m",
3314-
tmppath, path)));
3315-
return false;
3316-
}
3317-
unlink(tmppath);
3318-
#else
3319-
if (rename(tmppath, path) < 0)
3320-
{
3321-
if (use_lock)
3322-
LWLockRelease(ControlFileLock);
3323-
ereport(LOG,
3324-
(errcode_for_file_access(),
3325-
errmsg("could not rename file \"%s\" to \"%s\" (initialization of log file): %m",
3326-
tmppath, path)));
3309+
/* durable_link_or_rename already emitted log message */
33273310
return false;
33283311
}
3329-
#endif
33303312

33313313
if (use_lock)
33323314
LWLockRelease(ControlFileLock);
@@ -5339,11 +5321,7 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
53395321
* re-enter archive recovery mode in a subsequent crash.
53405322
*/
53415323
unlink(RECOVERY_COMMAND_DONE);
5342-
if (rename(RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE) != 0)
5343-
ereport(FATAL,
5344-
(errcode_for_file_access(),
5345-
errmsg("could not rename file \"%s\" to \"%s\": %m",
5346-
RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE)));
5324+
durable_rename(RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE, FATAL);
53475325

53485326
ereport(LOG,
53495327
(errmsg("archive recovery complete")));
@@ -6190,7 +6168,7 @@ StartupXLOG(void)
61906168
if (stat(TABLESPACE_MAP, &st) == 0)
61916169
{
61926170
unlink(TABLESPACE_MAP_OLD);
6193-
if (rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD) == 0)
6171+
if (durable_rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD, DEBUG1) == 0)
61946172
ereport(LOG,
61956173
(errmsg("ignoring file \"%s\" because no file \"%s\" exists",
61966174
TABLESPACE_MAP, BACKUP_LABEL_FILE),
@@ -6553,11 +6531,7 @@ StartupXLOG(void)
65536531
if (haveBackupLabel)
65546532
{
65556533
unlink(BACKUP_LABEL_OLD);
6556-
if (rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD) != 0)
6557-
ereport(FATAL,
6558-
(errcode_for_file_access(),
6559-
errmsg("could not rename file \"%s\" to \"%s\": %m",
6560-
BACKUP_LABEL_FILE, BACKUP_LABEL_OLD)));
6534+
durable_rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD, FATAL);
65616535
}
65626536

65636537
/*
@@ -6570,11 +6544,7 @@ StartupXLOG(void)
65706544
if (haveTblspcMap)
65716545
{
65726546
unlink(TABLESPACE_MAP_OLD);
6573-
if (rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD) != 0)
6574-
ereport(FATAL,
6575-
(errcode_for_file_access(),
6576-
errmsg("could not rename file \"%s\" to \"%s\": %m",
6577-
TABLESPACE_MAP, TABLESPACE_MAP_OLD)));
6547+
durable_rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD, FATAL);
65786548
}
65796549

65806550
/* Check that the GUCs used to generate the WAL allow recovery */
@@ -7351,11 +7321,7 @@ StartupXLOG(void)
73517321
*/
73527322
XLogArchiveCleanup(partialfname);
73537323

7354-
if (rename(origpath, partialpath) != 0)
7355-
ereport(ERROR,
7356-
(errcode_for_file_access(),
7357-
errmsg("could not rename file \"%s\" to \"%s\": %m",
7358-
origpath, partialpath)));
7324+
durable_rename(origpath, partialpath, ERROR);
73597325
XLogArchiveNotify(partialfname);
73607326
}
73617327
}
@@ -10911,7 +10877,7 @@ CancelBackup(void)
1091110877
/* remove leftover file from previously canceled backup if it exists */
1091210878
unlink(BACKUP_LABEL_OLD);
1091310879

10914-
if (rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD) != 0)
10880+
if (durable_rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD, DEBUG1) != 0)
1091510881
{
1091610882
ereport(WARNING,
1091710883
(errcode_for_file_access(),
@@ -10934,7 +10900,7 @@ CancelBackup(void)
1093410900
/* remove leftover file from previously canceled backup if it exists */
1093510901
unlink(TABLESPACE_MAP_OLD);
1093610902

10937-
if (rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD) == 0)
10903+
if (durable_rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD, DEBUG1) == 0)
1093810904
{
1093910905
ereport(LOG,
1094010906
(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
@@ -470,11 +470,7 @@ KeepFileRestoredFromArchive(char *path, char *xlogfname)
470470
reload = true;
471471
}
472472

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

479475
/*
480476
* Create .done file forcibly to prevent the restored segment from being
@@ -580,12 +576,7 @@ XLogArchiveForceDone(const char *xlog)
580576
StatusFilePath(archiveReady, xlog, ".ready");
581577
if (stat(archiveReady, &stat_buf) == 0)
582578
{
583-
if (rename(archiveReady, archiveDone) < 0)
584-
ereport(WARNING,
585-
(errcode_for_file_access(),
586-
errmsg("could not rename file \"%s\" to \"%s\": %m",
587-
archiveReady, archiveDone)));
588-
579+
(void) durable_rename(archiveReady, archiveDone, WARNING);
589580
return;
590581
}
591582

‎src/backend/postmaster/pgarch.c

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

729729
StatusFilePath(rlogready, xlog, ".ready");
730730
StatusFilePath(rlogdone, xlog, ".done");
731-
if (rename(rlogready, rlogdone) < 0)
732-
ereport(WARNING,
733-
(errcode_for_file_access(),
734-
errmsg("could not rename file \"%s\" to \"%s\": %m",
735-
rlogready, rlogdone)));
731+
(void) durable_rename(rlogready, rlogdone, WARNING);
736732
}

‎src/backend/replication/logical/origin.c

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -604,29 +604,10 @@ CheckPointReplicationOrigin(void)
604604
tmppath)));
605605
}
606606

607-
/* fsync the temporary file */
608-
if (pg_fsync(tmpfd) != 0)
609-
{
610-
CloseTransientFile(tmpfd);
611-
ereport(PANIC,
612-
(errcode_for_file_access(),
613-
errmsg("could not fsync file \"%s\": %m",
614-
tmppath)));
615-
}
616-
617607
CloseTransientFile(tmpfd);
618608

619-
/* rename to permanent file, fsync file and directory */
620-
if (rename(tmppath, path) != 0)
621-
{
622-
ereport(PANIC,
623-
(errcode_for_file_access(),
624-
errmsg("could not rename file \"%s\" to \"%s\": %m",
625-
tmppath, path)));
626-
}
627-
628-
fsync_fname(path, false);
629-
fsync_fname("pg_logical", true);
609+
/* fsync, rename to permanent file, fsync file and directory */
610+
durable_rename(tmppath, path, PANIC);
630611
}
631612

632613
/*

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7037,11 +7037,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
70377037
* at worst it can lose the parameters set by last ALTER SYSTEM
70387038
* command.
70397039
*/
7040-
if (rename(AutoConfTmpFileName, AutoConfFileName) < 0)
7041-
ereport(ERROR,
7042-
(errcode_for_file_access(),
7043-
errmsg("could not rename file \"%s\" to \"%s\": %m",
7044-
AutoConfTmpFileName, AutoConfFileName)));
7040+
durable_rename(AutoConfTmpFileName, AutoConfFileName, ERROR);
70457041
}
70467042
PG_CATCH();
70477043
{

0 commit comments

Comments
 (0)