From: Nathan Bossart <nathandbossart@gmail.com>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Michael Paquier <michael@paquier.xyz>
Subject: Re: verify predefined LWLocks have entries in wait_event_names.txt
Date: Mon, 8 Jan 2024 16:02:12 -0600
Message-ID: <20240108220212.GA2914881@nathanxps13> (raw)
In-Reply-To: <20240108201130.GA2747735@nathanxps13>
References: <CA+TgmoZGDO_hx8QvFkgj29JjyWm1+YJRELySwuLqiTJymAGVSw@mail.gmail.com>
<ZZUT8SqhoUyy/hhM@ip-10-97-1-34.eu-west-3.compute.internal>
<20240105061144.GA2166447@nathanxps13>
<ZZeyO4eMjR2eiIiP@ip-10-97-1-34.eu-west-3.compute.internal>
<20240105164203.GC2168314@nathanxps13>
<20240105174620.GA2285561@nathanxps13>
<ZZkXeBMdkg4A1f09@ip-10-97-1-34.eu-west-3.compute.internal>
<20240106161852.GA2435448@nathanxps13>
<ZZurTpDvwMJO1EDV@ip-10-97-1-34.eu-west-3.compute.internal>
<20240108201130.GA2747735@nathanxps13>
Sorry for the noise. I spent some more time tidying this up for commit,
which I am hoping to do in the next day or two.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.comAttachments:v6-0001-Cross-check-lists-of-predefined-LWLocks.patch (text/x-diff, 6.9K) download
From 4e222c1a1fc1a2476746cb7b68c1d2a203816699 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 8 Jan 2024 15:44:44 -0600
Subject: [PATCH v6 1/1] Cross-check lists of predefined LWLocks.
Both lwlocknames.txt and wait_event_names.txt contain a list of all
the predefined LWLocks, i.e., those with predefined positions
within MainLWLockArray. It is easy to miss one or the other,
especially since the list in wait_event_names.txt omits the "Lock"
suffix for all the LWLock wait events. This commit adds a cross-
check of these lists to the script that generates lwlocknames.h.
If the lists do not match exactly, building will fail.
Suggested-by: Robert Haas
Reviewed-by: Robert Haas, Michael Paquier, Bertrand Drouvot
Discussion: https://postgr.es/m/20240102173120.GA1061678%40nathanxps13
---
src/backend/Makefile | 2 +-
src/backend/storage/lmgr/Makefile | 4 +-
.../storage/lmgr/generate-lwlocknames.pl | 49 +++++++++++++++++++
.../utils/activity/wait_event_names.txt | 12 +++++
src/include/storage/meson.build | 4 +-
5 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/src/backend/Makefile b/src/backend/Makefileindex 816461aa17..7d2ea7d54a 100644--- a/src/backend/Makefile+++ b/src/backend/Makefile@@ -130,7 +130,7 @@ $(top_builddir)/src/port/libpgport_srv.a: | submake-libpgport
parser/gram.h: parser/gram.y
$(MAKE) -C parser gram.h
-storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl storage/lmgr/lwlocknames.txt+storage/lmgr/lwlocknames.h: storage/lmgr/generate-lwlocknames.pl storage/lmgr/lwlocknames.txt utils/activity/wait_event_names.txt
$(MAKE) -C storage/lmgr lwlocknames.h lwlocknames.c
utils/activity/wait_event_types.h: utils/activity/generate-wait_event_types.pl utils/activity/wait_event_names.txt
diff --git a/src/backend/storage/lmgr/Makefile b/src/backend/storage/lmgr/Makefileindex c48ba943c4..504480e847 100644--- a/src/backend/storage/lmgr/Makefile+++ b/src/backend/storage/lmgr/Makefile@@ -39,8 +39,8 @@ s_lock_test: s_lock.c $(top_builddir)/src/common/libpgcommon.a $(top_builddir)/s
lwlocknames.c: lwlocknames.h
touch $@
-lwlocknames.h: $(top_srcdir)/src/backend/storage/lmgr/lwlocknames.txt generate-lwlocknames.pl- $(PERL) $(srcdir)/generate-lwlocknames.pl $<+lwlocknames.h: $(top_srcdir)/src/backend/storage/lmgr/lwlocknames.txt $(top_srcdir)/src/backend/utils/activity/wait_event_names.txt generate-lwlocknames.pl+ $(PERL) $(srcdir)/generate-lwlocknames.pl $^
check: s_lock_test
./s_lock_test
diff --git a/src/backend/storage/lmgr/generate-lwlocknames.pl b/src/backend/storage/lmgr/generate-lwlocknames.plindex bef5e16e11..06401dbf01 100644--- a/src/backend/storage/lmgr/generate-lwlocknames.pl+++ b/src/backend/storage/lmgr/generate-lwlocknames.pl@@ -15,6 +15,7 @@ my $continue = "\n";
GetOptions('outdir:s' => \$output_path);
open my $lwlocknames, '<', $ARGV[0] or die;
+open my $wait_event_names, '<', $ARGV[1] or die;
# Include PID in suffix in case parallel make runs this multiple times.
my $htmp = "$output_path/lwlocknames.h.tmp$$";
@@ -30,6 +31,40 @@ print $c $autogen, "\n";
print $c "const char *const IndividualLWLockNames[] = {";
+#+# First, record the predefined LWLocks listed in wait_event_names.txt. We'll+# cross-check those with the ones in lwlocknames.txt.+#+my @wait_event_lwlocks;+my $record_lwlocks = 0;++while (<$wait_event_names>)+{+ chomp;++ # Check for end marker.+ last if /^# END OF PREDEFINED LWLOCKS/;++ # Skip comments and empty lines.+ next if /^#/;+ next if /^\s*$/;++ # Start recording LWLocks when we find the WaitEventLWLock section.+ if (/^Section: ClassName - WaitEventLWLock$/)+ {+ $record_lwlocks = 1;+ next;+ }++ # Go to the next line if we are not yet recording LWLocks.+ next if not $record_lwlocks;++ # Record the LWLock.+ (my $waiteventname, my $waitevendocsentence) = split(/\t/, $_);+ push(@wait_event_lwlocks, $waiteventname . "Lock");+}++my $i = 0;
while (<$lwlocknames>)
{
chomp;
@@ -50,6 +85,15 @@ while (<$lwlocknames>)
die "lwlocknames.txt not in order" if $lockidx < $lastlockidx;
die "lwlocknames.txt has duplicates" if $lockidx == $lastlockidx;
+ die "$lockname defined in lwlocknames.txt but missing from "+ . "wait_event_names.txt"+ if $i >= scalar @wait_event_lwlocks;+ die "lists of predefined LWLocks do not match (first mismatch at "+ . "$wait_event_lwlocks[$i] in wait_event_names.txt and $lockname in "+ . "lwlocknames.txt)"+ if $wait_event_lwlocks[$i] ne $lockname;+ $i++;+
while ($lastlockidx < $lockidx - 1)
{
++$lastlockidx;
@@ -63,6 +107,11 @@ while (<$lwlocknames>)
print $h "#define $lockname (&MainLWLockArray[$lockidx].lock)\n";
}
+die+ "$wait_event_lwlocks[$i] defined in wait_event_names.txt but missing from "+ . "lwlocknames.txt"+ if scalar @wait_event_lwlocks ne $i;+
printf $c "\n};\n";
print $h "\n";
printf $h "#define NUM_INDIVIDUAL_LWLOCKS %s\n", $lastlockidx + 1;
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txtindex 088eb977d4..ad6bf0de84 100644--- a/src/backend/utils/activity/wait_event_names.txt+++ b/src/backend/utils/activity/wait_event_names.txt@@ -276,6 +276,10 @@ Extension "Waiting in an extension."
# This class of wait events has its own set of C structure, so these are
# only used for the documentation.
#
+# NB: Predefined locks (i.e., those declared in lwlocknames.txt) must be listed+# in the top section of locks and must be listed in the same order as in+# lwlocknames.txt.+#
Section: ClassName - WaitEventLWLock
@@ -326,6 +330,14 @@ NotifyQueueTail "Waiting to update limit on <command>NOTIFY</command> message st
WaitEventExtension "Waiting to read or update custom wait events information for extensions."
WALSummarizer "Waiting to read or update WAL summarization state."
+#+# END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE)+#+# Predefined LWLocks (i.e., those declared in lwlocknames.txt) must be listed+# in the section above and must be listed in the same order as in+# lwlocknames.txt. Other LWLocks must be listed in the section below.+#+
XactBuffer "Waiting for I/O on a transaction status SLRU buffer."
CommitTsBuffer "Waiting for I/O on a commit timestamp SLRU buffer."
SubtransBuffer "Waiting for I/O on a sub-transaction SLRU buffer."
diff --git a/src/include/storage/meson.build b/src/include/storage/meson.buildindex 2a88248464..666fb22408 100644--- a/src/include/storage/meson.build+++ b/src/include/storage/meson.build@@ -1,7 +1,9 @@
# Copyright (c) 2022-2024, PostgreSQL Global Development Group
lwlocknames = custom_target('lwlocknames',
- input: files('../../backend/storage/lmgr/lwlocknames.txt'),+ input: files(+ '../../backend/storage/lmgr/lwlocknames.txt',+ '../../backend/utils/activity/wait_event_names.txt'),
output: ['lwlocknames.h', 'lwlocknames.c'],
command: [
perl, files('../../backend/storage/lmgr/generate-lwlocknames.pl'),
--
2.25.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, bertranddrouvot.pg@gmail.com, robertmhaas@gmail.com, michael@paquier.xyz
Subject: Re: verify predefined LWLocks have entries in wait_event_names.txt
In-Reply-To: <20240108220212.GA2914881@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox