Hi,
On 8/16/23 2:08 PM, Michael Paquier wrote:
> On Wed, Aug 16, 2023 at 01:43:35PM +0200, Drouvot, Bertrand wrote:
>> Yeah, agree, done that way in v6 (also added a test in 001_worker_spi.pl
>> to ensure that "worker_spi_main" is reported in pg_wait_event).
>
> -typedef struct WaitEventExtensionEntryByName
> -{
> - char wait_event_name[NAMEDATALEN]; /* hash key */
> - uint16 event_id; /* wait event ID */
> -} WaitEventExtensionEntryByName;
>
> I'd rather keep all these structures local to wait_event.c, as these
> cover the internals of the hash tables.
>
> Could you switch GetWaitEventExtensionEntries() so as it returns a
> list of strings or an array of char*? We probably can live with the
> list for that.
>
Yeah, I was not sure about this (returning a list of WaitEventExtensionEntryByName
or a list of wait event names) while working on v6.
That's true that the only need here is to get the names of the custom wait events.
Returning only the names would allow us to move the WaitEventExtensionEntryByName definition back
to the wait_event.c file.
It makes sense to me, done in v7 attached and renamed the function to GetWaitEventExtensionNames().
> + char buf[NAMEDATALEN + 44]; //"Custom wait event \"%Name%\" defined by extension" +
> Incorrect comment. This would be simpler as a StringInfo.
Yeah and probably less error prone: done in v7.
While at it, v7 is deliberately not calling "pfree(waiteventnames)" and "resetStringInfo(&buf)" in
pg_get_wait_events(): reason is that they are palloc in a short-lived memory context while executing
pg_get_wait_events().
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com