Hi,

Users wishing to debug slow connection establishment have little
visibility into which steps take the most time. We don't expose any
stats and none of the logging includes durations.

The attached patch logs durations for authentication, forking the
Postgres backend, and the end-to-end connection establishment duration
starting from when the postmaster calls accept() and ending the first
time the forked backend is ready for query.

As an example of how this could be useful, I artificially slowed down
authentication and you can see that that is now visible in logs:

LOG:  backend ready for query. pid=86341. socket=10. connection
establishment times (ms): total: 8, fork: 0, authentication: 0
LOG:  backend ready for query. pid=86794. socket=10. connection
establishment times (ms): total: 108, fork: 0, authentication: 100

Two notes on implementation:

To make this portable, the timestamps captured in the postmaster
(socket creation time, fork initiation time) are passed through the
ClientSocket and BackendStartupData structures instead of simply saved
in backend local memory inherited by the child process.

Secondly, I used TimestampTz for the times, but it is pretty unclear
to me when instr_time should be used vs TimestampTz for such things.
instr_time says it is more portable, but there are all sorts of places
that use TimestampTz that would probably need to be portable.
Also, INSTR_TIME_SUBTRACT() isn't overflow safe, which must be because
the actual data type of ticks is platform dependent and so the caller
is responsible for not passing it values that would overflow when
subtracted.

- Melanie
From f6fa312b605fac31463acc90446aad6afd6933ec Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Tue, 3 Dec 2024 18:02:15 -0500
Subject: [PATCH v1] Add connection establishment duration logging

Add durations for several key parts of connection establishment when
log_connections is enabled.

For a new incoming conneciton, starting from when the postmaster gets a
socket from accept() and ending when the forked child backend is first
ready for query, there are multiple steps that could each take longer
than expected due to external factors. Provide visibility into
authentication and fork duration as well as the end-to-end connection
establishment time with logging.

To make this portable, the timestamps captured in the postmaster (socket
creation time, fork initiation time) are passed through the ClientSocket
and BackendStartupData structures instead of simply saved in backend
local memory inherited by the child process.
---
 src/backend/postmaster/launch_backend.c | 25 +++++++++++++++++++++++++
 src/backend/postmaster/postmaster.c     |  8 ++++++++
 src/backend/tcop/postgres.c             | 23 +++++++++++++++++++++++
 src/backend/utils/adt/timestamp.c       | 13 +++++++++++++
 src/backend/utils/init/globals.c        |  2 ++
 src/backend/utils/init/postinit.c       | 11 +++++++++++
 src/include/libpq/libpq-be.h            |  1 +
 src/include/miscadmin.h                 |  3 +++
 src/include/postmaster/postmaster.h     |  7 +++++++
 src/include/tcop/backend_startup.h      |  3 +++
 src/include/utils/timestamp.h           |  3 +++
 src/tools/pgindent/typedefs.list        |  1 +
 12 files changed, 100 insertions(+)

diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c
index 1f2d829ec5a..4c3e3ced13a 100644
--- a/src/backend/postmaster/launch_backend.c
+++ b/src/backend/postmaster/launch_backend.c
@@ -232,6 +232,10 @@ postmaster_child_launch(BackendType child_type, int child_slot,
 
 	Assert(IsPostmasterEnvironment && !IsUnderPostmaster);
 
+	/* Capture time Postmaster initiates fork for logging */
+	if (child_type == B_BACKEND)
+		((BackendStartupData *) startup_data)->fork_time = GetCurrentTimestamp();
+
 #ifdef EXEC_BACKEND
 	pid = internal_forkexec(child_process_kinds[child_type].name, child_slot,
 							startup_data, startup_data_len, client_sock);
@@ -240,6 +244,16 @@ postmaster_child_launch(BackendType child_type, int child_slot,
 	pid = fork_process();
 	if (pid == 0)				/* child */
 	{
+		/* Calculate total fork duration in child backend for logging */
+		if (child_type == B_BACKEND)
+		{
+			TimestampTz fork_time = ((BackendStartupData *) startup_data)->fork_time;
+			TimestampTz cur_time = GetCurrentTimestamp();
+
+			conn_timing.fork_duration = TimestampDifferenceMicroseconds(fork_time,
+																		cur_time);
+		}
+
 		/* Close the postmaster's sockets */
 		ClosePostmasterPorts(child_type == B_LOGGER);
 
@@ -618,6 +632,17 @@ SubPostmasterMain(int argc, char *argv[])
 	/* Read in the variables file */
 	read_backend_variables(argv[2], &startup_data, &startup_data_len);
 
+	/* Calculate total fork duration in child backend for logging */
+	if (child_type == B_BACKEND)
+	{
+		TimestampTz fork_time = ((BackendStartupData *) startup_data)->fork_time;
+		TimestampTz cur_time = GetCurrentTimestamp();
+
+		conn_timing.fork_duration = TimestampDifferenceMicroseconds(fork_time,
+																	cur_time);
+	}
+
+
 	/* Close the postmaster's sockets (as soon as we know them) */
 	ClosePostmasterPorts(child_type == B_LOGGER);
 
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 6f37822c887..642dbb7d880 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1675,7 +1675,14 @@ ServerLoop(void)
 				ClientSocket s;
 
 				if (AcceptConnection(events[i].fd, &s) == STATUS_OK)
+				{
+					/*
+					 * Capture time that Postmaster got a socket from accept
+					 * (for logging connection establishment duration)
+					 */
+					s.creation_time = GetCurrentTimestamp();
 					BackendStartup(&s);
+				}
 
 				/* We no longer need the open socket in this process */
 				if (s.sock != PGINVALID_SOCKET)
@@ -3383,6 +3390,7 @@ BackendStartup(ClientSocket *client_sock)
 
 	/* Pass down canAcceptConnections state */
 	startup_data.canAcceptConnections = cac;
+	startup_data.fork_time = 0;
 	bn->rw = NULL;
 
 	/* Hasn't asked to be notified about any bgworkers yet */
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index e0a603f42bb..b1fae3cd476 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4280,6 +4280,7 @@ PostgresMain(const char *dbname, const char *username)
 	volatile bool send_ready_for_query = true;
 	volatile bool idle_in_transaction_timeout_enabled = false;
 	volatile bool idle_session_timeout_enabled = false;
+	bool		reported_first_ready_for_query = false;
 
 	Assert(dbname != NULL);
 	Assert(username != NULL);
@@ -4734,6 +4735,28 @@ PostgresMain(const char *dbname, const char *username)
 			/* Report any recently-changed GUC options */
 			ReportChangedGUCOptions();
 
+			/*
+			 * The first time this backend is ready for query, log the
+			 * durations of the different components of connection
+			 * establishment.
+			 */
+			if (!reported_first_ready_for_query &&
+				Log_connections && AmClientBackendProcess())
+			{
+				TimestampTz cur_time = GetCurrentTimestamp();
+				TimestampTz total_duration =
+					TimestampDifferenceMicroseconds(MyClientSocket->creation_time, cur_time);
+
+				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,
+							   (long int) total_duration / 1000,
+							   (long int) conn_timing.fork_duration / 1000,
+							   (long int) conn_timing.auth_duration / 1000));
+
+				reported_first_ready_for_query = true;
+			}
 			ReadyForQuery(whereToSendOutput);
 			send_ready_for_query = false;
 		}
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 18d7d8a108a..9adaad6ef01 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -1769,6 +1769,19 @@ TimestampDifferenceMilliseconds(TimestampTz start_time, TimestampTz stop_time)
 		return (long) ((diff + 999) / 1000);
 }
 
+/*
+ * Returns the difference between two timestamps safely as an unsigned int64.
+ */
+uint64
+TimestampDifferenceMicroseconds(TimestampTz start_time, TimestampTz stop_time)
+{
+	/* If start_time is in the future or now, no time has elapsed */
+	if (start_time >= stop_time)
+		return 0;
+
+	return (uint64) stop_time - start_time;
+}
+
 /*
  * TimestampDifferenceExceeds -- report whether the difference between two
  *		timestamps is >= a threshold (expressed in milliseconds)
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 03a54451ac2..f38a5c34cb1 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -43,6 +43,8 @@ volatile uint32 InterruptHoldoffCount = 0;
 volatile uint32 QueryCancelHoldoffCount = 0;
 volatile uint32 CritSectionCount = 0;
 
+ConnectionTiming conn_timing = {0};
+
 int			MyProcPid;
 pg_time_t	MyStartTime;
 TimestampTz MyStartTimestamp;
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 5b657a3f135..2c96ed56849 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -190,9 +190,15 @@ GetDatabaseTupleByOid(Oid dboid)
 static void
 PerformAuthentication(Port *port)
 {
+	TimestampTz auth_start_time = 0;
+
 	/* This should be set already, but let's make sure */
 	ClientAuthInProgress = true;	/* limit visibility of log messages */
 
