Обсуждение: HAVE_WORKING_LINK still needed?
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. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
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] is quite old, and is automatically redirected to the current documentation [2]. Maybe this patch should use the new path.
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