Recognize network-failure errnos as indicating hard connection loss.

Up to now, only ECONNRESET (and EPIPE, in most but not quite all places)
received special treatment in our error handling logic.  This patch
changes things so that related error codes such as ECONNABORTED are
also recognized as indicating that the connection's dead and unlikely
to come back.

We continue to think, however, that only ECONNRESET and EPIPE should be
reported as probable server crashes; the other cases indicate network
connectivity problems but prove little about the server's state.  Thus,
there's no change in the error message texts that are output for such
cases.  The key practical effect is that errcode_for_socket_access()
will report ERRCODE_CONNECTION_FAILURE rather than
ERRCODE_INTERNAL_ERROR for a network failure.  It's expected that this
will fix buildfarm member lorikeet's failures since commit 32a9c0bdf,
as that seems to be due to not treating ECONNABORTED equivalently to
ECONNRESET.

The set of errnos treated this way now includes ECONNABORTED, EHOSTDOWN,
EHOSTUNREACH, ENETDOWN, ENETRESET, and ENETUNREACH.  Several of these
were second-class citizens in terms of their handling in places like
get_errno_symbol(), so upgrade the infrastructure where necessary.

As committed, this patch assumes that all these symbols are defined
everywhere.  POSIX specifies all of them except EHOSTDOWN, but that
seems to exist on all platforms of interest; we'll see what the
buildfarm says about that.

Probably this should be back-patched, but let's see what the buildfarm
thinks of it first.

Fujii Masao and Tom Lane

Discussion: https://postgr.es/m/2621622.1602184554@sss.pgh.pa.us

diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c
index 6fbd1ed6fb49f5ecc6f98c0aca9c98f248007783..7c7611a01e233a83f8e966ffc257179a08fbedeb 100644 (file)
--- a/src/backend/port/win32/socket.c
+++ b/src/backend/port/win32/socket.c
@@ -120,13 +120,21 @@ TranslateSocketError(void)
        case WSAEADDRNOTAVAIL:
            errno = EADDRNOTAVAIL;
            break;
-       case WSAEHOSTUNREACH:
        case WSAEHOSTDOWN:
+           errno = EHOSTDOWN;
+           break;
+       case WSAEHOSTUNREACH:
        case WSAHOST_NOT_FOUND:
+           errno = EHOSTUNREACH;
+           break;
        case WSAENETDOWN:
+           errno = ENETDOWN;
+           break;
        case WSAENETUNREACH:
+           errno = ENETUNREACH;
+           break;
        case WSAENETRESET:
-           errno = EHOSTUNREACH;
+           errno = ENETRESET;
            break;
        case WSAENOTCONN:
        case WSAESHUTDOWN:

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index d0b368530e7e3764ad42029c3d69f6b5b02d5ea3..1ba47c194b2f7b6342edf9ea4ee003cff0b476e0 100644 (file)
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -711,10 +711,7 @@ errcode_for_socket_access(void)
    switch (edata->saved_errno)
    {
            /* Loss of connection */
-       case EPIPE:
-#ifdef ECONNRESET
-       case ECONNRESET:
-#endif
+       case ALL_CONNECTION_FAILURE_ERRNOS:
            edata->sqlerrcode = ERRCODE_CONNECTION_FAILURE;
            break;
 

diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index f0587f41e49253586565cf2311431773fc7477eb..4b38ed6c5a911614791c1900ccc5f028b881baeb 100644 (file)
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -1825,10 +1825,15 @@ piperead(int s, char *buf, int len)
 {
    int         ret = recv(s, buf, len, 0);
 
-   if (ret < 0 && WSAGetLastError() == WSAECONNRESET)
+   if (ret < 0)
    {
-       /* EOF on the pipe! */
-       ret = 0;
+       switch (TranslateSocketError())
+       {
+           case ALL_CONNECTION_FAILURE_ERRNOS:
+               /* Treat connection loss as EOF on the pipe */
+               ret = 0;
+               break;
+       }
    }
    return ret;
 }

diff --git a/src/include/port.h b/src/include/port.h
index 84bf2c363f55afa4a2f2f76e5cbd0a20b5f5f33a..d25716bf7f8368a5d4664ca0e856446d2fb4eaac 100644 (file)
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -99,6 +99,28 @@ extern void pgfnames_cleanup(char **filenames);
 )
 #endif
 
