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: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org, masao.fujii@oss.nttdata.com
Date: 2020-10-09T01:41:55Z
Lists: pgsql-hackers
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes: > At Thu, 08 Oct 2020 15:15:54 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in >> 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. > +1 for the direction. > In terms of connection errors, connect(2) and bind(2) can return > EADDRNOTAVAIL. bind(2) and listen(2) can return EADDRINUSE. FWIW I > recetnly saw pgbench getting EADDRNOTAVAIL. (They have mapping from > respective WSA errors in TranslateSocketError()) I do not think we have any issues with connection-time errors; or at least, if we do, the spots being touched here certainly shouldn't need to worry about them. These places are dealing with already-established connections. > I'd make errno_is_connection_loss use ALL_CONNECTION_LOSS_ERRNOS to > avoid duplication definition of the errno list. Hmm, might be worth doing, but I'm not sure. I am worried about whether compilers will generate equally good code that way. > - if (ret < 0 && WSAGetLastError() == WSAECONNRESET) > + if (ret < 0 && errno_is_connection_loss(WSAGetLastError())) > Don't we need to use TranslateSocketError() before? Oh, I missed that. But: > Perhaps I'm confused but SOCK_ERROR doesn't seem portable between > Windows and Linux. In that case, nothing would have worked on Windows for the last ten years, so you're mistaken. I think the actual explanation why this works, and why that test in parallel.c probably still works even with my mistake, is that win32_port.h makes sure that our values of ECONNRESET etc match WSAECONNRESET etc. IOW, we'd not actually need TranslateSocketError at all, except that it maps some not-similarly-named error codes for conditions that don't exist in Unix into ones that do. We probably do want TranslateSocketError in this parallel.c test so that anything that it maps to one of the errno_is_connection_loss codes will be recognized; but the basic cases would work anyway, unless I misunderstand this stuff entirely. 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