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
