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

@@ -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;
 

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"

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)

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;