Re: Rethink the wait event names for postgres_fdw, dblink and etc

On Fri, Aug 18, 2023 at 12:27:02PM +0900, Masahiro Ikeda wrote:
> I expect that no one will object to changing the names to appropriate
> ones. But, we need to discuss that naming convention, the names themselves,
> document descriptions and so on.
>
> I made the v1 patch
> * CamelCase naming convention

Not sure how others feel about that, but I am OK with camel-case style
for the modules.

> I haven't added document descriptions for pg_prewarm and test modules.
> The reason is that the wait event of autoprewarm is not shown on
> pg_stat_activity. It's not an auxiliary-process and doesn't connect to
> a database, so pgstat_bestart() isn't called.

Perhaps we could just leave it out, then, adding a comment instead.

> Feedback is always welcome and appreciated.

+ /* first time, allocate or get the custom wait event */
+ if (wait_event_info == 0)
+ wait_event_info = WaitEventExtensionNew("DblinkConnect");
[...]
+ /* first time, allocate or get the custom wait event */
+ if (wait_event_info == 0)
+ wait_event_info = WaitEventExtensionNew("DblinkConnect");

Shouldn't dblink use two different strings?

+ if (wait_event_info == 0)
+ wait_event_info = WaitEventExtensionNew("PgPrewarmDumpDelay");

Same about autoprewarm.c. The same flag is used in two different code
paths. If removed from the patch, no need to do that, of course.

+static uint32 wait_event_info_connect = 0;
+static uint32 wait_event_info_receive = 0;
+static uint32 wait_event_info_cleanup_receive = 0;

Perhaps such variables could be named with shorter names proper to
each module, like pgfdw_we_receive, etc.

+ <filename>dblink</filename> could show the following wait event under the wait

s/could show/can report/?

+ Waiting for same reason as <literal>PostgresFdwReceive</literal>, except that it's only for
+ abort.

"Waiting for transaction abort on remote server"?
---
Michael