Drop back the redundant "Lock" suffix from LWLock wait event names

Started by Bertrand Drouvotabout 1 year ago3 messages
#1Bertrand Drouvot
bertranddrouvot.pg@gmail.com
1 attachment(s)

Hi hackers,

da952b415f unintentionally added back the "Lock" suffix into the LWLock wait
event names:

- "added back" because the "Lock" suffix was removed in 14a9101091
- "unintentionally" because there is nothing in the thread [2]/messages/by-id/202401231025.gbv4nnte5fmm@alvherre.pgsql that explicitly
mentions that the idea was also to revert 14a9101091

Please find attached a patch to remove it back so that the pg_stat_activity
view now reports back the LWLock without the "Lock" suffix (as pg_wait_events
and the related documentation do).

It has been reported in bug #18728 (see [1]/messages/by-id/18728-450924477056a339@postgresql.org).

[1]: /messages/by-id/18728-450924477056a339@postgresql.org
[2]: /messages/by-id/202401231025.gbv4nnte5fmm@alvherre.pgsql

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v1-0001-Drop-back-the-redundant-Lock-suffix-from-LWLock-w.patchtext/x-diff; charset=us-asciiDownload
From 86c30d2daf939a83b578de33fe96cf1790dbd427 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 2 Dec 2024 07:43:35 +0000
Subject: [PATCH v1] Drop back the redundant "Lock" suffix from LWLock wait
 event names

da952b415f unintentionally added back the "Lock" suffix into the LWLock wait
event names.

Per bug #18728 from Christophe Courtois.
---
 src/backend/storage/lmgr/generate-lwlocknames.pl | 1 +
 src/backend/storage/lmgr/lwlock.c                | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
 100.0% src/backend/storage/lmgr/

diff --git a/src/backend/storage/lmgr/generate-lwlocknames.pl b/src/backend/storage/lmgr/generate-lwlocknames.pl
index eaddd9d3b9..c4267328d3 100644
--- a/src/backend/storage/lmgr/generate-lwlocknames.pl
+++ b/src/backend/storage/lmgr/generate-lwlocknames.pl
@@ -107,6 +107,7 @@ while (<$lwlocklist>)
 	$lastlockidx = $lockidx;
 	$continue = ",\n";
 
+	# Add the Lock suffix only here as the C code depends of it
 	print $h "#define ${lockname}Lock (&MainLWLockArray[$lockidx].lock)\n";
 }
 
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index db6ed784ab..87cf295262 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -124,7 +124,7 @@ StaticAssertDecl(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS,
  * ... and do not forget to update the documentation's list of wait events.
  */
 static const char *const BuiltinTrancheNames[] = {
-#define PG_LWLOCK(id, lockname) [id] = CppAsString(lockname) "Lock",
+#define PG_LWLOCK(id, lockname) [id] = CppAsString(lockname),
 #include "storage/lwlocklist.h"
 #undef PG_LWLOCK
 	[LWTRANCHE_XACT_BUFFER] = "XactBuffer",
-- 
2.34.1

#2Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Bertrand Drouvot (#1)
Re: Drop back the redundant "Lock" suffix from LWLock wait event names

On 2024-Dec-02, Bertrand Drouvot wrote:

Hi hackers,

da952b415f unintentionally added back the "Lock" suffix into the LWLock wait
event names:

- "added back" because the "Lock" suffix was removed in 14a9101091
- "unintentionally" because there is nothing in the thread [2] that explicitly
mentions that the idea was also to revert 14a9101091

Oh, you're right, this was unintentional and unnoticed. I'll push this
shortly, to both 17 and master.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

#3Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Alvaro Herrera (#2)
Re: Drop back the redundant "Lock" suffix from LWLock wait event names

On 2024-Dec-02, Alvaro Herrera wrote:

Oh, you're right, this was unintentional and unnoticed. I'll push this
shortly, to both 17 and master.

Pushed, thanks Christophe and Bertrand.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.