Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 25 Mar 2026 08:46:23 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>,
	Jeff Davis <pgsql@j-davis.com>,
	Greg Sabino Mullane <htamfids@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Adding locks statistics
Message-ID: <acMiT69EEGKHWGM5@paquier.xyz>
References: <aZiZTX1l5pX+BOXn@ip-10-97-1-34.eu-west-3.compute.internal>
 <aaEoN1XexvGiFvYp@ip-10-97-1-34.eu-west-3.compute.internal>
 <abortN3ngqMcaBJ7@ip-10-97-1-34.eu-west-3.compute.internal>
 <abpfH33mR4Mp9dtO@paquier.xyz>
 <abq71eL7qIdWAxMD@ip-10-97-1-34.eu-west-3.compute.internal>
 <abufUya2oK-_PJ3E@paquier.xyz>
 <abvrRZo52Yx9ZzWQ@ip-10-97-1-34.eu-west-3.compute.internal>
 <ab4y1UQUsSS7bSVd@paquier.xyz>
 <acI3H5aucJNS2w78@paquier.xyz>
 <hlkdrplgrmudbspibsuq6xooxrqxqsgwo6x5b6x5ptvkgjbe7w@xogt6xgua6dz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="+zaYSPkVvBWW9eqL"
Content-Disposition: inline
In-Reply-To: <hlkdrplgrmudbspibsuq6xooxrqxqsgwo6x5b6x5ptvkgjbe7w@xogt6xgua6dz>
Archived-At: 
 <https://www.postgresql.org/message-id/acMiT69EEGKHWGM5%40paquier.xyz>
Precedence: bulk


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

On Tue, Mar 24, 2026 at 04:09:37PM -0400, Andres Freund wrote:
> The test is extremely unstable on windows.  On CI 10/16 runs since the test in
> failed due to it, afaict.

I am not surprised by that.  Windows is good in catching race
conditions.

> I don't see how a test with a timeout setting that's anywhere remotely close
> to 10ms could be expected to be stable.

Well, the low value of deadlock_timeout is not the problem.  The
shorter the better to make the test go faster with more deadlock
checks.  What the buildfarm is telling is that we do not have the time
to process the deadlock_timeout request, and that we would need to pay
the cost of longer sleep if coded this way.  This is going to cost in
runtime on fast machines where the CheckDeadLock() call would happen
quickly. And on slow machines, we don't have the guarantee that the
sleep would be long enough to process the deadlock request.

This test would be better if rewritten as a TAP test, I guess, with a
NOTICE injection point before the CheckDeadLock() call in ProcSleep()
to make sure that the second session processes the deadlock timeout
request while it is waiting on its lock to be acquired.  One trickier
part is that we only care about the deadlock_timeout in s2, because we
want to measure the wait it has waited until the lock could be
acquired, meaning that we should make s1 use a large deadlock_timeout
to avoid interferences with a global injpoint.

I don't have the credits to test that in the CI for this month,
unfortunately, and this creates noise in the CI for the work of other
folks in this release cycle, so I am going to remove this test for
now.

> Also, anything that requires short sleeps (like pg_sleep(0.05);) is extremely
> likely to be a long time test stability hazard. It's a huge "test smell" to
> me, to the point that I think every single sleep in a test needs a comment
> explaining why that one use of sleep is correct, and that comment better be
> signed in blood.

Yep, agreed.
--
Michael

--+zaYSPkVvBWW9eqL
Content-Type: application/pgp-signature; name=signature.asc

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

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmnDIk8ACgkQnvQgOdby
QH2hSQ//X+3U4SJlKj5hKSCTF81C1cn6A7vegdLLSVqSJr/3QbaOgqTTjUj1aqnF
w4bklLLJo+EFunh1FzpUKkqC1RMEzOU5TJD1f8z8wk6/EM9oR4un9DXa9Z0trUzP
6vsQB4Mw3JgWOPTR7aaWb6mDXvMmOEt/eFGu2CCzXmaVDpS3dHLbZsOuLnQqC40q
73NcDU1LSDSM2WPm8iS7ULTE3ckbwVhOXceUVtOrN3Sz7GyKgC/ItVSLit6nV5HM
SwPoAEdknwwl/Ky62HrEIGoH9y+gdHQtPUx5QV4OLSBmZofJsB7txP6nzNu7znVg
N43McUYKuLuz79IESMImrf3NCYBf8EOXmWJBJ3YleMJMvaZ/GAONI59L1OdN+nv2
kMXP7TTnO3mrrQ+Jn92OCmSZAMDX1GI+gooZWt2qlyi0uAruR8a3QMbZUMccvluq
7ioGjjMU1pDFeCFlkSylQU5jrcj0wFl0sczRQfXvXRU5gz0zCAxgm1+dJDkLSNCc
fk3Xq8wxuwbo3ZyEzDc59TJ86uTqnWGnoMJqday7NVQBPVbDy1Pgp/it7HF+xHsX
5hi+Ie9tFAun8fSloORnn1ixaZ4NjGron6JRrGPMFpWTZLyt4qzXUsraB4qprUuI
fc+XRBa+tN4nTYVgseSmD6P+nK9QYO3lJUommOfAwx19D4ik+fc=
=eRe7
-----END PGP SIGNATURE-----

--+zaYSPkVvBWW9eqL--