diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2020-10-10 13:28:12 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2020-10-10 13:28:12 -0400 |
commit | fe27009cbb5fff53f87969786ca8dac82ed7deba (patch) | |
tree | 953d89b695115498aa6d4d4235391453bfab3beb /src/interfaces/libpq | |
parent | ed30b1a60dadf2b7cc58bce5009ad8676b8fe479 (diff) | |
download | postgresql-fe27009cbb5fff53f87969786ca8dac82ed7deba.tar.gz |
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
Diffstat (limited to 'src/interfaces/libpq')
-rw-r--r-- | src/interfaces/libpq/fe-misc.c | 66 | ||||
-rw-r--r-- | src/interfaces/libpq/fe-secure.c | 5 | ||||
-rw-r--r-- | src/interfaces/libpq/win32.h | 11 |
3 files changed, 39 insertions, 43 deletions
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index ff840b7730..4ffc7f33fb 100644 --- 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 3311fd7a5b..97c3805303 100644 --- 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 c42d7abfe3..fcce1e0544 100644 --- 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 |