Second try at fixing tcp_keepalives_idle option on Solaris.

tglsfdc · tglsfdc · commit cc154d9a0022 · 2017-06-28T12:30:16.000-04:00
Buildfarm evidence shows that TCP_KEEPALIVE_THRESHOLD doesn't exist after all on Solaris < 11. This means we need to take positive action to prevent the TCP_KEEPALIVE code path from being taken on that platform. I've chosen to limit it with "&& defined(__darwin__)", since it's unclear that anyone else would follow Apple's precedent of spelling the symbol that way. Also, follow a suggestion from Michael Paquier of eliminating code duplication by defining a couple of intermediate symbols for the socket option. In passing, make some effort to reduce the number of translatable messages by replacing "setsockopt(foo) failed" with "setsockopt(%s) failed", etc, throughout the affected files. And update relevant documentation so that it doesn't claim to provide an exhaustive list of the possible socket option names. Like the previous commit (f0256c7), back-patch to all supported branches. Discussion: https://postgr.es/m/20170627163757.25161.528@wrigleys.postgresql.org
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
@@ -677,8 +677,8 @@ include 'filename'
        <para>
         Specifies the number of seconds before sending a keepalive packet on
         an otherwise idle connection.  A value of 0 uses the system default.
-        This parameter is supported only on systems that support the
-        <symbol>TCP_KEEPIDLE</> or <symbol>TCP_KEEPALIVE</> symbols, and on
+        This parameter is supported only on systems that support
+        <symbol>TCP_KEEPIDLE</> or an equivalent socket option, and on
         Windows; on other systems, it must be zero.
         In sessions connected via a Unix-domain socket, this parameter is
         ignored and always reads as zero.
@@ -701,9 +701,9 @@ include 'filename'
        <para>
         Specifies the number of seconds between sending keepalives on an
         otherwise idle connection.  A value of 0 uses the system default.
-        This parameter is supported only on systems that support the
-        <symbol>TCP_KEEPINTVL</> symbol, and on Windows; on other systems, it
-        must be zero.
+        This parameter is supported only on systems that support
+        <symbol>TCP_KEEPINTVL</> or an equivalent socket option, and on
+        Windows; on other systems, it must be zero.
         In sessions connected via a Unix-domain socket, this parameter is
         ignored and always reads as zero.
        </para>
@@ -724,9 +724,10 @@ include 'filename'
       <listitem>
        <para>
         Specifies the number of keepalive packets to send on an otherwise idle
-        connection.  A value of 0 uses the system default.  This parameter is
-        supported only on systems that support the <symbol>TCP_KEEPCNT</>
-        symbol; on other systems, it must be zero.
+        connection.  A value of 0 uses the system default.
+        This parameter is supported only on systems that support
+        <symbol>TCP_KEEPCNT</> or an equivalent socket option;
+        on other systems, it must be zero.
         In sessions connected via a Unix-domain socket, this parameter is
         ignored and always reads as zero.
        </para>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
@@ -1069,10 +1069,10 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         Controls the number of seconds of inactivity after which TCP should
         send a keepalive message to the server.  A value of zero uses the
         system default. This parameter is ignored for connections made via a
-        Unix-domain socket, or if keepalives are disabled. It is only supported
-        on systems where the <symbol>TCP_KEEPIDLE</> or <symbol>TCP_KEEPALIVE</>
-        socket option is available, and on Windows; on other systems, it has no
-        effect.
+        Unix-domain socket, or if keepalives are disabled.
+        It is only supported on systems where <symbol>TCP_KEEPIDLE</> or
+        an equivalent socket option is available, and on Windows; on other
+        systems, it has no effect.
        </para>
       </listitem>
      </varlistentry>
@@ -1085,9 +1085,9 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         that is not acknowledged by the server should be retransmitted.  A
         value of zero uses the system default. This parameter is ignored for
         connections made via a Unix-domain socket, or if keepalives are disabled.
