Re: Autogenerate some wait events code and documentation

Hi,

On 5/6/23 4:23 AM, Michael Paquier wrote:
> On Thu, May 04, 2023 at 08:39:49AM +0200, Drouvot, Bertrand wrote:
>> On 5/1/23 1:59 AM, Michael Paquier wrote:
>> I'm not sure I like it. First, it does break the "Note" ordering as compare
>> to the current documentation. That's not a big deal though.
>>
>> Secondly, what If we need to add some note(s) in the future for
>> another wait class? Having all the notes after all the tables are
>> generated would sound weird to me.
>
> Appending these notes at the end of all the tables does not strike me
> as a big dea, TBH. But, well, my sole opinion is not the final choice
> either. For now, I am mostly tempted to keep the generation script as
> minimalistic as possible.
>

I agree that's not a big deal and I'm not against having these notes at the end
of all the tables.

>> We could discuss another approach for the "Note" part if there is a
>> need to add one for an existing/new wait class though.
>

means, that was more a NIT comment from my side.

> Documenting what's expected from the wait event classes is critical in
> the .txt file as that's what developers are going to look at when
> adding a new wait event. Adding them in the header is less appealing
> to me considering that is it now generated, and the docs provide a lot
> of explanation as well.
>

Your argument that the header is now generated makes me change my mind: I
know think that having the comments in the .txt file is enough.

>>> This has as extra consequence to require a change in
>>> wait_event.h so as PG_WAIT_BUFFER_PIN is renamed to PG_WAIT_BUFFERPIN,
>>> equally fine by me. Logically, this rename should be done in a patch
>>> of its own, for clarity.
>>
>> Yes, I can look at it.
>> [...]
>> Agree, I'll look at this.
>
> Thanks!

Please find the dedicated patch proposal in [1].

[1]: https://www.postgresql.org/message-id/c6f35117-4b20-4c78-1df5-d3056010dcf5%40gmail.com

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com