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.com
>From 4e222c1a1fc1a2476746cb7b68c1d2a203816699 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@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/Makefile index 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/Makefile index 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.pl index 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.txt index 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.build index 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
