Commit fe27009c authored by Tom Lane's avatar Tom Lane

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 32a9c0bd,
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
parent ed30b1a6
...@@ -120,13 +120,21 @@ TranslateSocketError(void) ...@@ -120,13 +120,21 @@ TranslateSocketError(void)
case WSAEADDRNOTAVAIL: case WSAEADDRNOTAVAIL:
errno = EADDRNOTAVAIL; errno = EADDRNOTAVAIL;
break; break;
case WSAEHOSTUNREACH:
case WSAEHOSTDOWN: case WSAEHOSTDOWN:
errno = EHOSTDOWN;
break;
case WSAEHOSTUNREACH:
case WSAHOST_NOT_FOUND: case WSAHOST_NOT_FOUND:
errno = EHOSTUNREACH;
break;
case WSAENETDOWN: case WSAENETDOWN:
errno = ENETDOWN;
break;
case WSAENETUNREACH: case WSAENETUNREACH:
errno = ENETUNREACH;
break;
case WSAENETRESET: case WSAENETRESET:
errno = EHOSTUNREACH; errno = ENETRESET;
break; break;
case WSAENOTCONN: case WSAENOTCONN:
case WSAESHUTDOWN: case WSAESHUTDOWN:
......
...@@ -711,10 +711,7 @@ errcode_for_socket_access(void) ...@@ -711,10 +711,7 @@ errcode_for_socket_access(void)
switch (edata->saved_errno) switch (edata->saved_errno)
{ {
/* Loss of connection */ /* Loss of connection */
case EPIPE: case ALL_CONNECTION_FAILURE_ERRNOS:
#ifdef ECONNRESET
case ECONNRESET:
#endif
edata->sqlerrcode = ERRCODE_CONNECTION_FAILURE; edata->sqlerrcode = ERRCODE_CONNECTION_FAILURE;
break; break;
......
...@@ -1825,10 +1825,15 @@ piperead(int s, char *buf, int len) ...@@ -1825,10 +1825,15 @@ piperead(int s, char *buf, int len)
{ {
int ret = recv(s, buf, len, 0); int ret = recv(s, buf, len, 0);
if (ret < 0 && WSAGetLastError() == WSAECONNRESET) if (ret < 0)
{ {
/* EOF on the pipe! */ switch (TranslateSocketError())
ret = 0; {
case ALL_CONNECTION_FAILURE_ERRNOS:
/* Treat connection loss as EOF on the pipe */
ret = 0;
break;
}
} }
return ret; return ret;
} }
......
...@@ -99,6 +99,28 @@ extern void pgfnames_cleanup(char **filenames); ...@@ -99,6 +99,28 @@ extern void pgfnames_cleanup(char **filenames);
) )
#endif #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) */ /* Portable locale initialization (in exec.c) */
extern void set_pglocale_pgservice(const char *argv0, const char *app); extern void set_pglocale_pgservice(const char *argv0, const char *app);
......
...@@ -369,8 +369,16 @@ extern int _pgstat64(const char *name, struct stat *buf); ...@@ -369,8 +369,16 @@ extern int _pgstat64(const char *name, struct stat *buf);
#define EADDRINUSE WSAEADDRINUSE #define EADDRINUSE WSAEADDRINUSE
#undef EADDRNOTAVAIL #undef EADDRNOTAVAIL
#define EADDRNOTAVAIL WSAEADDRNOTAVAIL #define EADDRNOTAVAIL WSAEADDRNOTAVAIL
#undef EHOSTDOWN
#define EHOSTDOWN WSAEHOSTDOWN
#undef EHOSTUNREACH #undef EHOSTUNREACH
#define EHOSTUNREACH WSAEHOSTUNREACH #define EHOSTUNREACH WSAEHOSTUNREACH
#undef ENETDOWN
#define ENETDOWN WSAENETDOWN
#undef ENETRESET
#define ENETRESET WSAENETRESET
#undef ENETUNREACH
#define ENETUNREACH WSAENETUNREACH
#undef ENOTCONN #undef ENOTCONN
#define ENOTCONN WSAENOTCONN #define ENOTCONN WSAENOTCONN
......
...@@ -668,24 +668,29 @@ retry3: ...@@ -668,24 +668,29 @@ retry3:
conn->inBufSize - conn->inEnd); conn->inBufSize - conn->inEnd);
if (nread < 0) if (nread < 0)
{ {
if (SOCK_ERRNO == EINTR) switch (SOCK_ERRNO)
goto retry3; {
/* Some systems return EAGAIN/EWOULDBLOCK for no data */ case EINTR:
goto retry3;
/* Some systems return EAGAIN/EWOULDBLOCK for no data */
#ifdef EAGAIN #ifdef EAGAIN
if (SOCK_ERRNO == EAGAIN) case EAGAIN:
return someread; return someread;
#endif #endif
#if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN)) #if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
if (SOCK_ERRNO == EWOULDBLOCK) case EWOULDBLOCK:
return someread; return someread;
#endif #endif
/* We might get ECONNRESET here if using TCP and backend died */
#ifdef ECONNRESET /* We might get ECONNRESET etc here if connection failed */
if (SOCK_ERRNO == ECONNRESET) case ALL_CONNECTION_FAILURE_ERRNOS:
goto definitelyFailed; goto definitelyFailed;
#endif
/* pqsecure_read set the error message for us */ default:
return -1; /* pqsecure_read set the error message for us */
return -1;
}
} }
if (nread > 0) if (nread > 0)
{ {
...@@ -758,24 +763,29 @@ retry4: ...@@ -758,24 +763,29 @@ retry4:
conn->inBufSize - conn->inEnd); conn->inBufSize - conn->inEnd);
if (nread < 0) if (nread < 0)
{ {
if (SOCK_ERRNO == EINTR) switch (SOCK_ERRNO)
goto retry4; {
/* Some systems return EAGAIN/EWOULDBLOCK for no data */ case EINTR:
goto retry4;
/* Some systems return EAGAIN/EWOULDBLOCK for no data */
#ifdef EAGAIN #ifdef EAGAIN
if (SOCK_ERRNO == EAGAIN) case EAGAIN:
return 0; return 0;
#endif #endif
#if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN)) #if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
if (SOCK_ERRNO == EWOULDBLOCK) case EWOULDBLOCK:
return 0; return 0;
#endif #endif
/* We might get ECONNRESET here if using TCP and backend died */
#ifdef ECONNRESET /* We might get ECONNRESET etc here if connection failed */
if (SOCK_ERRNO == ECONNRESET) case ALL_CONNECTION_FAILURE_ERRNOS:
goto definitelyFailed; goto definitelyFailed;
#endif
/* pqsecure_read set the error message for us */ default:
return -1; /* pqsecure_read set the error message for us */
return -1;
}
} }
if (nread > 0) if (nread > 0)
{ {
......
...@@ -261,14 +261,13 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len) ...@@ -261,14 +261,13 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
/* no error message, caller is expected to retry */ /* no error message, caller is expected to retry */
break; break;
#ifdef ECONNRESET case EPIPE:
case ECONNRESET: case ECONNRESET:
printfPQExpBuffer(&conn->errorMessage, printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("server closed the connection unexpectedly\n" libpq_gettext("server closed the connection unexpectedly\n"
"\tThis probably means the server terminated abnormally\n" "\tThis probably means the server terminated abnormally\n"
"\tbefore or while processing the request.\n")); "\tbefore or while processing the request.\n"));
break; break;
#endif
default: default:
printfPQExpBuffer(&conn->errorMessage, printfPQExpBuffer(&conn->errorMessage,
...@@ -374,11 +373,9 @@ retry_masked: ...@@ -374,11 +373,9 @@ retry_masked:
/* Set flag for EPIPE */ /* Set flag for EPIPE */
REMEMBER_EPIPE(spinfo, true); REMEMBER_EPIPE(spinfo, true);
#ifdef ECONNRESET
/* FALL THRU */ /* FALL THRU */
case ECONNRESET: case ECONNRESET:
#endif
printfPQExpBuffer(&conn->errorMessage, printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("server closed the connection unexpectedly\n" libpq_gettext("server closed the connection unexpectedly\n"
"\tThis probably means the server terminated abnormally\n" "\tThis probably means the server terminated abnormally\n"
......
...@@ -14,17 +14,6 @@ ...@@ -14,17 +14,6 @@
#define write(a,b,c) _write(a,b,c) #define write(a,b,c) _write(a,b,c)
#undef EAGAIN /* doesn't apply on sockets */ #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 * support for handling Windows Socket errors
......
...@@ -146,16 +146,12 @@ get_errno_symbol(int errnum) ...@@ -146,16 +146,12 @@ get_errno_symbol(int errnum)
return "EBUSY"; return "EBUSY";
case ECHILD: case ECHILD:
return "ECHILD"; return "ECHILD";
#ifdef ECONNABORTED
case ECONNABORTED: case ECONNABORTED:
return "ECONNABORTED"; return "ECONNABORTED";
#endif
case ECONNREFUSED: case ECONNREFUSED:
return "ECONNREFUSED"; return "ECONNREFUSED";
#ifdef ECONNRESET
case ECONNRESET: case ECONNRESET:
return "ECONNRESET"; return "ECONNRESET";
#endif
case EDEADLK: case EDEADLK:
return "EDEADLK"; return "EDEADLK";
case EDOM: case EDOM:
...@@ -166,10 +162,10 @@ get_errno_symbol(int errnum) ...@@ -166,10 +162,10 @@ get_errno_symbol(int errnum)
return "EFAULT"; return "EFAULT";
case EFBIG: case EFBIG:
return "EFBIG"; return "EFBIG";
#ifdef EHOSTUNREACH case EHOSTDOWN:
return "EHOSTDOWN";
case EHOSTUNREACH: case EHOSTUNREACH:
return "EHOSTUNREACH"; return "EHOSTUNREACH";
#endif
case EIDRM: case EIDRM:
return "EIDRM"; return "EIDRM";
case EINPROGRESS: case EINPROGRESS:
...@@ -198,6 +194,12 @@ get_errno_symbol(int errnum) ...@@ -198,6 +194,12 @@ get_errno_symbol(int errnum)
return "EMSGSIZE"; return "EMSGSIZE";
case ENAMETOOLONG: case ENAMETOOLONG:
return "ENAMETOOLONG"; return "ENAMETOOLONG";
case ENETDOWN:
return "ENETDOWN";
case ENETRESET:
return "ENETRESET";
case ENETUNREACH:
return "ENETUNREACH";
case ENFILE: case ENFILE:
return "ENFILE"; return "ENFILE";
case ENOBUFS: case ENOBUFS:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment