HAVE_WORKING_LINK still needed?

On 2020-Feb-28, Peter Eisentraut wrote:

@@ -788,7 +788,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 +797,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)

Maybe rename durable_link_or_rename to just durable_link?

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, Feb 28, 2020 at 2:15 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

I came across the HAVE_WORKING_LINK define in pg_config_manual.h.
AFAICT, hard links are supported on Windows and Cygwin in the OS
versions that we support, and pg_upgrade already contains the required
shim. It seems to me we could normalize and simplify that, as in the
attached patches. (Perhaps rename durable_link_or_rename() then.) I
successfully tested on MSVC, MinGW, and Cygwin.

The link referenced in the comments of win32_pghardlink() [1]http://msdn.microsoft.com/en-us/library/aa363860(VS.85).aspx is quite old,
and is automatically redirected to the current documentation [2]https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createhardlinka. Maybe
this patch should use the new path.

[1]: http://msdn.microsoft.com/en-us/library/aa363860(VS.85).aspx
[2]: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createhardlinka
https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-createhardlinka

Regards,

Juan José Santamaría Flecha

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

I came across the HAVE_WORKING_LINK define in pg_config_manual.h.
AFAICT, hard links are supported on Windows and Cygwin in the OS
versions that we support, and pg_upgrade already contains the required
shim. It seems to me we could normalize and simplify that, as in the
attached patches. (Perhaps rename durable_link_or_rename() then.) I
successfully tested on MSVC, MinGW, and Cygwin.

I don't have any way to test on Windows, but this patchset passes
eyeball review. +1 for getting rid of the special cases.
Also +1 for s/durable_link_or_rename/durable_link/.

regards, tom lane

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.

There's a lot of commonality between the two. Perhaps it's not entirely
silly to merge both as a single routine, with a flag to select either
behavior.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On 2020-03-04 17:37, Peter Eisentraut wrote:

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).

committed like that

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services