-        It is only supported on systems where the <symbol>TCP_KEEPINTVL</>
-        socket option is available, and on Windows; on other systems, it has no
-        effect.
+        It is only supported on systems where <symbol>TCP_KEEPINTVL</> or
+        an equivalent socket option is available, and on Windows; on other
+        systems, it has no effect.
        </para>
       </listitem>
      </varlistentry>
@@ -1100,8 +1100,9 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
         client's connection to the server is considered dead.  A value of
         zero uses the system default. This parameter is ignored for
         connections made via a Unix-domain socket, or if keepalives are disabled.
-        It is only supported on systems where the <symbol>TCP_KEEPCNT</>
-        socket option is available; on other systems, it has no effect.
+        It is only supported on systems where <symbol>TCP_KEEPCNT</> or
+        an equivalent socket option is available; on other systems, it has no
+        effect.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
@@ -96,6 +96,25 @@
 #include "utils/guc.h"
 #include "utils/memutils.h"
 
+/*
+ * Cope with the various platform-specific ways to spell TCP keepalive socket
+ * options.  This doesn't cover Windows, which as usual does its own thing.
+ */
+#if defined(TCP_KEEPIDLE)
+/* TCP_KEEPIDLE is the name of this option on Linux and *BSD */
+#define PG_TCP_KEEPALIVE_IDLE TCP_KEEPIDLE
+#define PG_TCP_KEEPALIVE_IDLE_STR "TCP_KEEPIDLE"
+#elif defined(TCP_KEEPALIVE_THRESHOLD)
+/* TCP_KEEPALIVE_THRESHOLD is the name of this option on Solaris >= 11 */
+#define PG_TCP_KEEPALIVE_IDLE TCP_KEEPALIVE_THRESHOLD
+#define PG_TCP_KEEPALIVE_IDLE_STR "TCP_KEEPALIVE_THRESHOLD"
+#elif defined(TCP_KEEPALIVE) && defined(__darwin__)
+/* TCP_KEEPALIVE is the name of this option on macOS */
+/* Caution: Solaris has this symbol but it means something different */
+#define PG_TCP_KEEPALIVE_IDLE TCP_KEEPALIVE
+#define PG_TCP_KEEPALIVE_IDLE_STR "TCP_KEEPALIVE"
+#endif
+
 /*
  * Configuration options
  */
@@ -415,7 +434,8 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
 			{
 				ereport(LOG,
 						(errcode_for_socket_access(),
-						 errmsg("setsockopt(SO_REUSEADDR) failed: %m")));
+						 errmsg("setsockopt(%s) failed: %m",
+								"SO_REUSEADDR")));
 				closesocket(fd);
 				continue;
 			}
@@ -430,7 +450,8 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber,
 			{
 				ereport(LOG,
 						(errcode_for_socket_access(),
-						 errmsg("setsockopt(IPV6_V6ONLY) failed: %m")));
+						 errmsg("setsockopt(%s) failed: %m",
+								"IPV6_V6ONLY")));
 				closesocket(fd);
 				continue;
 			}
@@ -667,15 +688,15 @@ StreamConnection(pgsocket server_fd, Port *port)
 		if (setsockopt(port->sock, IPPROTO_TCP, TCP_NODELAY,
 					   (char *) &on, sizeof(on)) < 0)
 		{
-			elog(LOG, "setsockopt(TCP_NODELAY) failed: %m");
+			elog(LOG, "setsockopt(%s) failed: %m", "TCP_NODELAY");
 			return STATUS_ERROR;
 		}
 #endif
 		on = 1;
 		if (setsockopt(port->sock, SOL_SOCKET, SO_KEEPALIVE,
 					   (char *) &on, sizeof(on)) < 0)
 		{
-			elog(LOG, "setsockopt(SO_KEEPALIVE) failed: %m");
+			elog(LOG, "setsockopt(%s) failed: %m", "SO_KEEPALIVE");
 			return STATUS_ERROR;
 		}
 
@@ -689,7 +710,7 @@ StreamConnection(pgsocket server_fd, Port *port)
 		if (setsockopt(port->sock, SOL_SOCKET, SO_SNDBUF, (char *) &on,
 					   sizeof(on)) < 0)
 		{
-			elog(LOG, "setsockopt(SO_SNDBUF) failed: %m");
+			elog(LOG, "setsockopt(%s) failed: %m", "SO_SNDBUF");
 			return STATUS_ERROR;
 		}
 #endif
