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

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index dffc4778fa8557d6eba21d1f864ad76ba34475ca..9ce60e696c9261ecae80ccb5a374764a1993cf2d 100644 (file)
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -741,11 +741,7 @@ pgss_shmem_shutdown(int code, Datum arg)
        /*
         * Rename file into place, so we atomically replace any old one.
         */
-       if (rename(PGSS_DUMP_FILE ".tmp", PGSS_DUMP_FILE) != 0)
-               ereport(LOG,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename pg_stat_statement file \"%s\": %m",
-                                               PGSS_DUMP_FILE ".tmp")));
+       (void) durable_rename(PGSS_DUMP_FILE ".tmp", PGSS_DUMP_FILE, LOG);
 
        /* Unlink query-texts file; it's not needed while shutdown */
        unlink(PGSS_TEXT_FILE);

diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c
index f6da673477aec310c4d15a7d147b97ff92f1d61c..bd91573708b224e84e36b2aff85f200a95986d92 100644 (file)
--- a/src/backend/access/transam/timeline.c
+++ b/src/backend/access/transam/timeline.c
@@ -418,24 +418,10 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
        TLHistoryFilePath(path, newTLI);
 
        /*
-        * Prefer link() to rename() here just to be really sure that we don't
-        * overwrite an existing file.  However, there shouldn't be one, so
-        * rename() is an acceptable substitute except for the truly paranoid.
+        * Perform the rename using link if available, paranoidly trying to avoid
+        * overwriting an existing file (there shouldn't be one).
         */
-#if HAVE_WORKING_LINK
-       if (link(tmppath, path) < 0)
-               ereport(ERROR,
-                               (errcode_for_file_access(),
-                                errmsg("could not link file \"%s\" to \"%s\": %m",
-                                               tmppath, path)));
-       unlink(tmppath);
-#else
-       if (rename(tmppath, path) < 0)
-               ereport(ERROR,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                               tmppath, path)));
-#endif
+       durable_link_or_rename(tmppath, path, ERROR);
 
        /* The history file can be archived immediately. */
        if (XLogArchivingActive())
@@ -508,24 +494,10 @@ writeTimeLineHistoryFile(TimeLineID tli, char *content, int size)
        TLHistoryFilePath(path, tli);
 
        /*
-        * Prefer link() to rename() here just to be really sure that we don't
-        * overwrite an existing logfile.  However, there shouldn't be one, so
-        * rename() is an acceptable substitute except for the truly paranoid.
+        * Perform the rename using link if available, paranoidly trying to avoid
+        * overwriting an existing file (there shouldn't be one).
         */
-#if HAVE_WORKING_LINK
-       if (link(tmppath, path) < 0)
-               ereport(ERROR,
-                               (errcode_for_file_access(),
-                                errmsg("could not link file \"%s\" to \"%s\": %m",
-                                               tmppath, path)));
-       unlink(tmppath);
-#else
-       if (rename(tmppath, path) < 0)
-               ereport(ERROR,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                               tmppath, path)));
-#endif
+       durable_link_or_rename(tmppath, path, ERROR);
 }
 
 /*

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 00f139a0f388ed90f70b1db75d7f64ca01270999..5b1c36111767e42d62be78491acebadc337f5b97 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3299,34 +3299,16 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
        }
 
        /*
-        * Prefer link() to rename() here just to be really sure that we don't
-        * overwrite an existing logfile.  However, there shouldn't be one, so
-        * rename() is an acceptable substitute except for the truly paranoid.
+        * Perform the rename using link if available, paranoidly trying to avoid
+        * overwriting an existing file (there shouldn't be one).
         */
-#if HAVE_WORKING_LINK
-       if (link(tmppath, path) < 0)
+       if (durable_link_or_rename(tmppath, path, LOG) != 0)
        {
                if (use_lock)
                        LWLockRelease(ControlFileLock);
-               ereport(LOG,
-                               (errcode_for_file_access(),
-                                errmsg("could not link file \"%s\" to \"%s\" (initialization of log file): %m",
-                                               tmppath, path)));
-               return false;
-       }
-       unlink(tmppath);
-#else
-       if (rename(tmppath, path) < 0)
-       {
-               if (use_lock)
-                       LWLockRelease(ControlFileLock);
-               ereport(LOG,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename file \"%s\" to \"%s\" (initialization of log file): %m",
-                                               tmppath, path)));
+               /* durable_link_or_rename already emitted log message */
                return false;
        }
-#endif
 
        if (use_lock)
                LWLockRelease(ControlFileLock);
@@ -5339,11 +5321,7 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
         * re-enter archive recovery mode in a subsequent crash.
         */
        unlink(RECOVERY_COMMAND_DONE);
-       if (rename(RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE) != 0)
-               ereport(FATAL,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                               RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE)));
+       durable_rename(RECOVERY_COMMAND_FILE, RECOVERY_COMMAND_DONE, FATAL);
 
        ereport(LOG,
                        (errmsg("archive recovery complete")));
