Re: verify predefined LWLocks have entries in wait_event_names.txt

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