Hi,
On Thu, Mar 06, 2025 at 06:16:07PM -0500, Melanie Plageman wrote:
> Attached v12 does this (uses timestamps instead of instr_time).
Thanks for the new version!
> I've done some assorted cleanup. Most notably:
>
> I removed LOG_CONNECTION_NONE because it could lead to wrong results
> to have a bitmask with a flag value that is 0 (it could be set at the
> same time other flags are set, so you could never use it correctly).
Oh right, and starting with LOG_CONNECTION_NONE/OFF = (1 << 0) does not
make that much sense...
A couple of comments regarding 0002:
=== 01
Given that conn_timing.ready_for_use is only used here:
+ if (!reported_first_ready_for_query &&
+ (log_connections &
LOG_CONNECTION_READY_FOR_USE) &&
+ IsConnectionBackend(MyBackendType))
+ {
+ uint64 total_duration,
+ fork_duration,
+ auth_duration;
+
+ conn_timing.ready_for_use =
GetCurrentTimestamp();
+
+ total_duration =
+
TimestampDifferenceMicroseconds(conn_timing.socket_create,
+
conn_timing.ready_for_use);
I wonder if ready_for_use needs to be part of ConnectionTiming after all.
I mean we could just:
"
total_duration = TimestampDifferenceMicroseconds(conn_timing.socket_create,
GetCurrentTimestamp())
"
That said, having ready_for_use part of ConnectionTiming could be
useful if we decide to create a SQL API on top of this struct though. So,
that's probably fine and better as it is.
And if we keep it part of ConnectionTiming, then:
=== 02
+ if (!reported_first_ready_for_query &&
+ (log_connections &
LOG_CONNECTION_READY_FOR_USE) &&
+ IsConnectionBackend(MyBackendType))
What about getting rid of reported_first_ready_for_query and check if
conn_timing.ready_for_use is zero instead?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