@@ -6190,7 +6168,7 @@ StartupXLOG(void)
                if (stat(TABLESPACE_MAP, &st) == 0)
                {
                        unlink(TABLESPACE_MAP_OLD);
-                       if (rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD) == 0)
+                       if (durable_rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD, DEBUG1) == 0)
                                ereport(LOG,
                                        (errmsg("ignoring file \"%s\" because no file \"%s\" exists",
                                                        TABLESPACE_MAP, BACKUP_LABEL_FILE),
@@ -6553,11 +6531,7 @@ StartupXLOG(void)
                if (haveBackupLabel)
                {
                        unlink(BACKUP_LABEL_OLD);
-                       if (rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD) != 0)
-                               ereport(FATAL,
-                                               (errcode_for_file_access(),
-                                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                                               BACKUP_LABEL_FILE, BACKUP_LABEL_OLD)));
+                       durable_rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD, FATAL);
                }
 
                /*
@@ -6570,11 +6544,7 @@ StartupXLOG(void)
                if (haveTblspcMap)
                {
                        unlink(TABLESPACE_MAP_OLD);
-                       if (rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD) != 0)
-                               ereport(FATAL,
-                                               (errcode_for_file_access(),
-                                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                                               TABLESPACE_MAP, TABLESPACE_MAP_OLD)));
+                       durable_rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD, FATAL);
                }
 
                /* Check that the GUCs used to generate the WAL allow recovery */
@@ -7351,11 +7321,7 @@ StartupXLOG(void)
                                 */
                                XLogArchiveCleanup(partialfname);
 
-                               if (rename(origpath, partialpath) != 0)
-                                       ereport(ERROR,
-                                                       (errcode_for_file_access(),
-                                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                                               origpath, partialpath)));
+                               durable_rename(origpath, partialpath, ERROR);
                                XLogArchiveNotify(partialfname);
                        }
                }
@@ -10911,7 +10877,7 @@ CancelBackup(void)
        /* remove leftover file from previously canceled backup if it exists */
        unlink(BACKUP_LABEL_OLD);
 
-       if (rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD) != 0)
+       if (durable_rename(BACKUP_LABEL_FILE, BACKUP_LABEL_OLD, DEBUG1) != 0)
        {
                ereport(WARNING,
                                (errcode_for_file_access(),
@@ -10934,7 +10900,7 @@ CancelBackup(void)
        /* remove leftover file from previously canceled backup if it exists */
        unlink(TABLESPACE_MAP_OLD);
 
-       if (rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD) == 0)
+       if (durable_rename(TABLESPACE_MAP, TABLESPACE_MAP_OLD, DEBUG1) == 0)
        {
                ereport(LOG,
                                (errmsg("online backup mode canceled"),

diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c
index 277c14a81047ab2099578a21ee319bf321d58baa..d153a44ea9a16634fc4692fecfd448b5fc511d59 100644 (file)
--- a/src/backend/access/transam/xlogarchive.c
+++ b/src/backend/access/transam/xlogarchive.c
@@ -470,11 +470,7 @@ KeepFileRestoredFromArchive(char *path, char *xlogfname)
                reload = true;
        }
 
-       if (rename(path, xlogfpath) < 0)
-               ereport(ERROR,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                               path, xlogfpath)));
+       durable_rename(path, xlogfpath, ERROR);
 
        /*
         * Create .done file forcibly to prevent the restored segment from being
@@ -580,12 +576,7 @@ XLogArchiveForceDone(const char *xlog)
        StatusFilePath(archiveReady, xlog, ".ready");
        if (stat(archiveReady, &stat_buf) == 0)
        {
-               if (rename(archiveReady, archiveDone) < 0)
-                       ereport(WARNING,
-                                       (errcode_for_file_access(),
-                                        errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                                       archiveReady, archiveDone)));
-
+               (void) durable_rename(archiveReady, archiveDone, WARNING);
                return;
        }
 

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 397f8024e289a6a367d93d4b3a5aaf249833bc4f..1aa6466d67f23a4752536527b450e038a7a1dca7 100644 (file)
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -728,9 +728,5 @@ pgarch_archiveDone(char *xlog)
 
        StatusFilePath(rlogready, xlog, ".ready");
        StatusFilePath(rlogdone, xlog, ".done");
-       if (rename(rlogready, rlogdone) < 0)
-               ereport(WARNING,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                               rlogready, rlogdone)));
+       (void) durable_rename(rlogready, rlogdone, WARNING);
 }

diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 7e2307f5f2228fbc0d9865c88f8fb6fd81000075..8c8833b71d0368d666096f25f30557ac2bdc90ad 100644 (file)
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -604,29 +604,10 @@ CheckPointReplicationOrigin(void)
                                                tmppath)));
        }
 
-       /* fsync the temporary file */
-       if (pg_fsync(tmpfd) != 0)
-       {
-               CloseTransientFile(tmpfd);
-               ereport(PANIC,
-                               (errcode_for_file_access(),
-                                errmsg("could not fsync file \"%s\": %m",
-                                               tmppath)));
-       }
-
        CloseTransientFile(tmpfd);
 
-       /* rename to permanent file, fsync file and directory */
-       if (rename(tmppath, path) != 0)
-       {
-               ereport(PANIC,
-                               (errcode_for_file_access(),
-                                errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                               tmppath, path)));
-       }
-
-       fsync_fname(path, false);
-       fsync_fname("pg_logical", true);
+       /* fsync, rename to permanent file, fsync file and directory */
+       durable_rename(tmppath, path, PANIC);
 }
 
 /*

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ea5a09ac1be785604d0a8fd74323dfa2aaa51880..0be64a1c9f3cc65ac0a10686f13b6e809df79bca 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7037,11 +7037,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
                 * at worst it can lose the parameters set by last ALTER SYSTEM
                 * command.
                 */
-               if (rename(AutoConfTmpFileName, AutoConfFileName) < 0)
-                       ereport(ERROR,
-                                       (errcode_for_file_access(),
-                                        errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                                       AutoConfTmpFileName, AutoConfFileName)));
+               durable_rename(AutoConfTmpFileName, AutoConfFileName, ERROR);
        }
        PG_CATCH();
        {