From cc3bda37f704dffb641a19d17d447e0eeb1a6c6c Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 12 Sep 2005 02:26:33 +0000
Subject: [PATCH] Tweak TCP-keepalive code so that an invalid setting doesn't
 cause us to drop connections unceremoniously.  Also some other marginal
 cleanups: don't query getsockopt() repeatedly if it fails, and avoid having
 the apparent definition of struct Port depend on which system headers you
 might have included or not.  Oliver Jowett and Tom Lane.

---
 src/backend/libpq/pqcomm.c                    | 97 ++++++++++++-------
 src/backend/utils/misc/guc.c                  | 30 +++---
 src/backend/utils/misc/postgresql.conf.sample | 14 +--
 src/include/libpq/libpq-be.h                  | 35 +++----
 4 files changed, 98 insertions(+), 78 deletions(-)

diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 6c2f6fd14e8..23a9865acb5 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -30,7 +30,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/libpq/pqcomm.c,v 1.178 2005/07/30 20:28:20 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/libpq/pqcomm.c,v 1.179 2005/09/12 02:26:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -595,18 +595,16 @@ StreamConnection(int server_fd, Port *port)
 			return STATUS_ERROR;
 		}
 
-		/* Set default keepalive parameters. This should also catch
-		 * misconfigurations (non-zero values when socket options aren't
-		 * supported)
+		/*
+		 * Also apply the current keepalive parameters.  If we fail to set
+		 * a parameter, don't error out, because these aren't universally
+		 * supported.  (Note: you might think we need to reset the GUC
+		 * variables to 0 in such a case, but it's not necessary because
+		 * the show hooks for these variables report the truth anyway.)
 		 */
-		if (pq_setkeepalivesidle(tcp_keepalives_idle, port) != STATUS_OK)
-			return STATUS_ERROR;
-
-		if (pq_setkeepalivesinterval(tcp_keepalives_interval, port) != STATUS_OK)
-			return STATUS_ERROR;
-
-		if (pq_setkeepalivescount(tcp_keepalives_count, port) != STATUS_OK)
-			return STATUS_ERROR;
+		(void) pq_setkeepalivesidle(tcp_keepalives_idle, port);
+		(void) pq_setkeepalivesinterval(tcp_keepalives_interval, port);
+		(void) pq_setkeepalivescount(tcp_keepalives_count, port);
 	}
 
 	return STATUS_OK;
@@ -1172,11 +1170,16 @@ pq_endcopyout(bool errorAbort)
 	DoingCopyOut = false;
 }
 
+
+/*
+ * Support for TCP Keepalive parameters
+ */
+
 int
 pq_getkeepalivesidle(Port *port)
 {
 #ifdef TCP_KEEPIDLE
-	if (IS_AF_UNIX(port->laddr.addr.ss_family))
+	if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
 		return 0;
 
 	if (port->keepalives_idle != 0)
@@ -1185,12 +1188,13 @@ pq_getkeepalivesidle(Port *port)
 	if (port->default_keepalives_idle == 0)
 	{
 		socklen_t size = sizeof(port->default_keepalives_idle);
+
 		if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPIDLE,
-					   (char *) &port->default_keepalives_idle, 
+					   (char *) &port->default_keepalives_idle,
 					   &size) < 0)
 		{
 			elog(LOG, "getsockopt(TCP_KEEPIDLE) failed: %m");
-			return -1;
+			port->default_keepalives_idle = -1;	/* don't know */
 		}
 	}
 
@@ -1199,23 +1203,28 @@ pq_getkeepalivesidle(Port *port)
 	return 0;
 #endif
 }
-   
+
 int
 pq_setkeepalivesidle(int idle, Port *port)
 {
-	if (IS_AF_UNIX(port->laddr.addr.ss_family))
+	if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
 		return STATUS_OK;
 
 #ifdef TCP_KEEPIDLE
 	if (idle == port->keepalives_idle)
 		return STATUS_OK;
 
-	if (port->default_keepalives_idle == 0)
+	if (port->default_keepalives_idle <= 0)
 	{
 		if (pq_getkeepalivesidle(port) < 0)
-			return STATUS_ERROR;
+		{
+			if (idle == 0)
+				return STATUS_OK; /* default is set but unknown */
+			else
+				return STATUS_ERROR;
+		}
 	}
-			
+
 	if (idle == 0)
 		idle = port->default_keepalives_idle;
 
@@ -1242,7 +1251,7 @@ int
 pq_getkeepalivesinterval(Port *port)
 {
 #ifdef TCP_KEEPINTVL
-	if (IS_AF_UNIX(port->laddr.addr.ss_family))
+	if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
 		return 0;
 
 	if (port->keepalives_interval != 0)
@@ -1251,12 +1260,13 @@ pq_getkeepalivesinterval(Port *port)
 	if (port->default_keepalives_interval == 0)
 	{
 		socklen_t size = sizeof(port->default_keepalives_interval);
+
 		if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPINTVL,
-					   (char *) &port->default_keepalives_interval, 
+					   (char *) &port->default_keepalives_interval,
 					   &size) < 0)
 		{
 			elog(LOG, "getsockopt(TCP_KEEPINTVL) failed: %m");
-			return -1;
+			port->default_keepalives_interval = -1;	/* don't know */
 		}
 	}
 
