Date: Tue, 2 Jan 2024 11:31:20 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
	Michael Paquier <michael@paquier.xyz>
Subject: verify predefined LWLocks have entries in wait_event_names.txt
Message-ID: <20240102173120.GA1061678@nathanxps13>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="huq684BweRXVnRxX"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/20240102173120.GA1061678%40nathanxps13>
Precedence: bulk


--huq684BweRXVnRxX
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

(new thread)

On Tue, Jan 02, 2024 at 10:34:11AM -0500, Robert Haas wrote:
> On Wed, Dec 27, 2023 at 10:36 AM Nathan Bossart
> <nathandbossart@gmail.com> wrote:
>> Thanks!  I also noticed that WALSummarizerLock probably needs a mention in
>> wait_event_names.txt.
> 
> Fixed.

I think we're supposed to omit the "Lock" suffix in wait_event_names.txt.

> It seems like it would be good if there were an automated cross-check
> between lwlocknames.txt and wait_event_names.txt.

+1.  Here's a hastily-thrown-together patch for that.  I basically copied
003_check_guc.pl and adjusted it for this purpose.  This test only checks
that everything in lwlocknames.txt has a matching entry in
wait_event_names.txt.  It doesn't check that everything in the predefined
LWLock section of wait_event_names.txt has an entry in lwlocknames.txt.
AFAICT that would be a little more difficult because you can't distinguish
between the two in pg_wait_events.

Even with this test, I worry that we could easily forget to add entries in
wait_event_names.txt for the non-predefined locks, but I don't presently
have a proposal for how to prevent that.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

--huq684BweRXVnRxX
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0001-verify-wait-events-for-all-lwlocks.patch"

From 5031b55d8e1095ca919fe9a088ec0539b28984fc Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 2 Jan 2024 11:19:20 -0600
Subject: [PATCH v1 1/1] verify wait events for all lwlocks

---
 .../utils/activity/wait_event_names.txt       |  2 +-
 src/test/modules/test_misc/meson.build        |  1 +
 .../modules/test_misc/t/005_check_lwlock.pl   | 51 +++++++++++++++++++
 3 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 src/test/modules/test_misc/t/005_check_lwlock.pl

diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index d876f8a667..f61ec3e59d 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -324,7 +324,7 @@ XactTruncation	"Waiting to execute <function>pg_xact_status</function> or update
 WrapLimitsVacuum	"Waiting to update limits on transaction id and multixact consumption."
 NotifyQueueTail	"Waiting to update limit on <command>NOTIFY</command> message storage."
 WaitEventExtension	"Waiting to read or update custom wait events information for extensions."
-WALSummarizerLock	"Waiting to read or update WAL summarization state."
+WALSummarizer	"Waiting to read or update WAL summarization state."
 
 XactBuffer	"Waiting for I/O on a transaction status SLRU buffer."
 CommitTsBuffer	"Waiting for I/O on a commit timestamp SLRU buffer."
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 911084ac0f..13bc6cb423 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -10,6 +10,7 @@ tests += {
       't/002_tablespace.pl',
       't/003_check_guc.pl',
       't/004_io_direct.pl',
+      't/005_check_lwlock.pl',
     ],
   },
 }
diff --git a/src/test/modules/test_misc/t/005_check_lwlock.pl b/src/test/modules/test_misc/t/005_check_lwlock.pl
new file mode 100644
index 0000000000..aff83e879e
--- /dev/null
+++ b/src/test/modules/test_misc/t/005_check_lwlock.pl
@@ -0,0 +1,51 @@
+# Tests to cross-check the consistency of pg_wait_events with lwlocknames.h.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->start;
+
+# Grab the names of all the LWLocks from pg_wait_events.
+my $all_lwlocks = $node->safe_psql(
+	'postgres',
+	"SELECT name FROM pg_wait_events WHERE type = 'LWLock'");
+my @all_lwlocks_array = split("\n", $all_lwlocks);
+
+# Find the location of lwlocknames.h.
+my $include_dir = $node->config_data('--includedir');
+my $lwlocknames_file = "$include_dir/server/storage/lwlocknames.h";
+
+# Read the list of locks from lwlocknames.h.
+my $num_tests = 0;
+my @lwlocks_found;
+open(my $contents, '<', $lwlocknames_file)
+  || die "Could not open $lwlocknames_file: $!";
+while (my $line = <$contents>)
+{
+	if ($line =~ m/^#define ([a-zA-Z]+)Lock .*/)
+	{
+		push @lwlocks_found, $1;
+	}
+}
+close $contents;
+
+# Cross-check that all the locks found in lwlocknames.h have matching entries
+# in pg_wait_events.
+my %all_lwlocks_hash = map { $_ => 1 } @all_lwlocks_array;
+my @missing_from_wait_events = grep(!$all_lwlocks_hash{$_}, @lwlocks_found);
+is(scalar(@missing_from_wait_events),
+	0, "no LWLocks missing from pg_wait_events");
+
+foreach my $lwlock (@missing_from_wait_events)
+{
+	print(
+		"found $lwlock in lwlocknames.h, missing from wait_event_names.txt\n"
+	);
+}
+
+done_testing();
-- 
2.25.1


--huq684BweRXVnRxX--