Date: Thu, 5 May 2022 20:10:02 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, tgl@sss.pgh.pa.us,
	robertmhaas@gmail.com, pgsql-hackers@postgresql.org
Subject: Re: avoid multiple hard links to same WAL file after a crash
Message-ID: <YnOwioN/mo6k1Dtb@paquier.xyz>
References: 
 <CA+TgmobGi7b-5fjAWgEO0ebn516n0OyOi-j7+Hyz_j9sAG17og@mail.gmail.com>
 <289699.1649694527@sss.pgh.pa.us>
 <20220411165257.GB1915258@nathanxps13>
 <20220412.154631.417529439388886590.horikyota.ntt@gmail.com>
 <20220412162742.GB2062175@nathanxps13>
 <Ym6GZbqQdlalSKSG@paquier.xyz>
 <Ym+28lDf5sCJ6pAj@paquier.xyz>
 <20220502173907.GB3351307@nathanxps13>
 <20220502230613.GA3398932@nathanxps13>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="alhZUDhsqkFHkfBR"
Content-Disposition: inline
In-Reply-To: <20220502230613.GA3398932@nathanxps13>
Archived-At: 
 <https://www.postgresql.org/message-id/YnOwioN/mo6k1Dtb%40paquier.xyz>
Precedence: bulk


--alhZUDhsqkFHkfBR
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, May 02, 2022 at 04:06:13PM -0700, Nathan Bossart wrote:
> Here is a new patch set.  For now, I've only removed the file existence
> check in writeTimeLineHistoryFile().  I don't know if I'm totally convinced
> that there isn't a problem here (e.g., due to concurrent .ready file
> creation), but since some platforms have been using rename() for some time,
> I don't know how worried we should be.

That's only about Windows these days, meaning that there is much less
coverage in this code path.

> I thought about adding some kind of
> locking between the WAL receiver and startup processes, but that seems
> excessive.

Agreed.

> Alternatively, we could just fix xlog.c as proposed earlier
> [0].  AFAICT that is the only caller that can experience problems due to
> the multiple-hard-link issue.  All other callers are simply renaming a
> temporary file into place, and the temporary file can be discarded if left
> behind after a crash.

I'd agree with removing all the callers at the end.  pgrename() is
quite robust on Windows, but I'd keep the two checks in
writeTimeLineHistory(), as the logic around findNewestTimeLine() would
consider a past TLI history file as in-use even if we have a crash
just after the file got created in the same path by the same standby,
and the WAL segment init part.  Your patch does that.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmJzsIoACgkQnvQgOdby
QH2feA//bamFbSk8zmQQi7DCHF6KQ1CahvBwpdVbPqsMwAsdVzUBPphQfsE9Terz
bUMSD4dO4uqwkLjOIgwe2Decvsq+yyJBDkC120g69xHtBsiKcOD1+xg/yMiipYCB
dKeDFl6CabbpqZSeLFNBwrcMegnGZ314Cq8MAdobvn0EJhnj+S+0Gk9pkBfuVh62
gWnB5eLiAZhCIG9c40YqvwfnTaFxrdED9ncgV9Ov6tMPEpboTBMuid/7y7f6oeIw
lw9A4ozUg8XFJY9kFaaFjeIsjiP1WYIOnKXFSaV9WSKzEpgF7ygyINjywyD2UmaW
RgaUw1aKC5/KSRUW8y5lkA8vwfFKO5B+QdhlPKuIoQfMRp6P38mbwDBxj8lOUfdc
l5AFmfHGComSlNX02yRCfhWaTBikHC6pt7fjH8NIO8SXanlWVQUa4Ai1ec62grCl
kOJshve6hm3PGNh3HPDx5b1TD/KuaQzCMtbGqa3Yd43F9TzhZ51jVQp8VqZjnEvs
eq0Xr0de49P7aNq6AW1k6j3nrMs/JaysCK5Q4rYHOPLshIqs6/scXGIpEfhzyfS0
i4QtEiXVhwsbZxd3LIKdqCafY19mnksW9V1caJJfS/70eU9E5r5CHn65A74mcETh
/s/9utomrwkenLBOVx5AZ/TzZhuYOUXVTH1dXO3iQddyo1qFYrU=
=fWrG
-----END PGP SIGNATURE-----

--alhZUDhsqkFHkfBR--