Re: Expansion of our checks for connection-loss errors
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2020-10-09T16:14:17Z
Lists: pgsql-hackers
Attachments
- recognize-more-connection-loss-errnos-2.patch (text/x-diff) patch
I wrote: > Hmm, excellent point. While our code response to all these errors > should be the same, you are right that that doesn't extend to emitting > identical error texts. For EHOSTUNREACH/ENETDOWN/ENETUNREACH, we > should say something like "connection to server lost", without claiming > that the server crashed. It is less clear what to do with ECONNABORTED, > but I'm inclined to put it in the network-problem bucket not the > server-crash bucket, despite lorikeet's behavior. Thoughts? > This also destroys the patch's idea that switch statements should be > able to handle these all alike. If we group things as "ECONNRESET means > server crash and the others are all network failures", then I'd be > inclined to leave the ECONNRESET cases alone and just introduce > new infrastructure to recognize all the network-failure errnos. Actually, treating it that way seems like a good thing because it nets out as (nearly) no change to our error message behavior. The connection failure errnos fall through to the default case, which produces a perfectly reasonable report that includes strerror(). The only big thing we're changing is the set of errnos that errcode_for_socket_access will map to ERRCODE_CONNECTION_FAILURE, so this is spiritually closer to your original patch. Some other changes in the attached v2: * I incorporated Kyotaro-san's suggested improvements. * I went ahead and included ENETRESET and EHOSTDOWN, figuring that if they exist we definitely want to class them as network failures. We can worry about ifdef'ing them when and if we find a platform that hasn't got them. (I don't see any non-ugly way to make the ALL_NETWORK_FAILURE_ERRNOS macro vary for missing symbols, so I'd rather not deal with that unless it's proven necessary.) * I noticed that we were not terribly consistent about whether EPIPE is regarded as indicating a server failure like ECONNRESET does. So this patch also makes sure that EPIPE is treated like ECONNRESET everywhere. (Hence, pqsecure_raw_read's error reporting does change, since it'll now report EPIPE as server failure.) I lack a way to test this on Windows, but otherwise it feels like it's about ready. regards, tom lane
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