MIME-Version: 1.0
References: <aIyNxBWFCybgBZBS@ip-10-97-1-34.eu-west-3.compute.internal>
In-Reply-To: <aIyNxBWFCybgBZBS@ip-10-97-1-34.eu-west-3.compute.internal>
From: Greg Sabino Mullane <htamfids@gmail.com>
Date: Mon, 11 Aug 2025 13:54:38 -0400
Message-ID: 
 <CAKAnmm+USTr5MeKuyrAvbCjHqbSFbZ+czJsV7fPYt9nECQkf-A@mail.gmail.com>
Subject: Re: Adding locks statistics
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Content-Type: multipart/alternative; boundary="000000000000123b8e063c1aa0fe"
Archived-At: 
 <https://www.postgresql.org/message-id/CAKAnmm%2BUSTr5MeKuyrAvbCjHqbSFbZ%2BczJsV7fPYt9nECQkf-A%40mail.gmail.com>
Precedence: bulk

--000000000000123b8e063c1aa0fe
Content-Type: text/plain; charset="UTF-8"

Great idea. +1. Here is a quick overall review to get things started.

Meta:
patch did not apply via "git apply". Also has carriage returns (e.g. DOS
file), and some errant whitespace. Seems to pass pgindent, though.

Name:
I think the name would read better as pg_stat_locks, especially as it
returns multiple rows.

Docs: seem good. Needs a section on how to reset via
SELECT pg_stat_reset_shared('lock');
Also plural better here ('locks')

Code:

+ * Copyright (c) 2021-2025, PostgreSQL Global Development Group

If a new file, we can simply say 2025?

+ LWLock locks[LOCKTAG_LAST_TYPE + 1];
+ PgStat_Lock stats;

Adding a lock to the system that counts locks! :) (just found amusing, not
a comment)

-#define PGSTAT_KIND_SLRU 11
-#define PGSTAT_KIND_WAL 12
+#define PGSTAT_KIND_LOCK 11
+#define PGSTAT_KIND_SLRU 12
+#define PGSTAT_KIND_WAL 13

Why not just add LOCK as #13?

What about the overhead of maintaining this system? I know it's overall
very lightweight, but from my testing, the relation locktype in particular
is very, very busy. Busier than I realized until I saw it in this useful
view!

Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

--000000000000123b8e063c1aa0fe
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Great idea.=C2=A0+1.=C2=A0Here is a quick=
 overall review to get things started.<br><br>Meta:<br>patch did not apply =
via &quot;git apply&quot;. Also has carriage returns (e.g. DOS file), and s=
ome errant whitespace. Seems to pass pgindent, though.<br><br>Name:<br>I th=
ink the name would read better as pg_stat_locks, especially as it returns m=
ultiple rows.<br><br>Docs: seem good. Needs a section on how to reset via <=
br>SELECT pg_stat_reset_shared(&#39;lock&#39;);<br>Also plural better here =
(&#39;locks&#39;)<br><br>Code:<br><br>+ * Copyright (c) 2021-2025, PostgreS=
QL Global Development Group<br><br>If a new file, we can simply say 2025?<b=
r><br>+	LWLock		locks[LOCKTAG_LAST_TYPE + 1];<br>+	PgStat_Lock stats;<br><b=
r>Adding a lock to the system that counts locks! :) (just found amusing, no=
t a comment)<br><br>-#define PGSTAT_KIND_SLRU	11<br>-#define PGSTAT_KIND_WA=
L	12<br>+#define PGSTAT_KIND_LOCK	11<br>+#define PGSTAT_KIND_SLRU	12<br>+#d=
efine PGSTAT_KIND_WAL	13<br><br>Why not just add LOCK as #13?<br><br>What a=
bout the overhead of maintaining this system? I know it&#39;s overall very =
lightweight, but from my testing, the relation locktype in particular is ve=
ry, very busy. Busier than I realized until I saw it in this useful view!<b=
r><br></div><div dir=3D"ltr" class=3D"gmail_signature"><div dir=3D"ltr"><di=
v>Cheers,</div><div>Greg</div><div><br></div><div>--</div><div>Crunchy Data=
 - <a href=3D"https://www.crunchydata.com" target=3D"_blank">https://www.cr=
unchydata.com</a></div><div>Enterprise Postgres Software Products &amp; Tec=
h Support</div><div><br></div></div></div></div>

--000000000000123b8e063c1aa0fe--