+/*
+ * This macro provides a centralized list of all errnos that identify
+ * hard failure of a previously-established network connection.
+ * The macro is intended to be used in a switch statement, in the form
+ * "case ALL_CONNECTION_FAILURE_ERRNOS:".
+ *
+ * Note: this groups EPIPE and ECONNRESET, which we take to indicate a
+ * probable server crash, with other errors that indicate loss of network
+ * connectivity without proving much about the server's state.  Places that
+ * are actually reporting errors typically single out EPIPE and ECONNRESET,
+ * while allowing the network failures to be reported generically.
+ */
+#define ALL_CONNECTION_FAILURE_ERRNOS \
+   EPIPE: \
+   case ECONNRESET: \
+   case ECONNABORTED: \
+   case EHOSTDOWN: \
+   case EHOSTUNREACH: \
+   case ENETDOWN: \
+   case ENETRESET: \
+   case ENETUNREACH
+
 /* Portable locale initialization (in exec.c) */
 extern void set_pglocale_pgservice(const char *argv0, const char *app);
 

diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index f65f426cdbd9a219af335582d7933140dc0ab9c5..59c7f35e3dfab0c759e7258d8f40ee6f800637c3 100644 (file)
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -369,8 +369,16 @@ extern int _pgstat64(const char *name, struct stat *buf);
 #define EADDRINUSE WSAEADDRINUSE
 #undef EADDRNOTAVAIL
 #define EADDRNOTAVAIL WSAEADDRNOTAVAIL
+#undef EHOSTDOWN
+#define EHOSTDOWN WSAEHOSTDOWN
 #undef EHOSTUNREACH
 #define EHOSTUNREACH WSAEHOSTUNREACH
