Message-ID: <2f84b397-d020-493c-924c-610b9dd18f8d@gmail.com>
Date: Wed, 16 Aug 2023 07:04:53 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Subject: Re: WIP: new system catalog pg_wait_event
To: Michael Paquier <michael@paquier.xyz>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, tgl@sss.pgh.pa.us,
 pgsql-hackers@lists.postgresql.org
References: <3658241.1691161701@sss.pgh.pa.us>
 <0109438a-9672-322b-7638-04ad47c97d0f@gmail.com>
 <1dc98591-0d06-f0da-bc88-21f529d6fb8e@gmail.com>
 <20230808.115332.301701413576096139.horikyota.ntt@gmail.com>
 <ZNGw9T2t5z5Kg1xd@paquier.xyz>
 <b21e5706-b6dd-42f1-a744-3afffe87c2c9@gmail.com>
 <c915434a-3d3a-4135-8f3a-fca0cf22c6ae@gmail.com>
 <ZNNGq7DTv9ur5113@paquier.xyz>
 <b71d1977-1a8b-4129-8c03-533031dbd004@gmail.com>
 <ZNmvegQirGI9ifVw@paquier.xyz>
Content-Language: en-US
In-Reply-To: <ZNmvegQirGI9ifVw@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/2f84b397-d020-493c-924c-610b9dd18f8d%40gmail.com>
Precedence: bulk

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