Date: Wed, 17 Mar 2021 07:30:00 +0900
From: Michael Paquier <michael@paquier.xyz>
To: =?utf-8?B?0K/RgNC+0YHQu9Cw0LIg0J/QsNGI0LjQvdGB0LrQuNC5?=
 <yarik97.6@gmail.com>
Cc: Postgres bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #16927: Postgres can`t access WAL files
Message-ID: <YFExaMohbb8tqM3k@paquier.xyz>
References: <16927-67c570d968c99567@postgresql.org>
 <YFAH1LbeUeXTFcEM@paquier.xyz>
 <CADLmToLD6a=eXvh+b6u+Ju5Bm2ZvtCHBnCqY_d8U9we+315HZg@mail.gmail.com>
 <YFBu8b4dH32kQwDG@paquier.xyz>
 <CADLmTo+Kb0EzCFH1yYdjKneQYG=OsEX4=naAjaHOAeeNmY=HLA@mail.gmail.com>
 <YFCF+JbyfObn5zPu@paquier.xyz>
 <CADLmTo+gkFJ9or_Mfp6KKEj9MAip_BgHG6A9mzxR1DMpuRp3fg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="fC8U0n/jXYphDPI0"
Content-Disposition: inline
In-Reply-To: 
 <CADLmTo+gkFJ9or_Mfp6KKEj9MAip_BgHG6A9mzxR1DMpuRp3fg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YFExaMohbb8tqM3k%40paquier.xyz>
Precedence: bulk


--fC8U0n/jXYphDPI0
Content-Type: multipart/mixed; boundary="P4CG9I4HkKcLBrFW"
Content-Disposition: inline


--P4CG9I4HkKcLBrFW
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

