Hi,
On Fri, Jan 05, 2024 at 12:11:44AM -0600, Nathan Bossart wrote:
> On Wed, Jan 03, 2024 at 07:59:45AM +0000, Bertrand Drouvot wrote:
> > +1 to add a test and put in a place that would produce failures at build time.
> > I think that having the test in the script that generates the header file is more
> > appropriate (as building the documentation looks less usual to me when working on
> > a patch).
>
> Okay, I did that in v2.
Thanks!
> +# NB: Predefined locks (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,12 @@ 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."
>
> +#
> +# Predefined LWLocks (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.
> +#
> +
Another option could be to create a sub-section for predefined LWLocks that are
part of lwlocknames.txt and then sort both list (the one in the sub-section and
the one in lwlocknames.txt). That would avoid the "must be listed in the same order"
constraint. That said, I think the way it is done in the patch is fine because
if one does not follow the constraint then the build would fail.
I did a few tests leading to:
CommitBDTTsSLRALock defined in lwlocknames.txt but missing from wait_event_names.txt at ./generate-lwlocknames.pl line
107,<$lwlocknames> line 58.
OR
lists of predefined LWLocks in lwlocknames.txt and wait_event_names.txt do not match at ./generate-lwlocknames.pl line
109,<$lwlocknames> line 46.
OR
CommitBDTTsSLRALock defined in wait_event_names.txt but missing from lwlocknames.txt at ./generate-lwlocknames.pl line
126,<$lwlocknames> line 57.
So, that looks good to me except one remark regarding:
+ die "lists of predefined LWLocks in lwlocknames.txt and wait_event_names.txt do not match"
+ unless $wait_event_lwlocks[$i] eq $lockname;
What about printing $wait_event_lwlocks[$i] and $lockname in the error message?
Something like?
"
die "lists of predefined LWLocks in lwlocknames.txt and wait_event_names.txt do not match (comparing $lockname and
$wait_event_lwlocks[$i])"
unless $wait_event_lwlocks[$i] eq $lockname;
"
I think that would give more clues for debugging purpose.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