+	/* Capture authentication start time for logging */
+	if (AmClientBackendProcess())
+		auth_start_time = GetCurrentTimestamp();
+
 	/*
 	 * In EXEC_BACKEND case, we didn't inherit the contents of pg_hba.conf
 	 * etcetera from the postmaster, and have to load them ourselves.
@@ -251,6 +257,11 @@ PerformAuthentication(Port *port)
 	 */
 	disable_timeout(STATEMENT_TIMEOUT, false);
 
+	/* Calculate authentication duration for logging */
+	if (AmClientBackendProcess())
+		conn_timing.auth_duration = TimestampDifferenceMicroseconds(auth_start_time,
+																	GetCurrentTimestamp());
+
 	if (Log_connections)
 	{
 		StringInfoData logmsg;
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 9109b2c3344..f0ad28c04ec 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -243,6 +243,7 @@ typedef struct ClientSocket
 {
 	pgsocket	sock;			/* File descriptor */
 	SockAddr	raddr;			/* remote addr (client) */
+	TimestampTz creation_time;
 } ClientSocket;
 
 #ifdef USE_SSL
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 42a2b38cac9..677afee2350 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -105,6 +105,8 @@ extern PGDLLIMPORT volatile uint32 InterruptHoldoffCount;
 extern PGDLLIMPORT volatile uint32 QueryCancelHoldoffCount;
 extern PGDLLIMPORT volatile uint32 CritSectionCount;
 
+extern PGDLLIMPORT struct ConnectionTiming conn_timing;
+
 /* in tcop/postgres.c */
 extern void ProcessInterrupts(void);
 
@@ -371,6 +373,7 @@ extern PGDLLIMPORT BackendType MyBackendType;
 
 #define AmAutoVacuumLauncherProcess() (MyBackendType == B_AUTOVAC_LAUNCHER)
 #define AmAutoVacuumWorkerProcess()	(MyBackendType == B_AUTOVAC_WORKER)
+#define AmClientBackendProcess() (MyBackendType == B_BACKEND)
 #define AmBackgroundWorkerProcess() (MyBackendType == B_BG_WORKER)
 #define AmWalSenderProcess()        (MyBackendType == B_WAL_SENDER)
 #define AmLogicalSlotSyncWorkerProcess() (MyBackendType == B_SLOTSYNC_WORKER)
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 24d49a5439e..e7562cb39b8 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -126,6 +126,12 @@ extern PMChild *AllocDeadEndChild(void);
 extern bool ReleasePostmasterChildSlot(PMChild *pmchild);
 extern PMChild *FindPostmasterChildByPid(int pid);
 
+typedef struct ConnectionTiming
+{
+	uint64		fork_duration;
+	uint64		auth_duration;
+} ConnectionTiming;
+
 /*
  * Note: MAX_BACKENDS is limited to 2^18-1 because that's the width reserved
  * for buffer references in buf_internals.h.  This limitation could be lifted
@@ -155,4 +161,5 @@ typedef enum DispatchOption
 
 extern DispatchOption parse_dispatch_option(const char *name);
 
+
 #endif							/* _POSTMASTER_H */
diff --git a/src/include/tcop/backend_startup.h b/src/include/tcop/backend_startup.h
index 993b013afdd..8e248bb8666 100644
--- a/src/include/tcop/backend_startup.h
+++ b/src/include/tcop/backend_startup.h
@@ -14,6 +14,8 @@
 #ifndef BACKEND_STARTUP_H
 #define BACKEND_STARTUP_H
 
+#include "datatype/timestamp.h"
+
 /* GUCs */
 extern PGDLLIMPORT bool Trace_connection_negotiation;
 
@@ -37,6 +39,7 @@ typedef enum CAC_state
 typedef struct BackendStartupData
 {
 	CAC_state	canAcceptConnections;
+	TimestampTz fork_time;
 } BackendStartupData;
 
 extern void BackendMain(char *startup_data, size_t startup_data_len) pg_attribute_noreturn();
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index a6ce03ed460..77014cd4fff 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -104,6 +104,9 @@ extern void TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
 								long *secs, int *microsecs);
 extern long TimestampDifferenceMilliseconds(TimestampTz start_time,
 											TimestampTz stop_time);
+
+extern uint64 TimestampDifferenceMicroseconds(TimestampTz start_time,
+											  TimestampTz stop_time);
 extern bool TimestampDifferenceExceeds(TimestampTz start_time,
 									   TimestampTz stop_time,
 									   int msec);
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ce33e55bf1d..a6792ec17a3 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -475,6 +475,7 @@ ConnParams
 ConnStatusType
 ConnType
 ConnectionStateEnum
+ConnectionTiming
 ConsiderSplitContext
 Const
 ConstrCheck
-- 
2.34.1

Reply via email to