On Tue, Mar 16, 2021 at 12:40:15PM +0200, =D0=AF=D1=80=D0=BE=D1=81=D0=BB=D0=
=B0=D0=B2 =D0=9F=D0=B0=D1=88=D0=B8=D0=BD=D1=81=D0=BA=D0=B8=D0=B9 wrote:
> On prod server with windows server 2019 I used .zip downloaded from
> https://www.enterprisedb.com/download-postgresql-binaries and unpacked it
> to the default folder (Program Files/PostgreSQL/13). But the previous
> version (9.6) was installed via .exe installer.
> On developer server with Windows server 2016 I used an installer to insta=
ll
> postgres 13, but as I said before there are the same issues on both serve=
rs.
> Are you talking about building patch from scratch? If yes, I`ll try it.

Yes, that's the idea, and it is an experience by itself to compile
the Postgres source code on Windows :)

We'd need to check after two things:
1) The code compiled from the source code of 13.2 is still able to
reproduce the issue.
2) Once the patch attached is applied on top of 13.2, check if the
problem goes away or not.

I am still running some tests on my own environments, but that's much
harder to hit for me, visibly, and the error code path complaining is
not the same.  If I may ask, what are the contents of pg_wal on the
instances where the errors happen.  Do you have some files suffixed
with ".deleted" around, or anything named like xlogtemp.N, where N is
an integer for a PID?

By the way, could you hit "reply-all" for the emails or attach
properly in CC pgsql-bugs so as everybody can see the discussion
happening here?
--
Michael

--P4CG9I4HkKcLBrFW
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="0001-Revert-Remove-HAVE_WORKING_LINK.patch"
Content-Transfer-Encoding: quoted-printable

=46rom 961f9a03d4c27220c33e88402d5ef274424a0ab2 Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Wed, 17 Mar 2021 07:12:35 +0900
Subject: [PATCH] Revert "Remove HAVE_WORKING_LINK"

This reverts commit aaa3aeddee51dd0058d38469907865052706a590.
---
 src/include/pg_config_manual.h        |  7 +++++++
 src/include/storage/fd.h              |  2 +-
 src/backend/access/transam/timeline.c |  4 ++--
 src/backend/access/transam/xlog.c     |  4 ++--
 src/backend/storage/file/fd.c         | 21 ++++++++++++++++-----
 5 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 8f3ec6bde1..966da99742 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/include/storage/fd.h b/src/include/storage/fd.h
index 8cd125d7df..2085c62b41 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 lo=
glevel);
 extern int	durable_unlink(const char *fname, int loglevel);
-extern int	durable_rename_excl(const char *oldfile, const char *newfile, i=
nt loglevel);
+extern int	durable_link_or_rename(const char *oldfile, const char *newfile=
, int loglevel);
 extern void SyncDataDirectory(void);
 extern int	data_sync_elevel(int elevel);
=20
diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/tra=
nsam/timeline.c
index e6a29d9a9b..27d70ff869 100644
--- a/src/backend/access/transam/timeline.c
+++ b/src/backend/access/transam/timeline.c
@@ -446,7 +446,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID pare=
ntTLI,
 	 * Perform the rename using link if available, paranoidly trying to avoid
 	 * overwriting an existing file (there shouldn't be one).
 	 */
-	durable_rename_excl(tmppath, path, ERROR);
+	durable_link_or_rename(tmppath, path, ERROR);
=20
 	/* The history file can be archived immediately. */
 	if (XLogArchivingActive())
@@ -524,7 +524,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_rename_excl(tmppath, path, ERROR);
+	durable_link_or_rename(tmppath, path, ERROR);
 }
=20
 /*
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam=
/xlog.c
index 7daa7c43ad..82e070e431 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3624,11 +3624,11 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmpp=
ath,
 	 * Perform the rename using link if available, paranoidly trying to avoid
 	 * overwriting an existing file (there shouldn't be one).
 	 */
-	if (durable_rename_excl(tmppath, path, LOG) !=3D 0)
+	if (durable_link_or_rename(tmppath, path, LOG) !=3D 0)
 	{
 		if (use_lock)
 			LWLockRelease(ControlFileLock);
-		/* durable_rename_excl already emitted log message */
+		/* durable_link_or_rename already emitted log message */
 		return false;
 	}
=20
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index e5950b0726..ba92ceaf65 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -767,11 +767,10 @@ durable_unlink(const char *fname, int elevel)
 }
=20
 /*
- * durable_rename_excl -- rename a file in a durable manner, without
- * overwriting an existing target file
+ * durable_link_or_rename -- rename a file in a durable manner.
  *
- * Similar to durable_rename(), except that this routine will fail if the
- * target file already exists.
+ * Similar to durable_rename(), except that this routine tries (but does n=
ot
+ * guarantee) not to overwrite the target file.
  *
  * Note that a crash in an unfortunate moment can leave you with two links=
 to
  * the target file.
@@ -782,7 +781,7 @@ durable_unlink(const char *fname, int elevel)
  * valid upon return.
  */
 int
-durable_rename_excl(const char *oldfile, const char *newfile, int elevel)
+durable_link_or_rename(const char *oldfile, const char *newfile, int eleve=
l)
 {
 	/*
 	 * Ensure that, if we crash directly after the rename/link, a file with
@@ -791,6 +790,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,
@@ -800,6 +800,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
--=20
2.30.2


--P4CG9I4HkKcLBrFW--

--fC8U0n/jXYphDPI0
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmBRMWgACgkQnvQgOdby
QH2pNA//bajWK69eQcTThtJ7eQskYEAkmEfQ7Xmrf9VC+sorn0TKhMsT3nqa2/Eo
C7I56WFIFdOJntW03Y4fMv9Bilpx9XtbrizQErZDgfYlRXgs8V/9bkSOq8yT8c21
L4O6CLDQVI2lL4vWFW5kIQSkuBdJhAj61mTYCEQiCcJuDCSmot/H0gv7ROS2coKT
C/RYmHdIJ2hrzyoW0S6+JMiC3SETKZ+u6rr59QZV14btRW4FWtM+Fx7gGiutrgxZ
xKF5XS7q0DPEefAqrdmkQNDhP4lN9qIHZEoRNYKjH2gwEXMKlK5uD0H+sJXut/6I
/1Jc/VXEcRXKbFlj9Dj5NVGnHGmGh2E19QZxaaWgdxEUcBQ50g/HrzdIo3Z8wYU3
cCN+Rf5QFxQL+TaAMUIY77PBlZsxhWfxRbnkNzNQ/BomFh6CHniU6wzQJyQ0GN2/
0zvhJgun8rSkPiRsf0oST+Ivvo3xaMA6OYqnMVFuwWvnXDhybVlr1FeJXp0kzalz
SbRLcDaKai3XVybbrvxJZdsxJdk/M5bduvvcej7s23JvXoUD5as+8wNClWUHI+BQ
9vtY4p0hRS4AZqi8sJKi30Yo3OtcmEqp6TvGpQtAXT9XQPbdehBrAz+/6LHWRpdc
oj+wcRTMgOUWFx77IqMZiMr2oI69Q2hcZ3+cOd1NfH7pUBdQ2lU=
=Qyj2
-----END PGP SIGNATURE-----

--fC8U0n/jXYphDPI0--