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
signature.asc
Description: PGP signature