public inbox for [email protected]  
help / color / mirror / Atom feed
From: Drouvot, Bertrand <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: WIP: new system catalog pg_wait_event
Date: Wed, 16 Aug 2023 07:04:53 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Hi,

On 8/14/23 6:37 AM, Michael Paquier wrote:
> On Thu, Aug 10, 2023 at 08:09:34PM +0200, Drouvot, Bertrand wrote:
>> Agree that's worth it given the fact that iterating one more time is not that
>> costly here.
> 
> I have reviewed v4, and finished by putting my hands on it to see what
> I could do.

Thanks!


> There are two things
> that we could do:
> - Hide that behind a macro defined in wait_event_funcs.c.
> - Feed the data generated here into a static structure, like:
> +static const struct
> +{
> +   const char *type;
> +   const char *name;
> +   const char *description;
> +}
> 
> After experimenting with both, I've found the latter a tad cleaner, so
> the attached version does that.

Yeah, looking at what you've done in v5, I also agree that's better
that what has been done in v4 (I also think it will be easier to maintain).

> I am not sure that "pg_wait_event" is a good idea for the name if the
> new view.  How about "pg_wait_events" instead, in plural form?  There
> is more than one wait event listed.
> 

I'd prefer the singular form. There is a lot of places where it's already used
(pg_database, pg_user, pg_namespace...to name a few) and it looks like that using
the plural form are exceptions.

> One log entry in Solution.pm has missed the addition of a reference to
> wait_event_funcs_data.c.
> 

Oh right, thanks for fixing it in v5.

> And..  src/backend/Makefile missed two updates for maintainer-clean & co,
> no?
>  
Oh right, thanks for fixing it in v5.

> One thing that the patch is still missing is the handling of custom
> wait events for extensions. 

Yeah, now that af720b4c50 is done, I'll add the custom wait events handling
in v6.

> So this still requires more code.  I was
> thinking about listed these events as:
> - Type: "Extension"
> - Name: What a module has registered.
> - Description: "Custom wait event \"%Name%\" defined by extension".

That sounds good to me, I'll do that.

Regards,

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






view thread (7+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: WIP: new system catalog pg_wait_event
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox