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