Re: libpq copy error handling busted
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org
Date: 2020-06-04T16:29:06Z
Lists: pgsql-hackers
Attachments
- fix-cancelrequested.patch (text/x-diff) patch
I wrote: > * As for control-C not getting out of it: there is > if (CancelRequested) > break; > in pgbench's loop, but this does nothing in this scenario because > fe-utils/cancel.c only sets that flag when it successfully sends a > Cancel ... which it certainly cannot if the postmaster is gone. > I suspect that this may be relatively recent breakage. It doesn't look > too well thought out, in any case. The places that are testing this > flag look like they'd rather not be bothered with the fine point of > whether the cancel request actually went anywhere. On closer inspection, it seems that scripts_parallel.c does have a dependency on the cancel request having been sent, because it insists on collecting a query result off the active connection after detecting CancelRequested. This seems dangerously overoptimistic to me; it will lock up if for any reason the server doesn't honor the cancel request. It's also pointless, because all the calling apps are just going to close their connections and exit(1) afterwards, so there's no use in trying to resynchronize the connection state. (Plus, disconnectDatabase will issue cancels on any busy connections; which would be necessary anyway in a parallel operation, since cancel.c could only have signaled one of them.) So the attached patch just removes the useless consumeQueryResult call, and then simplifies select_loop's API a bit. With that change, I don't see any place that wants the existing definition of CancelRequested rather than the simpler meaning of "SIGINT was received", so I just changed it to mean that. We could certainly also have a variable tracking whether a cancel request was sent, but I see no point in one right now. It's easiest to test this *without* the other patch -- just run the pgbench scenario Andres demonstrated, and see whether control-C gets pgbench to quit cleanly. regards, tom lane
Commits
-
Try to read data from the socket in pqSendSome's write_failed paths.
- 7247e243a803 13.0 landed
- 2edf14f5ac34 12.4 landed
-
Rethink definition of cancel.c's CancelRequested flag.
- 92f33bb7afd3 13.0 landed
-
Restructure libpq's handling of send failures.
- 1f39a1c06415 12.0 cited