Skip to content

Commit fe27009

Browse files
tglsfdctglsfdc
committed
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 32a9c0b, 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
1 parent ed30b1a commit fe27009

File tree

9 files changed

+96
-58
lines changed

9 files changed

+96
-58
lines changed

‎src/backend/port/win32/socket.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,21 @@ TranslateSocketError(void)
120120
case WSAEADDRNOTAVAIL:
121121
errno = EADDRNOTAVAIL;
122122
break;
123-
case WSAEHOSTUNREACH:
124123
case WSAEHOSTDOWN:
124+
errno = EHOSTDOWN;
125+
break;
126+
case WSAEHOSTUNREACH:
125127
case WSAHOST_NOT_FOUND:
128+
errno = EHOSTUNREACH;
129+
break;
126130
case WSAENETDOWN:
131+
errno = ENETDOWN;
132+
break;
127133
case WSAENETUNREACH:
134+
errno = ENETUNREACH;
135+
break;
128136
case WSAENETRESET:
129-
errno = EHOSTUNREACH;
137+
errno = ENETRESET;
130138
break;
131139
case WSAENOTCONN:
132140
case WSAESHUTDOWN:

‎src/backend/utils/error/elog.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -711,10 +711,7 @@ errcode_for_socket_access(void)
711711
switch (edata->saved_errno)
712712
{
713713
/* Loss of connection */
714-
case EPIPE:
715-
#ifdef ECONNRESET
716-
case ECONNRESET:
717-
#endif
714+
case ALL_CONNECTION_FAILURE_ERRNOS:
718715
edata->sqlerrcode = ERRCODE_CONNECTION_FAILURE;
719716
break;
720717

‎src/bin/pg_dump/parallel.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1825,10 +1825,15 @@ piperead(int s, char *buf, int len)
18251825
{
18261826
int ret = recv(s, buf, len, 0);
18271827

1828-
if (ret < 0 && WSAGetLastError() == WSAECONNRESET)
1828+
if (ret < 0)
18291829
{
1830-
/* EOF on the pipe! */
1831-
ret = 0;
1830+
switch (TranslateSocketError())
1831+
{
1832+
case ALL_CONNECTION_FAILURE_ERRNOS:
1833+
/* Treat connection loss as EOF on the pipe */
1834+
ret = 0;
1835+
break;
1836+
}
18321837
}
18331838
return ret;
18341839
}

‎src/include/port.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,28 @@ extern void pgfnames_cleanup(char **filenames);
9999
)
100100
#endif
101101

102+
/*
103+
* This macro provides a centralized list of all errnos that identify
104+
* hard failure of a previously-established network connection.
105+
* The macro is intended to be used in a switch statement, in the form
106+
* "case ALL_CONNECTION_FAILURE_ERRNOS:".
107+
*
108+
* Note: this groups EPIPE and ECONNRESET, which we take to indicate a
109+
* probable server crash, with other errors that indicate loss of network
110+
* connectivity without proving much about the server's state. Places that
111+
* are actually reporting errors typically single out EPIPE and ECONNRESET,
112+
* while allowing the network failures to be reported generically.
113+
*/
114+
#define ALL_CONNECTION_FAILURE_ERRNOS \
115+
EPIPE: \
116+
case ECONNRESET: \
117+
case ECONNABORTED: \
118+
case EHOSTDOWN: \
119+
case EHOSTUNREACH: \
120+
case ENETDOWN: \
121+
case ENETRESET: \
122+
case ENETUNREACH
123+
102124
/* Portable locale initialization (in exec.c) */
103125
extern void set_pglocale_pgservice(const char *argv0, const char *app);
104126

‎src/include/port/win32_port.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,8 +369,16 @@ extern int _pgstat64(const char *name, struct stat *buf);
369369
#define EADDRINUSE WSAEADDRINUSE
370370
#undef EADDRNOTAVAIL
371371
#define EADDRNOTAVAIL WSAEADDRNOTAVAIL
372+
#undef EHOSTDOWN
373+
#define EHOSTDOWN WSAEHOSTDOWN
372374
#undef EHOSTUNREACH
373375
#define EHOSTUNREACH WSAEHOSTUNREACH
376+
#undef ENETDOWN
377+
#define ENETDOWN WSAENETDOWN
378+
#undef ENETRESET
379+
#define ENETRESET WSAENETRESET
380+
#undef ENETUNREACH
381+
#define ENETUNREACH WSAENETUNREACH
374382
#undef ENOTCONN
375383
#define ENOTCONN WSAENOTCONN
376384

