On 2025/02/26 6:36, Melanie Plageman wrote:
On Tue, Feb 25, 2025 at 3:23 PM Melanie Plageman
<melanieplage...@gmail.com> wrote:

Thanks for doing this! I have implemented your suggestion in attached v3.

I missed an include in the EXEC_BACKEND not defined case. attached v4
is fixed up.

Thanks for updating the patch!

+       /* Capture time Postmaster initiates fork for logging */
+       if (child_type == B_BACKEND)
+               INSTR_TIME_SET_CURRENT(((BackendStartupData *) 
startup_data)->fork_time);

When log_connections is enabled, walsender connections are also logged.
However, with the patch, it seems the connection time for walsenders isn't 
captured.
Is this intentional?


With the current patch, when log_connections is enabled, the connection time is 
always
captured, and which might introduce performance overhead. No? Some users who 
enable
log_connections may not want this extra detail and want to avoid such overhead.
So, would it make sense to extend log_connections with a new option like 
"timing" and
log the connection time only when "timing" is specified?


+                               ereport(LOG,
+                                               errmsg("backend ready for query. 
pid=%d. socket=%d. connection establishment times (ms): total: %ld, fork: %ld, 
authentication: %ld",
+                                                          MyProcPid,
+                                                          (int) 
MyClientSocket->sock,

Why expose the socket's file descriptor? I'm not sure how users would use that 
information.


Including the PID seems unnecessary since it's already available via 
log_line_prefix with %p?


Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION



Reply via email to