@@ -1593,7 +1614,7 @@ pq_setkeepaliveswin32(Port *port, int idle, int interval)
 int
 pq_getkeepalivesidle(Port *port)
 {
-#if defined(TCP_KEEPIDLE) || defined(TCP_KEEPALIVE_THRESHOLD) || defined(TCP_KEEPALIVE) || defined(WIN32)
+#if defined(PG_TCP_KEEPALIVE_IDLE) || defined(SIO_KEEPALIVE_VALS)
 	if (port == NULL || IS_AF_UNIX(port->laddr.addr.ss_family))
 		return 0;
 
@@ -1605,34 +1626,13 @@ pq_getkeepalivesidle(Port *port)
 #ifndef WIN32
 		ACCEPT_TYPE_ARG3 size = sizeof(port->default_keepalives_idle);
 
-#if defined(TCP_KEEPIDLE)
-		/* TCP_KEEPIDLE is the name of this option on Linux and *BSD */
-		if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPIDLE,
-					   (char *) &port->default_keepalives_idle,
-					   &size) < 0)
-		{
-			elog(LOG, "getsockopt(TCP_KEEPIDLE) failed: %m");
-			port->default_keepalives_idle = -1; /* don't know */
-		}
-#elif defined(TCP_KEEPALIVE_THRESHOLD)
-		/* TCP_KEEPALIVE_THRESHOLD is the name of this option on Solaris */
-		if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPALIVE_THRESHOLD,
+		if (getsockopt(port->sock, IPPROTO_TCP, PG_TCP_KEEPALIVE_IDLE,
 					   (char *) &port->default_keepalives_idle,
 					   &size) < 0)
 		{
-			elog(LOG, "getsockopt(TCP_KEEPALIVE_THRESHOLD) failed: %m");
+			elog(LOG, "getsockopt(%s) failed: %m", PG_TCP_KEEPALIVE_IDLE_STR);
 			port->default_keepalives_idle = -1; /* don't know */
 		}
-#else							/* must have TCP_KEEPALIVE */
-		/* TCP_KEEPALIVE is the name of this option on macOS */
-		if (getsockopt(port->sock, IPPROTO_TCP, TCP_KEEPALIVE,
-					   (char *) &port->default_keepalives_idle,
-					   &size) < 0)
-		{
-			elog(LOG, "getsockopt(TCP_KEEPALIVE) failed: %m");
-			port->default_keepalives_idle = -1; /* don't know */
-		}
-#endif   /* KEEPIDLE/KEEPALIVE_THRESHOLD/KEEPALIVE */
 #else							/* WIN32 */
 		/* We can't get the defaults on Windows, so return "don't know" */
 		port->default_keepalives_idle = -1;
@@ -1652,7 +1652,7 @@ pq_setkeepalivesidle(int idle, Port *port)
 		return STATUS_OK;
 
 /* check SIO_KEEPALIVE_VALS here, not just WIN32, as some toolchains lack it */
-#if defined(TCP_KEEPIDLE) || defined(TCP_KEEPALIVE_THRESHOLD) || defined(TCP_KEEPALIVE) || defined(SIO_KEEPALIVE_VALS)
+#if defined(PG_TCP_KEEPALIVE_IDLE) || defined(SIO_KEEPALIVE_VALS)
 	if (idle == port->keepalives_idle)
 		return STATUS_OK;
 
@@ -1671,43 +1671,25 @@ pq_setkeepalivesidle(int idle, Port *port)
 	if (idle == 0)
 		idle = port->default_keepalives_idle;
 
