Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 19 Feb 2026 13:06:52 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Jeff Davis <pgsql@j-davis.com>,
	Greg Sabino Mullane <htamfids@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Adding locks statistics
Message-ID: <aZaMXK90sRjSQBJP@paquier.xyz>
References: <1a236172c7dda72939e4293657a90536cce7dd16.camel@j-davis.com>
 <aJwMi6qWKNc/ygg9@ip-10-97-1-34.eu-west-3.compute.internal>
 <aUA7ymkrh/UpkJ6O@ip-10-97-1-34.eu-west-3.compute.internal>
 <aYreqqn3meYqPm1A@ip-10-97-1-34.eu-west-3.compute.internal>
 <aY7UmTIiBScP4rAq@paquier.xyz>
 <aY779Gu3rQwJ4Erq@ip-10-97-1-34.eu-west-3.compute.internal>
 <aY-MRFnzTDJO265s@alap3.anarazel.de>
 <aZLtDbGEqSN2MuRb@ip-10-97-1-34.eu-west-3.compute.internal>
 <otjn7ykaezlueayyp7fppggkldw23ykh3i6ite2rqq4xh7dwvl@w5dvqqgvqf25>
 <aZSYcsRmqh7grqJb@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="+jFt4TJQcTD34qRn"
Content-Disposition: inline
In-Reply-To: <aZSYcsRmqh7grqJb@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: <https://www.postgresql.org/message-id/aZaMXK90sRjSQBJP%40paquier.xyz>
Precedence: bulk


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

On Tue, Feb 17, 2026 at 04:33:54PM +0000, Bertrand Drouvot wrote:
> Okay, done that way in the attached. To avoid overhead due to timing as m=
uch as
> possible, the patch simply relies on log_lock_waits and deadlock_timeout.=
 It means
> that it relies on the existing code, and increments waits and wait_time o=
nly if
> log_lock_waits is on and if the session waited longer than deadlock_timeo=
ut.
>=20
> I did not want to dissociate the waits and wait_time increments so that t=
heir
> ratio could still make sense.
>=20
> That sounds like a good compromise, thoughts?

             else if (myWaitStatus =3D=3D PROC_WAIT_STATUS_OK)
+            {
+                /* Increment the lock statistics counters */
+                pgstat_count_lock_waits(locallock->tag.lock.locktag_type);
+                pgstat_count_lock_wait_time(locallock->tag.lock.locktag_ty=
pe, msecs);

Not sure that it makes much sense to me to rely on log_lock_waits
being enabled to decide if this count and this time are aggregated.
The log information and the stats gathering are two separate things.
Wouldn't it make more sense to call pgstat_count_lock_waits() outside
of this code path, when we know myWaitStatus?

While relying on the time calculating for the logs data is a good
idea, it seems to me that we should have a separate GUC to enable this
number, like a new track_lock_timings?  If track_lock_timings or
log_lock_waits is enabled, we should calculate the time difference.
All these decisions also depends on what deadlock_state holds on top
of myWaitStatus, I guess..
--
Michael

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

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

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmmWjFwACgkQnvQgOdby
QH26Wg/9GPmgeOlLC1Nni8sAqgMKlOJ2fq3OafSgRCXTZXkiySoaJrKcAakVWdqx
/ZDvtGbWJQWc+nuNjQWH/2fvs/4FBbgwy4nPhO+5c0V3215d45eVB2JZTJG9aOKr
oRfSzce2msnWWLh3z+LqC5DIJCgZxH5WkT/8SmaxX6N9MQJpTB0z4n4BN6TpF+Oe
C7w0VfdoNd1zbtFISVcdUnVoP4bcWW/Nb5BueXSdJ9u/BdDFK3EruILQlPBCFv00
udLdjpbHEwKWEF/PVuLbMZ3AT705zhFq6J9fiBCKI53Gz+AiEPFAcBJWW2mRdfzz
L8e/HtscDrVNTBwYzhZa7TbBmgK/JSNQsBRBQKsjsvUJR5maxykyw/Dlwe/YGre6
2LryqVi6GkwtE4SK0lIXK777vF5ER/jPbMDH38zVk8HXMcrFFySYwwtF3u1IFAu1
y8VFeB7lOAXMz/e9EgQnzsIAvADbxOtUBm8rzD3Bo64559Rg/IyP+CGuAOmJFL1f
cq7ZAeAwvpT8fA5smm0pVmqfGtoI+ims5M6oNbMkGa6kJ/2nHV9T6P//4nYa5exA
ZqgWmV/VRRGNSTpM6MdoqDON1RIbig+d/sk8kak5Tj8hSvKccRtnIj/iEWI6cSDa
RjF4BtAs8smLztx5apBsR6KjKON6jiW8CZzQUiVlS6n+xUL7GHM=
=99+w
-----END PGP SIGNATURE-----

--+jFt4TJQcTD34qRn--