Re: verify predefined LWLocks have entries in wait_event_names.txt - Mailing list pgsql-hackers

From Bertrand Drouvot
Subject Re: verify predefined LWLocks have entries in wait_event_names.txt
Date
Msg-id ZZeyO4eMjR2eiIiP@ip-10-97-1-34.eu-west-3.compute.internal
Whole thread Raw
In response to Re: verify predefined LWLocks have entries in wait_event_names.txt  (Nathan Bossart <nathandbossart@gmail.com>)
Responses Re: verify predefined LWLocks have entries in wait_event_names.txt
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Ashutosh Bapat
Date:
Subject: Re: Adding facility for injection points (or probe points?) for more advanced tests
Next
From: Jeff Davis
Date:
Subject: Re: [17] CREATE SUBSCRIPTION ... SERVER