-#if defined(TCP_KEEPIDLE)
-	/* TCP_KEEPIDLE is the name of this option on Linux and *BSD */
-	if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPIDLE,
-				   (char *) &idle, sizeof(idle)) < 0)
-	{
-		elog(LOG, "setsockopt(TCP_KEEPIDLE) failed: %m");
-		return STATUS_ERROR;
-	}
-#elif defined(TCP_KEEPALIVE_THRESHOLD)
-	/* TCP_KEEPALIVE_THRESHOLD is the name of this option on Solaris */
-	if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPALIVE_THRESHOLD,
+	if (setsockopt(port->sock, IPPROTO_TCP, PG_TCP_KEEPALIVE_IDLE,
 				   (char *) &idle, sizeof(idle)) < 0)
 	{
-		elog(LOG, "setsockopt(TCP_KEEPALIVE_THRESHOLD) failed: %m");
+		elog(LOG, "setsockopt(%s) failed: %m", PG_TCP_KEEPALIVE_IDLE_STR);
 		return STATUS_ERROR;
 	}
-#else							/* must have TCP_KEEPALIVE */
-	/* TCP_KEEPALIVE is the name of this option on macOS */
-	if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPALIVE,
-				   (char *) &idle, sizeof(idle)) < 0)
-	{
-		elog(LOG, "setsockopt(TCP_KEEPALIVE) failed: %m");
-		return STATUS_ERROR;
-	}
-#endif
 
 	port->keepalives_idle = idle;
 #else							/* WIN32 */
 	return pq_setkeepaliveswin32(port, idle, port->keepalives_interval);
 #endif
-#else							/* no way to set it */
+#else
 	if (idle != 0)
 	{
 		elog(LOG, "setting the keepalive idle time is not supported");
 		return STATUS_ERROR;
 	}
 #endif
+
 	return STATUS_OK;
 }
 
@@ -1730,7 +1712,7 @@ pq_getkeepalivesinterval(Port *port)
 					   (char *) &port->default_keepalives_interval,
 					   &size) < 0)
 		{
-			elog(LOG, "getsockopt(TCP_KEEPINTVL) failed: %m");
+			elog(LOG, "getsockopt(%s) failed: %m", "TCP_KEEPINTVL");
 			port->default_keepalives_interval = -1;		/* don't know */
 		}
 #else
@@ -1773,7 +1755,7 @@ pq_setkeepalivesinterval(int interval, Port *port)
 	if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPINTVL,
 				   (char *) &interval, sizeof(interval)) < 0)
 	{
-		elog(LOG, "setsockopt(TCP_KEEPINTVL) failed: %m");
+		elog(LOG, "setsockopt(%s) failed: %m", "TCP_KEEPINTVL");
 		return STATUS_ERROR;
 	}
 
@@ -1784,7 +1766,7 @@ pq_setkeepalivesinterval(int interval, Port *port)
 #else
 	if (interval != 0)
 	{
-		elog(LOG, "setsockopt(TCP_KEEPINTVL) not supported");
+		elog(LOG, "setsockopt(%s) not supported", "TCP_KEEPINTVL");
 		return STATUS_ERROR;
 	}
 #endif
@@ -1810,7 +1792,7 @@ pq_getkeepalivescount(Port *port)
 					   (char *) &port->default_keepalives_count,
 					   &size) < 0)
 		{
-			elog(LOG, "getsockopt(TCP_KEEPCNT) failed: %m");
+			elog(LOG, "getsockopt(%s) failed: %m", "TCP_KEEPCNT");
 			port->default_keepalives_count = -1;		/* don't know */
 		}
 	}
@@ -1848,15 +1830,15 @@ pq_setkeepalivescount(int count, Port *port)
 	if (setsockopt(port->sock, IPPROTO_TCP, TCP_KEEPCNT,
 				   (char *) &count, sizeof(count)) < 0)
 	{
-		elog(LOG, "setsockopt(TCP_KEEPCNT) failed: %m");
+		elog(LOG, "setsockopt(%s) failed: %m", "TCP_KEEPCNT");
 		return STATUS_ERROR;
 	}
 
 	port->keepalives_count = count;
 #else
 	if (count != 0)
 	{
-		elog(LOG, "setsockopt(TCP_KEEPCNT) not supported");
+		elog(LOG, "setsockopt(%s) not supported", "TCP_KEEPCNT");
 		return STATUS_ERROR;
 	}
 #endif
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
