Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 24 Mar 2026 16:02:55 +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: <acI3H5aucJNS2w78@paquier.xyz>
References: <aZgBTyzxWmPJL0Tt@ip-10-97-1-34.eu-west-3.compute.internal>
 <isi5wszczhusgjetxwe6khxa5mbm5jms3msmbhwl73jktxw7ic@3mympoumua76>
 <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>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="SRiiXhd6YS+PYxRu"
Content-Disposition: inline
In-Reply-To: <ab4y1UQUsSS7bSVd@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/acI3H5aucJNS2w78%40paquier.xyz>
Precedence: bulk


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

On Sat, Mar 21, 2026 at 02:55:33PM +0900, Michael Paquier wrote:
> On Thu, Mar 19, 2026 at 12:25:41PM +0000, Bertrand Drouvot wrote:
>> I did not check if there are any other files that could benefit of using
>> locktag.h instead of lock.h but that's something I'll do and open a dedi=
cated
>> if any (once locktag.h is in the tree).
>=20
> I have checked after that, and did not spot an area (except your patch
> of course).  And applied this part.

The main patch has some churn in proc.c and lock.c, moving some code
blocks while the main focus of the patch is to add the two pgstat()
calls to report some data, so I have moved this part into its own
commit, and applied it.  One thing to not in lock.c: we will calculate
the time difference of the wait even if log_lock_waits is disabled,
without the lock stats part.  As this GUC is enabled by default, it
does not matter much in practice, I guess, and it matters even less
with the main patch merged.

The implementation of the main patch is close enough to pgstat_io.c
that your logic is a no-brainer (timestamp protected by the first
LWLock, each field incremented depending on locktags, etc.).  Instead
of a boolean flag tracking if some stats have been set, we could also
have used an approach like the checkpointer stats with is_all_zeros on
the pending data, perhaps?  At the end, with two incrementation
routines, I've let the code as-is.

Another thing I am not completely sure is if the sleep time of the
isolation tests is long enough.  I have tested things with
CLOBBER_CACHE_ALWAYS to make the setup more sensitive to timings but
could not get it to fail.  We'll know soon enough if the buildfarm
complains.

After a few more tweaks here and there (code, comments, some
beautification), done.
--
Michael

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

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

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmnCNx8ACgkQnvQgOdby
QH3Gog/8DhRo5Ghoz6FXTxOgtHCN4+aYEItA0sZQGdYnvu+Scc7/3pt9Ddl4kimp
6GFdMHHChZ/dl4fi5Zw3rxVzI5lzVNb16tf9Wh9gorTX/GKUmcJbcnxXfkJ25Vxo
1gLqRfZUCLCds2DoZPKs2RfrdW5ekcntG8UBKtT9sKOXouIZWgkzVmDtnKO7q8ne
cUVoKfXVIqg6QJsq5DvQlyc60jkSVqF1omvAMzbSISyxSMd6LAY/GGUOOho1aaYg
uHgm6AJx78yTi6Ysw9VTVEA75k6njHgAFM4b+C05oPntcCmcwxYqBjVL/vULNN8R
mDkzzRIs+2MtRxBfN31SxAhL/OXHiiWl1JJIyabJ3uQSxxRlbZBhZ6n+qWW2gJ5I
OJxdOVagkfOYJQgJMMcXw4k9A94AQXtKovLAaBfQ7FpoyyzyZu4zcZ2rR69BSTLD
QeCTCRXx5+XNtvLagWockIQX40zZ2nROoAvYeTsHQckC2pWnpjSAya0UPLYmr65N
df/A3uhGNsOp0FHR8JBr2SOePa935tWFGhtyGf2ygyVgwt0/Uqb/YDdYY5pXgsTH
i6WHkJcj1cxq26Ec3/NjbPbKzq+phIWp6TnUVv6F8jkUy2a97CX+C41acfWdjaGh
9BYT2m7slTKOj5BAOefceScaosGByJ7H28tHLfdmWtd1jPoY320=
=Uwhd
-----END PGP SIGNATURE-----

--SRiiXhd6YS+PYxRu--