Date: Sat, 20 Mar 2021 07:32:43 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Magnus Hagander <magnus@hagander.net>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
	=?utf-8?B?0K/RgNC+0YHQu9Cw0LIg0J/QsNGI0LjQvdGB0LrQuNC5?=
 <yarik97.6@gmail.com>,
	Postgres bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #16927: Postgres can`t access WAL files
Message-ID: <YFUmi3diUlqyoL44@paquier.xyz>
References: 
 <CADLmTo+AOq2UE3Gr22omfcxhZajqkeGYgWFowKNScOZA1fWVFA@mail.gmail.com>
 <YFM2MRbHZGQ4DsG/@paquier.xyz>
 <CADLmToJJykdGUdKiPkut4mq8oZJ8z4+8d5aSUU9zCtey7yu_9g@mail.gmail.com>
 <YFPMl3TUPXbN127E@paquier.xyz>
 <CADLmTo+GoZ1tuyZxx5uprqSkwqqw3yajry36o-+=N=Jqe=0oZA@mail.gmail.com>
 <YFRglwF+j2RMmbXF@paquier.xyz>
 <CADLmTo+KJoh9+53=7UMfDQALzDEZWXYEUuv3k9YFT4Mo7+0NWQ@mail.gmail.com>
 <YFSMsxc8w2eqUHHR@paquier.xyz>
 <4065519.1616166832@sss.pgh.pa.us>
 <CABUevExwJrT9tAZz0gJKoSc3qLhg3NGsKJH7QWj4BtEK=JHChg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="dsOAZ2Tqn+6oU6jh"
Content-Disposition: inline
In-Reply-To: 
 <CABUevExwJrT9tAZz0gJKoSc3qLhg3NGsKJH7QWj4BtEK=JHChg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YFUmi3diUlqyoL44%40paquier.xyz>
Precedence: bulk


--dsOAZ2Tqn+6oU6jh
Content-Type: multipart/mixed; boundary="sfeWsZ5gKWiB1x63"
Content-Disposition: inline


--sfeWsZ5gKWiB1x63
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 19, 2021 at 04:19:50PM +0100, Magnus Hagander wrote:
> On Fri, Mar 19, 2021 at 4:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Given the evidence that there's a problem, I agree with reverting
>> that.  I'd suggest keeping the cosmetic rename of the function,
>> but we have to put back the Windows-doesn't-HAVE_WORKING_LINK logic.
>=20
> +1. I think the indications are definitely clear enough that this has
> to go back in.

No problem from me to keep the rename, and so this leads to the simple
patch attached, then.  Any comments?

>> Grepping in the v12 branch, I find a second use of HAVE_WORKING_LINK
>> in contrib/pg_standby.  But that seems to be in a non-WIN32 code path,
>> so I don't think putting that back is necessary.
>=20
> .. and apart front aht I *really* doubt that one has many users,
> especially on Windows :)

Yeah, agreed.
--
Michael

--sfeWsZ5gKWiB1x63
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="win32-link.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index f10ad0acd6..e28c990382 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -135,6 +135,13 @@
 #define EXEC_BACKEND
 #endif
=20
+/*
+ * 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/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 110ba31517..92b1959648 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -820,6 +820,7 @@ durable_rename_excl(const char *oldfile, const char *ne=
wfile, int elevel)
 	if (fsync_fname_ext(oldfile, false, false, elevel) !=3D 0)
 		return -1;
=20
+#ifdef HAVE_WORKING_LINK
 	if (link(oldfile, newfile) < 0)
 	{
 		ereport(elevel,
@@ -829,6 +830,17 @@ durable_rename_excl(const char *oldfile, const char *n=
ewfile, 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
=20
 	/*
 	 * Make change persistent in case of an OS crash, both the new entry and

--sfeWsZ5gKWiB1x63--

--dsOAZ2Tqn+6oU6jh
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmBVJosACgkQnvQgOdby
QH3arxAAp83CrNTRrAhj3YBqSazLn4IF5e740c0+GzmzVpt4JGPRCjIcFuYCcJb0
4LlK2QmOi4lcGEu1ivpwZzyrVBsT2SOJ5jJAGvISGo8e2tkeSnFUVuFZcxkYtVNy
qvZQkqKl4SqnBVnp9HgKtjXRtDyy1vOK9WvNg2zoEOaMTgRVAY/XRDnhQBzsJt3v
F1oq1Iz3a2fTwiTodu/ILnq13W5NsSFTglTXdMIDPqN1phbE3sdpPK6cSpSpi+64
cVKsmv8mcgMx05T5gsehh6IRvjNShOL+gJuORtwscA/HZoOM3YIaPEURXZTDcve8
bmtU6b+Co81p1okBSGEyxOprFekpnSgf/9okBUp2fBw5aTqDbf5ZiQIZ55CyQnxS
rf4R+/+XjCMFsURTU1p5t2vLS8AR+1T4cc4ZD1CA1SIVIzrdheD91odE1+aLG/Ig
n98HJhHiUXQjNC+RvaXAgNkuNJPvUOlBq/4INFTkUGA0CydfYs9uATmKxVnc5ohf
oxx6GFXy33ezRexGaaFqlODhwLVbt2gFv7C8kjqICCyLLK+BYikJ8DhE32XSqmiz
MtrYT3qRrKI81CFza9Pd05m06hf86vX58tJqNhIAY/rsZFQJRh15gUnm5bHEaFps
cpEMhNeSRNZVSemOUsQC1D3vzzOrMF+mE0RSHIW5zo0YjoBUliU=
=lUDy
-----END PGP SIGNATURE-----

--dsOAZ2Tqn+6oU6jh--