Expansion of our checks for connection-loss errors
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>
Date: 2020-10-08T19:15:54Z
Lists: pgsql-hackers
Attachments
- recognize-more-connection-loss-errnos-1.patch (text/x-diff) patch
Over in the thread at [1], we've tentatively determined that the reason buildfarm member lorikeet is currently failing is that its network stack returns ECONNABORTED for (some?) connection failures, whereas our code is only expecting ECONNRESET. Fujii Masao therefore proposes that we treat ECONNABORTED the same as ECONNRESET. I think this is a good idea, but after a bit of research I feel it does not go far enough. I find these POSIX-standard errnos that also seem likely candidates to be returned for a hard loss of connection: ECONNABORTED EHOSTUNREACH ENETDOWN ENETUNREACH All of these have been in POSIX since SUSv2, so it seems unlikely that we need to #ifdef any of them. (It is in any case pretty silly that we have #ifdefs around a very small minority of our references to ECONNRESET :-(.) There are some other related errnos, such as ECONNREFUSED, that don't seem like they'd be returned for a failure of a pre-existing connection, so we don't need to include them in such tests. Accordingly, I propose the attached patch (an expansion of Fujii-san's) that causes us to test for all five errnos anyplace we had been checking for ECONNRESET. I felt that this was getting to the point where we'd better centralize the knowledge of what to check, so the patch does that, via an inline function and an admittedly hacky macro. I also upgraded some places such as strerror.c to have full support for these symbols. All of the machines I have (even as far back as HPUX 10.20) also define ENETRESET and EHOSTDOWN. However, those symbols do not appear in SUSv2. ENETRESET was added at some later point, but EHOSTDOWN is still not in POSIX. For the moment I've left these second-tier symbols out of the patch, but there's a case for adding them. I'm not sure whether there'd be any point in trying to #ifdef them. BTW, I took out the conditional defines of some of these errnos in libpq's win32.h; AFAICS that's been dead code ever since we added #define's for them to win32_port.h. Am I missing something? This seems like a bug fix to me, so I'm inclined to back-patch. regards, tom lane [1] https://www.postgresql.org/message-id/flat/E1kPc9v-0005L4-2l%40gemulon.postgresql.org
Commits
-
Band-aid new postgres_fdw test case to remove error text dependency.
- 85d08b8b721f 14.0 landed
-
Remove pointless error-code checking in pg_dump/parallel.c.
- eeb01eb1f560 14.0 landed
-
Recognize network-failure errnos as indicating hard connection loss.
- fe27009cbb5f 14.0 landed