diff options
| author | Andres Freund <andres@anarazel.de> | 2016-03-09 18:53:53 -0800 |
|---|---|---|
| committer | Andres Freund <andres@anarazel.de> | 2016-03-09 18:53:53 -0800 |
| commit | 1d4a0ab19a7e45aa8b94d7f720d1d9cefb81ec40 (patch) | |
| tree | aba18aaf2557befbbf9f028a4a25e12843c51379 /src/backend/replication | |
| parent | 606e0f9841b820d826f837bf741a3e5e9cc62fa1 (diff) | |
| download | postgresql-1d4a0ab19a7e45aa8b94d7f720d1d9cefb81ec40.tar.gz postgresql-1d4a0ab19a7e45aa8b94d7f720d1d9cefb81ec40.zip | |
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
Diffstat (limited to 'src/backend/replication')
| -rw-r--r-- | src/backend/replication/logical/origin.c | 23 |
1 files changed, 2 insertions, 21 deletions
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c index 7e2307f5f22..8c8833b71d0 100644 --- 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); } /* |