Re: Autogenerate some wait events code and documentation

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Autogenerate some wait events code and documentation
Date: 2024-04-04 09:28:36
Message-ID: Zg5yxKtZQqBXwLIo@ip-10-97-1-34.eu-west-3.compute.internal
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Thu, Apr 04, 2024 at 03:50:21PM +0900, Michael Paquier wrote:
> On Tue, Mar 19, 2024 at 07:34:09AM +0000, Bertrand Drouvot wrote:
> > I'm not sure as v2 used the "version >= 17" wording which I think would not need
> > manual refresh each time a new stable branch is forked.
> >
> > But to avoid any doubt, I'm following your recommendation in v3 attached (then
> > only mentioning the "master branch" and "any other branch").
>
> I don't see why we could not be more generic, TBH. Note that the
> Backpatch region should be empty not only the master branch but also
> on stable and unreleased branches (aka REL_XX_STABLE branches from
> their fork from master to their .0 release). I have reworded the
> whole, mentioning ABI compatibility, as well.

Yeah, agree. I do prefer your wording.

> The position of the Backpatch regions were a bit incorrect (extra one
> in LWLock, and the one in Lock was not needed).

oops, thanks for the fixes!

> We could be stricter with the order of the elements in
> pgstat_wait_event.c and wait_event_funcs_data.c, but there's no
> consequence feature-wise and I cannot get excited about the extra
> complexity this creates in generate-wait_event_types.pl between the
> enum generation and the rest.

Yeah, and I think generate-wait_event_types.pl is already complex enough.
So better to add only the strict necessary in it IMHO.

> Is "Backpatch" the best choice we have, though? It speaks by itself
> but I was thinking about something different, like "Stable". Other
> ideas or objections are welcome. My naming sense is usually not that
> good, so there's that.

I think "Stable" is more confusing because the section should also be empty until
the .0 is released.

That said, what about "ABI_compatibility"? (that would also match the comment
added in wait_event_names.txt). Attached v4 making use of the ABI_compatibility
proposal.

> 0001 is the patch with my tweaks.

Thanks!

+# No "Backpatch" region here as code is generated automatically.

What about "....region here as has its own C code" (that would be more consistent
with the comment in the "header" for the file). Done that way in v4.

It looks like WAL_SENDER_WRITE_ZZZ was also added in it (I guess for testing
purpose, so I removed it in v4).

Regards,

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

Attachment Content-Type Size
v4-0001-Add-ABI_compatibility-regions-in-wait_event_names.patch text/x-diff 5.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2024-04-04 09:29:09 Re: Synchronizing slots from primary to standby
Previous Message Etsuro Fujita 2024-04-04 09:13:00 Re: postgres_fdw: Useless test in pgfdw_exec_cleanup_query_end()