Re: Rethink the wait event names for postgres_fdw, dblink and etc - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Rethink the wait event names for postgres_fdw, dblink and etc
Date
Msg-id ZN79kznGqo7e6Ivn@paquier.xyz
Whole thread Raw
In response to Rethink the wait event names for postgres_fdw, dblink and etc  (Masahiro Ikeda <ikedamsh@oss.nttdata.com>)
Responses Re: Rethink the wait event names for postgres_fdw, dblink and etc
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: logical decoding and replication of sequences, take 2
Next
From: Peter Eisentraut
Date:
Subject: Re: dubious warning: FORMAT JSON has no effect for json and jsonb types