Re: verify predefined LWLocks have entries in wait_event_names.txt
Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, Michael Paquier <michael@paquier.xyz>
Date: 2024-01-03T07:59:45Z
Lists: pgsql-hackers
Hi, On Tue, Jan 02, 2024 at 10:49:03PM -0500, Robert Haas wrote: > On Tue, Jan 2, 2024 at 4:45 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > > That seems to date back to commit 14a9101. I can agree that the suffix is > > somewhat redundant since these are already marked as type "LWLock", but > > I'll admit I've been surprised by this before, too. IMHO it makes this > > proposed test more important because you can't just grep for a different > > lock to find all the places you need to update. > > I agree. I am pretty sure that the reason this happened in the first > place is that I grepped for the name of some other LWLock and adjusted > things for the new lock at every place where that found a hit. > > > > - Check in both directions instead of just one? > > > > > > - Verify ordering? > > > > To do those things, I'd probably move the test to one of the scripts that > > generates the documentation or header file (pg_wait_events doesn't tell us > > whether a lock is predefined or what order it's listed in). That'd cause > > failures at build time instead of during testing, which might be kind of > > nice, too. > > Yeah, I think that would be better. +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). Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Commits
-
Cross-check lists of predefined LWLocks.
- 5b1b9bce8449 17.0 landed
-
Remove Lock suffix from WALSummarizerLock in wait_event_names.txt
- 371b07e89449 17.0 landed
-
Drop the redundant "Lock" suffix from LWLock wait event names.
- 14a910109126 13.0 cited