Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 6 Mar 2024 14:47:55 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	pgsql-hackers@lists.postgresql.org, exclusion@gmail.com
Subject: Re: Fix race condition in InvalidatePossiblyObsoleteSlot()
Message-ID: <ZegDi9A54TPaIRFp@paquier.xyz>
References: <Zc30i8fB7VYrTVw+@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZdLxr2CPpPVNh9Zi@paquier.xyz>
 <CALj2ACUMPTvuk0_GuPiFbUb=PMTQq0WMi5Kvj8Mz0JZ_ico1Lw@mail.gmail.com>
 <ZdMkJAaF7HcGutE2@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZdPpdYBIrDqEl25c@paquier.xyz>
 <ZdQ5uDWxzu9O1T03@paquier.xyz>
 <ZdTNafYSxwnKNIhj@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZdyZya4YrNapWKqz@ip-10-97-1-34.eu-west-3.compute.internal>
 <ZeZqbHjLrc8hkIvu@paquier.xyz>
 <ZebxH6YJ8mfB5rJ6@ip-10-97-1-34.eu-west-3.compute.internal>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="P+BWhkz4qHrqiyai"
Content-Disposition: inline
In-Reply-To: <ZebxH6YJ8mfB5rJ6@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: 
 <https://www.postgresql.org/message-id/ZegDi9A54TPaIRFp%40paquier.xyz>
Precedence: bulk


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

On Tue, Mar 05, 2024 at 10:17:03AM +0000, Bertrand Drouvot wrote:
> On Tue, Mar 05, 2024 at 09:42:20AM +0900, Michael Paquier wrote:
>> First, in a build where 818fefd8fd is included, this makes the test
>> script a lot slower.  Most of the logic is quick, but we're spending
>> 10s or so checking that catalog_xmin has advanced.  Could it be
>> possible to make that faster?
>=20
> Yeah, v2 attached changes this. It moves the injection point after the pr=
ocess
> has been killed so that another process can decode at wish (without the n=
eed
> to wait for a walsender timeout) to reach LogicalConfirmReceivedLocation(=
).

Ah, OK.  Indeed that's much faster this way.

> I try to simulate a revert of 818fefd8fd (replacing "!terminated" by "1 =
=3D=3D 1"
> before the initial_* assignements).

Yeah.  I can see how this messes up with the calculation of the
conditions, which is enough from my perspective, even if we don't have
any sanity checks in 818fefd8fd originally.

> The issue is that then the new ASSERT is
> triggered leading to the standby shutdown. So, I'm not sure how to improv=
e this
> case.

It's been mentioned recently that we are not good at detecting crashes
in the TAP tests.  I am wondering if we should check the status of the
node in the most popular routines of Cluster.pm and die hard, as one
way to make the tests more responsive..  A topic for a different
thread, for sure.

> Done in v2.

Reworded a few things and applied this version.
--
Michael

--P+BWhkz4qHrqiyai
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmXoA4sACgkQnvQgOdby
QH3fMA/7B27pCEY3pPwMYbFvxIiVgH/FIhiDkAiSZLIrUcSFuHwZ6QRVpdmbWh+u
rK8yzygY8QfadIa13l34n5eqcO3Vgmsd2Xx1GJ1JAxRsgV5L/E1mJnPuCg/nWxPV
TQ27kmjUMpvPK+ikZWEQlv8lBGj2h8pWtVrLqLH99mTaTFtZPya6Y0jeSrgyCmlM
nCdu6Zuy34cLZIWM9ONQ6GrB1NK1DkJuvdrVYAMkV7gzvvdUuEOkkJD6/co/Vc/g
ZqpVn88Qq1nazedoHDIkIVHp7EXoy0ucI+TrGZWZij0O2sSoj3eGWcaQhRG4fv0t
n2PzfpDHCksgIpBAicVvETlK331HQFPSmRuJNiw9D4LgHH2cBkqG0uaJsSz9CEns
6lv+WsX3Oh9R3x7gPe/V6gv63TZY73dNd+AyR8M5VnbNzvdYUD6JuDgmrPOF/8cw
bOcbLvg13nWA8odBFvd9+rNnuhp3jRlxjyvwCU59cZvfatVMsaH4wSAZ+hMZsMZu
Mq5uYogR291jcQvkTTrat1zdugRJzRvktpVNfZ9HmL7v7Pfbv2aWPvuVYdCRQSrK
+H3Ox43yPVEMorToiBUQT2c5VCWpUbVng3tf4HwZYeDoRzqFCRGxc49p0EkJTn/2
qxDyKbSkADcsBtsSlg2GH38CG63JCqKjxD4k653FJWAjbRAUCrE=
=bxqC
-----END PGP SIGNATURE-----

--P+BWhkz4qHrqiyai--