+#undef ENETDOWN
+#define ENETDOWN WSAENETDOWN
+#undef ENETRESET
+#define ENETRESET WSAENETRESET
+#undef ENETUNREACH
+#define ENETUNREACH WSAENETUNREACH
 #undef ENOTCONN
 #define ENOTCONN WSAENOTCONN
 

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index ff840b7730d81d765f9a457fad685fc8f660c998..4ffc7f33fb5e742eb7cba246d1c6c7d243c8e9f9 100644 (file)
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -668,24 +668,29 @@ retry3:
                          conn->inBufSize - conn->inEnd);
    if (nread < 0)
    {
-       if (SOCK_ERRNO == EINTR)
-           goto retry3;
-       /* Some systems return EAGAIN/EWOULDBLOCK for no data */
+       switch (SOCK_ERRNO)
+       {
+           case EINTR:
+               goto retry3;
+
+               /* Some systems return EAGAIN/EWOULDBLOCK for no data */
 #ifdef EAGAIN
-       if (SOCK_ERRNO == EAGAIN)
-           return someread;
+           case EAGAIN:
+               return someread;
 #endif
 #if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
-       if (SOCK_ERRNO == EWOULDBLOCK)
-           return someread;
+           case EWOULDBLOCK:
+               return someread;
 #endif
-       /* We might get ECONNRESET here if using TCP and backend died */
-#ifdef ECONNRESET
-       if (SOCK_ERRNO == ECONNRESET)
-           goto definitelyFailed;
-#endif
-       /* pqsecure_read set the error message for us */
-       return -1;
+
+               /* We might get ECONNRESET etc here if connection failed */
+           case ALL_CONNECTION_FAILURE_ERRNOS:
+               goto definitelyFailed;
+
+           default:
+               /* pqsecure_read set the error message for us */
+               return -1;
+       }
    }
    if (nread > 0)
    {
@@ -758,24 +763,29 @@ retry4:
                          conn->inBufSize - conn->inEnd);
    if (nread < 0)
    {
-       if (SOCK_ERRNO == EINTR)
-           goto retry4;
-       /* Some systems return EAGAIN/EWOULDBLOCK for no data */
+       switch (SOCK_ERRNO)
+       {
+           case EINTR:
+               goto retry4;
+
+               /* Some systems return EAGAIN/EWOULDBLOCK for no data */
 #ifdef EAGAIN
-       if (SOCK_ERRNO == EAGAIN)
-           return 0;
+           case EAGAIN:
+               return 0;
 #endif
 #if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
-       if (SOCK_ERRNO == EWOULDBLOCK)
-           return 0;
+           case EWOULDBLOCK:
+               return 0;
 #endif
-       /* We might get ECONNRESET here if using TCP and backend died */
-#ifdef ECONNRESET
-       if (SOCK_ERRNO == ECONNRESET)
-           goto definitelyFailed;
-#endif
-       /* pqsecure_read set the error message for us */
-       return -1;
+
+               /* We might get ECONNRESET etc here if connection failed */
+           case ALL_CONNECTION_FAILURE_ERRNOS:
+               goto definitelyFailed;
+
+           default:
+               /* pqsecure_read set the error message for us */
+               return -1;
+       }
    }
    if (nread > 0)
    {

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 3311fd7a5bdaeb14725e6add1228495b9e4e7360..97c3805303f54a971ea25eb862851a3f2def61dc 100644 (file)
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -261,14 +261,13 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
                /* no error message, caller is expected to retry */
                break;
 
-#ifdef ECONNRESET
+           case EPIPE:
            case ECONNRESET:
                printfPQExpBuffer(&conn->errorMessage,
                                  libpq_gettext("server closed the connection unexpectedly\n"
                                                "\tThis probably means the server terminated abnormally\n"
                                                "\tbefore or while processing the request.\n"));
                break;
-#endif
 
            default:
                printfPQExpBuffer(&conn->errorMessage,
@@ -374,11 +373,9 @@ retry_masked:
                /* Set flag for EPIPE */
                REMEMBER_EPIPE(spinfo, true);
 
-#ifdef ECONNRESET
                /* FALL THRU */
 
            case ECONNRESET:
-#endif
                printfPQExpBuffer(&conn->errorMessage,
                                  libpq_gettext("server closed the connection unexpectedly\n"
                                                "\tThis probably means the server terminated abnormally\n"

diff --git a/src/interfaces/libpq/win32.h b/src/interfaces/libpq/win32.h
index c42d7abfe30a45efa0796eec1cd8bc2e13483f31..fcce1e0544a7c3512145f96b9223e83580038d8d 100644 (file)
--- a/src/interfaces/libpq/win32.h
+++ b/src/interfaces/libpq/win32.h
@@ -14,17 +14,6 @@
 #define write(a,b,c) _write(a,b,c)
 
 #undef EAGAIN                  /* doesn't apply on sockets */
-#undef EINTR
-#define EINTR WSAEINTR
-#ifndef EWOULDBLOCK
-#define EWOULDBLOCK WSAEWOULDBLOCK
-#endif
-#ifndef ECONNRESET
-#define ECONNRESET WSAECONNRESET
-#endif
-#ifndef EINPROGRESS
-#define EINPROGRESS WSAEINPROGRESS
-#endif
 
 /*
  * support for handling Windows Socket errors

diff --git a/src/port/strerror.c b/src/port/strerror.c
index 375edb0f5abde7faa7e286787528dd75db005cfe..43a9761d90bd2bf359e11df6709ec04e22b5945c 100644 (file)
--- a/src/port/strerror.c
+++ b/src/port/strerror.c
@@ -146,16 +146,12 @@ get_errno_symbol(int errnum)
            return "EBUSY";
        case ECHILD:
            return "ECHILD";
-#ifdef ECONNABORTED
        case ECONNABORTED:
            return "ECONNABORTED";
-#endif
        case ECONNREFUSED:
            return "ECONNREFUSED";
-#ifdef ECONNRESET
        case ECONNRESET:
            return "ECONNRESET";
-#endif
        case EDEADLK:
            return "EDEADLK";
        case EDOM:
@@ -166,10 +162,10 @@ get_errno_symbol(int errnum)
            return "EFAULT";
        case EFBIG:
            return "EFBIG";
-#ifdef EHOSTUNREACH
+       case EHOSTDOWN:
+           return "EHOSTDOWN";
        case EHOSTUNREACH:
            return "EHOSTUNREACH";
-#endif
        case EIDRM:
            return "EIDRM";
        case EINPROGRESS:
@@ -198,6 +194,12 @@ get_errno_symbol(int errnum)
            return "EMSGSIZE";
        case ENAMETOOLONG:
            return "ENAMETOOLONG";
+       case ENETDOWN:
+           return "ENETDOWN";
+       case ENETRESET:
+           return "ENETRESET";
+       case ENETUNREACH:
+           return "ENETUNREACH";
        case ENFILE:
            return "ENFILE";
        case ENOBUFS: