Date: Fri, 5 Jan 2024 10:42:03 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
	Michael Paquier <michael@paquier.xyz>
Subject: Re: verify predefined LWLocks have entries in wait_event_names.txt
Message-ID: <20240105164203.GC2168314@nathanxps13>
References: <20240102173120.GA1061678@nathanxps13>
 <CA+TgmoYRhhL12iOcNKv6f__iZMk1_BPgbASFykPazhr_bwk0=w@mail.gmail.com>
 <20240102214544.GA1063425@nathanxps13>
 <CA+TgmoZGDO_hx8QvFkgj29JjyWm1+YJRELySwuLqiTJymAGVSw@mail.gmail.com>
 <ZZUT8SqhoUyy/hhM@ip-10-97-1-34.eu-west-3.compute.internal>
 <20240105061144.GA2166447@nathanxps13>
 <ZZeyO4eMjR2eiIiP@ip-10-97-1-34.eu-west-3.compute.internal>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZZeyO4eMjR2eiIiP@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: 
 <https://www.postgresql.org/message-id/20240105164203.GC2168314%40nathanxps13>
Precedence: bulk

Thanks for reviewing.

On Fri, Jan 05, 2024 at 07:39:39AM +0000, Bertrand Drouvot wrote:
> 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.

IMHO the ordering constraint makes it easier for humans to verify the lists
match.

> +       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.

Sure, I'll add something like that.  I think this particular scenario is
less likely, but that's not a reason to make the error message hard to
decipher.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com