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.

diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 6c2f6fd14e81c291740b51521164cede2cc3e83c..23a9865acb561caec3aa639517280b09dcbd4077 100644 (file)
--- 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 b9e8ff3122e99e950d98a04e5ffa2babd5dfbfe1..1c44b0d2f77a0044f7fcdbb2822ae5890123a6e9 100644 (file)
--- 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 509035ce3bf9762d7bd0f3d2ce2930599ee6669d..44179f72f86a86dc86c77d945e93edabb969ef29 100644 (file)
--- 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 bbc218a0e1e5fdf1b25cb1ed1166e7eaeb84cf56..b7d8e3658218572d6bb2fe335089167bc6af2e64 100644 (file)
--- 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;