Hi,
On Tue, May 27, 2025 at 08:30:56AM +0900, Michael Paquier wrote:
> On Mon, May 26, 2025 at 07:49:34AM +0000, Bertrand Drouvot wrote:
> > That makes me think that it is easy to miss adding a new LWLock in
> > wait_event_names.txt and generate-wait_event_types.pl does not detect that.
>
> Agreed, adding a check is a good idea
Thanks for sharing your thoughts! Yeah and avoid code/doc discrepancies was
also one of the reason for generate-wait_event_types.pl creation, so I think
that we must ensure that it does so.
> This check is only in the docs, but the CI would detect
> that.
Right.
> This is not something strictly required for v18. Let's tackle
> that in v19~.
Yes.
> + if (exists $hashwe{'WaitEventLWLock'})
>
> So, the only reason why this is included in
> generate-wait_event_types.pl is that we need the data from %hashwe,
> reused in the new function you have added to perform the validation.
>
> The implementation of your validation check is not consistent with the
> existing generate-wait_event_types.pl, adding knowledge about lwlock.h
> lwlocklist.h which are passed as extra arguments for the function
> validate_lwlock_count().
I'm not sure to get what you mean with "not consistent". I think that we need
lwlock.h and lwlocklist.h so that generate-wait_event_types.pl can do its
work efficiently i.e generate the code, doc and avoid code/doc discrepancies.
> Perhaps it would be better if split into its
> own script, with the code in charge of building %hashwe (aka parsing
> the .txt file given in input argument) moved to a .pm "Parsing"
> module shared by both scripts, the one generating the data and the one
> cross-checking the lwlock numbers?
That could probably work, but do we really need building a new .pl file? I think
that it would make more sense if the "new" .pl in charge of cross-checking the
lwlock numbers would do more checks (unrelated to generate-wait_event_types.pl).
Then, we could have something like the .pm, check_generated_code.pl (new pl) and
generate-wait_event_types.pl with check_generated_code.pl doing things that are
related or not to generate-wait_event_types.pl.
I feel that what is proposed here is really linked to
generate-wait_event_types.pl
"only" (it's part of its duties) and that's probably why I'm having some
difficulty
to see the pros of the split.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