@@ -1265,22 +1275,28 @@ pq_getkeepalivesinterval(Port *port)
 	return 0;
 #endif
 }
-   
+
 int
 pq_setkeepalivesinterval(int interval, Port *port)
 {
-	if (IS_AF_UNIX(port->laddr.addr.ss_family))
+	if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
 		return STATUS_OK;
 
 #ifdef TCP_KEEPINTVL
 	if (interval == port->keepalives_interval)
 		return STATUS_OK;
 
-	if (port->default_keepalives_interval == 0) {
+	if (port->default_keepalives_interval <= 0)
+	{
 		if (pq_getkeepalivesinterval(port) < 0)
-			return STATUS_ERROR;
+		{
+			if (interval == 0)
+				return STATUS_OK; /* default is set but unknown */
+			else
+				return STATUS_ERROR;
+		}
 	}
-			
+
 	if (interval == 0)
 		interval = port->default_keepalives_interval;
 
@@ -1297,7 +1313,7 @@ pq_setkeepalivesinterval(int interval, Port *port)
 	{
 		elog(LOG, "setsockopt(TCP_KEEPINTVL) not supported");
 		return STATUS_ERROR;
-	}		
+	}
 #endif
 
 	return STATUS_OK;
@@ -1307,7 +1323,7 @@ int
 pq_getkeepalivescount(Port *port)
 {
 #ifdef TCP_KEEPCNT
-	if (IS_AF_UNIX(port->laddr.addr.ss_family))
+	if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
 		return 0;
 
 	if (port->keepalives_count != 0)
@@ -1316,12 +1332,13 @@ pq_getkeepalivescount(Port *port)
 	if (port->default_keepalives_count == 0)
 	{
 		socklen_t size = sizeof(port->default_keepalives_count);
+
 		if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPCNT,
-					   (char *) &port->default_keepalives_count, 
+					   (char *) &port->default_keepalives_count,
 					   &size) < 0)
 		{
 			elog(LOG, "getsockopt(TCP_KEEPCNT) failed: %m");
-			return -1;
+			port->default_keepalives_count = -1;	/* don't know */
 		}
 	}
 
@@ -1330,22 +1347,28 @@ pq_getkeepalivescount(Port *port)
 	return 0;
 #endif
 }
-   
+
 int
 pq_setkeepalivescount(int count, Port *port)
 {
-	if (IS_AF_UNIX(port->laddr.addr.ss_family))
+	if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
 		return STATUS_OK;
 
 #ifdef TCP_KEEPCNT
 	if (count == port->keepalives_count)
 		return STATUS_OK;
 
-	if (port->default_keepalives_count == 0) {
+	if (port->default_keepalives_count <= 0)
+	{
 		if (pq_getkeepalivescount(port) < 0)
-			return STATUS_ERROR;
+		{
+			if (count == 0)
+				return STATUS_OK; /* default is set but unknown */
+			else
+				return STATUS_ERROR;
+		}
 	}
-			
+
 	if (count == 0)
 		count = port->default_keepalives_count;
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index b9e8ff3122e..1c44b0d2f77 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.287 2005/08/29 21:38:18 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.288 2005/09/12 02:26:32 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -5884,10 +5884,8 @@ assign_canonical_path(const char *newval, bool doit, GucSource source)
 static bool
 assign_tcp_keepalives_idle(int newval, bool doit, GucSource source)
 {
-	if (doit && MyProcPort != NULL)
-	{
+	if (doit)
 		return (pq_setkeepalivesidle(newval, MyProcPort) == STATUS_OK);
-	}
 
 	return true;
 }
@@ -5895,18 +5893,17 @@ assign_tcp_keepalives_idle(int newval, bool doit, GucSource source)
 static const char *
 show_tcp_keepalives_idle(void)
 {
-	static char nbuf[32];
-	snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivesidle(MyProcPort));
+	static char nbuf[16];
+
+	snprintf(nbuf, sizeof(nbuf), "%d", pq_getkeepalivesidle(MyProcPort));
 	return nbuf;
 }
 
 static bool
 assign_tcp_keepalives_interval(int newval, bool doit, GucSource source)
 {
-	if (doit && MyProcPort != NULL)
-	{
+	if (doit)
 		return (pq_setkeepalivesinterval(newval, MyProcPort) == STATUS_OK);
-	}
 
 	return true;
 }
@@ -5914,18 +5911,17 @@ assign_tcp_keepalives_interval(int newval, bool doit, GucSource source)
 static const char *
 show_tcp_keepalives_interval(void)
 {
-	static char nbuf[32];
-	snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivesinterval(MyProcPort));
+	static char nbuf[16];
+
+	snprintf(nbuf, sizeof(nbuf), "%d", pq_getkeepalivesinterval(MyProcPort));
 	return nbuf;
 }
 
 static bool
 assign_tcp_keepalives_count(int newval, bool doit, GucSource source)
 {
-	if (doit && MyProcPort != NULL)
-	{
+	if (doit)
 		return (pq_setkeepalivescount(newval, MyProcPort) == STATUS_OK);
-	}
 
 	return true;
 }
