On 2020-02-28 19:44, Alvaro Herrera wrote:
On 2020-Feb-28, Tom Lane wrote:
Also +1 for s/durable_link_or_rename/durable_link/.
Actually, it's not *that* either, because what the function does is link
followed by unlink. So it's more a variation of durable_rename with
slightly different semantics -- the difference is what happens if a file
with the target name already exists. Maybe call it durable_rename_no_overwrite.
I have committed the first two patches.
Here is the third patch again, we renaming durable_link_or_rename() to
durable_rename_excl(). This seems to match existing Unix system call
naming best (see open() flag O_EXCL, and macOS has a renamex_np() flag
RENAME_EXCL).
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From e892237439e49770cc5013848f4b1f8b5c6c0ff4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 4 Mar 2020 17:33:10 +0100
Subject: [PATCH v2] Remove HAVE_WORKING_LINK
Previously, hard links were not used on Windows and Cygwin, but they
support them just fine in currently supported OS versions, so we can
use them there as well.
Since all supported platforms now support hard links, we can remove
the alternative code paths.
Rename durable_link_or_rename() to durable_rename_excl() to make the
purpose more clear without referencing the implementation details.
Discussion:
https://www.postgresql.org/message-id/flat/72fff73f-dc9c-4ef4-83e8-d2e60c98df48%402ndquadrant.com
---
src/backend/access/transam/timeline.c | 4 ++--
src/backend/access/transam/xlog.c | 4 ++--
src/backend/storage/file/fd.c | 21 +++++----------------
src/include/pg_config_manual.h | 7 -------
src/include/storage/fd.h | 2 +-
5 files changed, 10 insertions(+), 28 deletions(-)
diff --git a/src/backend/access/transam/timeline.c
b/src/backend/access/transam/timeline.c
index 57ee4e048b..860a996414 100644
--- a/src/backend/access/transam/timeline.c
+++ b/src/backend/access/transam/timeline.c
@@ -429,7 +429,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID
parentTLI,
* Perform the rename using link if available, paranoidly trying to
avoid
* overwriting an existing file (there shouldn't be one).
*/
- durable_link_or_rename(tmppath, path, ERROR);
+ durable_rename_excl(tmppath, path, ERROR);
/* The history file can be archived immediately. */
if (XLogArchivingActive())
@@ -507,7 +507,7 @@ writeTimeLineHistoryFile(TimeLineID tli, char *content, int
size)
* Perform the rename using link if available, paranoidly trying to
avoid
* overwriting an existing file (there shouldn't be one).
*/
- durable_link_or_rename(tmppath, path, ERROR);
+ durable_rename_excl(tmppath, path, ERROR);
}
/*
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 4361568882..108358df57 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3591,11 +3591,11 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
* Perform the rename using link if available, paranoidly trying to
avoid
* overwriting an existing file (there shouldn't be one).
*/
- if (durable_link_or_rename(tmppath, path, LOG) != 0)
+ if (durable_rename_excl(tmppath, path, LOG) != 0)
{
if (use_lock)
LWLockRelease(ControlFileLock);
- /* durable_link_or_rename already emitted log message */
+ /* durable_rename_excl already emitted log message */
return false;
}
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 34f7443110..7dc6dd2f15 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -765,10 +765,11 @@ durable_unlink(const char *fname, int elevel)
}
/*
- * durable_link_or_rename -- rename a file in a durable manner.
+ * durable_rename_excl -- rename a file in a durable manner, without
+ * overwriting an existing target file
*
- * Similar to durable_rename(), except that this routine tries (but does not
- * guarantee) not to overwrite the target file.
+ * Similar to durable_rename(), except that this routine will fail if the
+ * target file already exists.
*
* Note that a crash in an unfortunate moment can leave you with two links to
* the target file.
@@ -779,7 +780,7 @@ durable_unlink(const char *fname, int elevel)
* valid upon return.
*/
int
-durable_link_or_rename(const char *oldfile, const char *newfile, int elevel)
+durable_rename_excl(const char *oldfile, const char *newfile, int elevel)
{
/*
* Ensure that, if we crash directly after the rename/link, a file with
@@ -788,7 +789,6 @@ durable_link_or_rename(const char *oldfile, const char
*newfile, int elevel)
if (fsync_fname_ext(oldfile, false, false, elevel) != 0)
return -1;
-#ifdef HAVE_WORKING_LINK
if (link(oldfile, newfile) < 0)
{
ereport(elevel,
@@ -798,17 +798,6 @@ durable_link_or_rename(const char *oldfile, const char
*newfile, int elevel)
return -1;
}
unlink(oldfile);
-#else
- /* XXX: Add racy file existence check? */
- if (rename(oldfile, newfile) < 0)
- {
- ereport(elevel,
- (errcode_for_file_access(),
- errmsg("could not rename file \"%s\" to
\"%s\": %m",
- oldfile, newfile)));
- return -1;
- }
-#endif
/*
* Make change persistent in case of an OS crash, both the new entry and
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index b4ce53300b..d74a8dd808 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -129,13 +129,6 @@
#undef HAVE_UNIX_SOCKETS
#endif
-/*
- * Define this if your operating system supports link()
- */
-#if !defined(WIN32) && !defined(__CYGWIN__)
-#define HAVE_WORKING_LINK 1
-#endif
-
/*
* USE_POSIX_FADVISE controls whether Postgres will attempt to use the
* posix_fadvise() kernel call. Usually the automatic configure tests are
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 2085c62b41..8cd125d7df 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -157,7 +157,7 @@ extern void fsync_fname(const char *fname, bool isdir);
extern int fsync_fname_ext(const char *fname, bool isdir, bool
ignore_perm, int elevel);
extern int durable_rename(const char *oldfile, const char *newfile, int
loglevel);
extern int durable_unlink(const char *fname, int loglevel);
-extern int durable_link_or_rename(const char *oldfile, const char
*newfile, int loglevel);
+extern int durable_rename_excl(const char *oldfile, const char *newfile,
int loglevel);
extern void SyncDataDirectory(void);
extern int data_sync_elevel(int elevel);
--
2.25.0