‎src/interfaces/libpq/fe-misc.c

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -668,24 +668,29 @@ pqReadData(PGconn *conn)
668668
conn->inBufSize - conn->inEnd);
669669
if (nread < 0)
670670
{
671-
if (SOCK_ERRNO == EINTR)
672-
goto retry3;
673-
/* Some systems return EAGAIN/EWOULDBLOCK for no data */
671+
switch (SOCK_ERRNO)
672+
{
673+
case EINTR:
674+
goto retry3;
675+
676+
/* Some systems return EAGAIN/EWOULDBLOCK for no data */
674677
#ifdef EAGAIN
675-
if (SOCK_ERRNO == EAGAIN)
676-
return someread;
678+
case EAGAIN:
679+
return someread;
677680
#endif
678681
#if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
679-
if (SOCK_ERRNO == EWOULDBLOCK)
680-
return someread;
682+
case EWOULDBLOCK:
683+
return someread;
681684
#endif
682-
/* We might get ECONNRESET here if using TCP and backend died */
683-
#ifdef ECONNRESET
684-
if (SOCK_ERRNO == ECONNRESET)
685-
goto definitelyFailed;
686-
#endif
687-
/* pqsecure_read set the error message for us */
688-
return -1;
685+
686+
/* We might get ECONNRESET etc here if connection failed */
687+
case ALL_CONNECTION_FAILURE_ERRNOS:
688+
goto definitelyFailed;
689+
690+
default:
691+
/* pqsecure_read set the error message for us */
692+
return -1;
693+
}
689694
}
690695
if (nread > 0)
691696
{
@@ -758,24 +763,29 @@ pqReadData(PGconn *conn)
758763
conn->inBufSize - conn->inEnd);
759764
if (nread < 0)
760765
{
761-
if (SOCK_ERRNO == EINTR)
762-
goto retry4;
763-
/* Some systems return EAGAIN/EWOULDBLOCK for no data */
766+
switch (SOCK_ERRNO)
767+
{
768+
case EINTR:
769+
goto retry4;
770+
771+
/* Some systems return EAGAIN/EWOULDBLOCK for no data */
764772
#ifdef EAGAIN
765-
if (SOCK_ERRNO == EAGAIN)
766-
return 0;
773+
case EAGAIN:
774+
return 0;
767775
#endif
768776
#if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
769-
if (SOCK_ERRNO == EWOULDBLOCK)
770-
return 0;
777+
case EWOULDBLOCK:
778+
return 0;
771779
#endif
772-
/* We might get ECONNRESET here if using TCP and backend died */
773-
#ifdef ECONNRESET
774-
if (SOCK_ERRNO == ECONNRESET)
775-
goto definitelyFailed;
776-
#endif
777-
/* pqsecure_read set the error message for us */
778-
return -1;
780+
781+
/* We might get ECONNRESET etc here if connection failed */
782+
case ALL_CONNECTION_FAILURE_ERRNOS:
783+
goto definitelyFailed;
784+
785+
default:
786+
/* pqsecure_read set the error message for us */
787+
return -1;
788+
}
779789
}
780790
if (nread > 0)
781791
{

‎src/interfaces/libpq/fe-secure.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,14 +261,13 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
261261
/* no error message, caller is expected to retry */
262262
break;
263263

264-
#ifdef ECONNRESET
264+
case EPIPE:
265265
case ECONNRESET:
266266
printfPQExpBuffer(&conn->errorMessage,
267267
libpq_gettext("server closed the connection unexpectedly\n"
268268
"\tThis probably means the server terminated abnormally\n"
269269
"\tbefore or while processing the request.\n"));
270270
break;
271-
#endif
272271

273272
default:
274273
printfPQExpBuffer(&conn->errorMessage,
@@ -374,11 +373,9 @@ pqsecure_raw_write(PGconn *conn, const void *ptr, size_t len)
374373
/* Set flag for EPIPE */
375374
REMEMBER_EPIPE(spinfo, true);
376375

377-
#ifdef ECONNRESET
378376
/* FALL THRU */
379377

380378
case ECONNRESET:
381-
#endif
382379
printfPQExpBuffer(&conn->errorMessage,
383380
libpq_gettext("server closed the connection unexpectedly\n"
384381
"\tThis probably means the server terminated abnormally\n"

‎src/interfaces/libpq/win32.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,6 @@
1414
#define write(a,b,c) _write(a,b,c)
1515

1616
#undef EAGAIN /* doesn't apply on sockets */
17-
#undef EINTR
18-
#define EINTR WSAEINTR
19-
#ifndef EWOULDBLOCK
20-
#define EWOULDBLOCK WSAEWOULDBLOCK
21-
#endif
22-
#ifndef ECONNRESET
23-
#define ECONNRESET WSAECONNRESET
24-
#endif
25-
#ifndef EINPROGRESS
26-
#define EINPROGRESS WSAEINPROGRESS
27-
#endif
2817

2918
/*
3019
* support for handling Windows Socket errors

‎src/port/strerror.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,16 +146,12 @@ get_errno_symbol(int errnum)
146146
return "EBUSY";
147147
case ECHILD:
148148
return "ECHILD";
149-
#ifdef ECONNABORTED
150149
case ECONNABORTED:
151150
return "ECONNABORTED";
152-
#endif
153151
case ECONNREFUSED:
154152
return "ECONNREFUSED";
155-
#ifdef ECONNRESET
156153
case ECONNRESET:
157154
return "ECONNRESET";
158-
#endif
159155
case EDEADLK:
160156
return "EDEADLK";
161157
case EDOM:
@@ -166,10 +162,10 @@ get_errno_symbol(int errnum)
166162
return "EFAULT";
167163
case EFBIG:
168164
return "EFBIG";
169-
#ifdef EHOSTUNREACH
165+
case EHOSTDOWN:
166+
return "EHOSTDOWN";
170167
case EHOSTUNREACH:
171168
return "EHOSTUNREACH";
172-
#endif
173169
case EIDRM:
174170
return "EIDRM";
175171
case EINPROGRESS:
@@ -198,6 +194,12 @@ get_errno_symbol(int errnum)
198194
return "EMSGSIZE";
199195
case ENAMETOOLONG:
200196
return "ENAMETOOLONG";
197+
case ENETDOWN:
198+
return "ENETDOWN";
199+
case ENETRESET:
200+
return "ENETRESET";
201+
case ENETUNREACH:
202+
return "ENETUNREACH";
201203
case ENFILE:
202204
return "ENFILE";
203205
case ENOBUFS:

0 commit comments

Comments
 (0)