@@ -5933,9 +5929,11 @@ assign_tcp_keepalives_count(int newval, bool doit, GucSource source)
 static const char *
 show_tcp_keepalives_count(void)
 {
-	static char nbuf[32];
-	snprintf(nbuf, sizeof(nbuf), "%d", MyProcPort == NULL ? 0 : pq_getkeepalivescount(MyProcPort));
+	static char nbuf[16];
+
+	snprintf(nbuf, sizeof(nbuf), "%d", pq_getkeepalivescount(MyProcPort));
 	return nbuf;
 }
 
+
 #include "guc-file.c"
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 509035ce3bf..44179f72f86 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -75,12 +75,14 @@
 
 # - TCP Keepalives -
 # see 'man 7 tcp' for details
-#tcp_keepalives_idle = 0		# TCP_KEEPIDLE, in secs; 0 uses the
-					# system default
-#tcp_keepalives_interval = 0		# TCP_KEEPINTVL, in seconds; 0 uses the
-					# system default
-#tcp_keepalives_count = 0		# TCP_KEEPCNT, in seconds; 0 uses the
-					# system default
+
+#tcp_keepalives_idle = 0		# TCP_KEEPIDLE, in seconds;
+					# 0 selects the system default
+#tcp_keepalives_interval = 0		# TCP_KEEPINTVL, in seconds;
+					# 0 selects the system default
+#tcp_keepalives_count = 0		# TCP_KEEPCNT;
+					# 0 selects the system default
+
 
 #---------------------------------------------------------------------------
 # RESOURCE USAGE (except WAL)
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index bbc218a0e1e..b7d8e365821 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/libpq/libpq-be.h,v 1.50 2005/07/30 15:17:25 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/libpq/libpq-be.h,v 1.51 2005/09/12 02:26:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -85,6 +85,21 @@ typedef struct Port
 	const char *commandTag;		/* current command tag */
 	struct timeval session_start;		/* for session duration logging */
 
+	/*
+	 * TCP keepalive settings.
+	 *
+	 *	default values are 0 if AF_UNIX or not yet known;
+	 *	current values are 0 if AF_UNIX or using the default.
+	 *	Also, -1 in a default value means we were unable to find out the
+	 *	default (getsockopt failed).
+	 */
+	int         default_keepalives_idle;
+	int         default_keepalives_interval;
+	int         default_keepalives_count;
+	int         keepalives_idle;
+	int         keepalives_interval;
+	int         keepalives_count;
+
 	/*
 	 * SSL structures
 	 */
@@ -95,24 +110,6 @@ typedef struct Port
 	char		peer_cn[SM_USER + 1];
 	unsigned long count;
 #endif
-
-	/*
-	 * TCP keepalive settings;
-	 *  default values are 0 if AF_UNIX or not yet known;
-	 *  current values are 0 if AF_UNIX or using the default.
-	 */
-#ifdef TCP_KEEPIDLE
-	int         default_keepalives_idle;
-	int         keepalives_idle;
-#endif
-#ifdef TCP_KEEPINTVL
-	int         default_keepalives_interval;
-	int         keepalives_interval;
-#endif
-#ifdef TCP_KEEPCNT
-	int         default_keepalives_count;
-	int         keepalives_count;
-#endif
 } Port;
 
 
-- 
GitLab