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 5793068446d8c40a0446d4810ef8a64ffce9794c..18d501348c3a2c22e789691d4b37923c1b43e9a0 100644 (file)
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -504,11 +504,7 @@ pgss_shmem_shutdown(int code, Datum arg)
        /*
         * Rename file into place, so we atomically replace the 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);
 
        return;
 

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3e619c81e2547c085ea9c1f4fa900834891fdb9c..1e79ebdc8fb0c1beaa279f5a7557295fab1ad159 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2672,34 +2672,16 @@ InstallXLogFileSegment(uint32 *log, uint32 *seg, 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 %u, segment %u): %m",
-                                               tmppath, path, *log, *seg)));
+               /* durable_link_or_rename already emitted log message */
                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 %u, segment %u): %m",
-                                               tmppath, path, *log, *seg)));
-               return false;
-       }
-#endif
 
        if (use_lock)
                LWLockRelease(ControlFileLock);
@@ -4620,24 +4602,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 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);
 
        /* The history file can be archived immediately. */
        if (XLogArchivingActive())
@@ -5595,11 +5563,7 @@ exitArchiveRecovery(TimeLineID endTLI, uint32 endLogId, uint32 endLogSeg)
                                (errmsg_internal("moving last restored xlog to \"%s\"",
                                                                 xlogpath)));
                unlink(xlogpath);               /* might or might not exist */
-               if (rename(recoveryPath, xlogpath) != 0)
-                       ereport(FATAL,
-                                       (errcode_for_file_access(),
-                                        errmsg("could not rename file \"%s\" to \"%s\": %m",
-                                                       recoveryPath, xlogpath)));
+               durable_rename(recoveryPath, xlogpath, FATAL);
                /* XXX might we need to fix permissions on the file? */
        }
        else
@@ -5648,11 +5612,7 @@ exitArchiveRecovery(TimeLineID endTLI, uint32 endLogId, uint32 endLogSeg)
         * 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")));
@@ -6548,11 +6508,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);
                }
 
                /* Check that the GUCs used to generate the WAL allow recovery */
@@ -10148,7 +10104,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(LOG,
                                (errmsg("online backup mode canceled"),

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 212432ec06b76d1988ec4536bd9267f56431f499..c7596a949a3fc33a8b9e72bb5b258340dcbb7688 100644 (file)
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -709,9 +709,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);
 }