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