• Tom Lane's avatar
    Rethink definition of cancel.c's CancelRequested flag. · 92f33bb7
    Tom Lane authored
    As it stands, this flag is only set when we've successfully sent a
    cancel request, not if we get SIGINT and then fail to send a cancel.
    However, for almost all callers, that's the Wrong Thing: we'd prefer
    to abort processing after control-C even if no cancel could be sent.
    
    As an example, since commit 1d468b9a "pgbench -i" fails to give up
    sending COPY data even after control-C, if the postmaster has been
    stopped, which is clearly not what the code intends and not what anyone
    would want.  (The fact that it keeps going at all is the fault of a
    separate bug in libpq, but not letting CancelRequested become set is
    clearly not what we want here.)
    
    The sole exception, as far as I can find, is that scripts_parallel.c's
    ParallelSlotsGetIdle tries to consume a query result after issuing a
    cancel, which of course might not terminate quickly if no cancel
    happened.  But that behavior was poorly thought out too.  No user of
    ParallelSlotsGetIdle tries to continue processing after a cancel,
    so there is really no point in trying to clear the connection's state.
    Moreover this has the same defect as for other users of cancel.c,
    that if the cancel request fails for some reason then we end up with
    control-C being completely ignored.  (On top of that, select_loop failed
    to distinguish clearly between SIGINT and other reasons for select(2)
    failing, which means that it's possible that the existing code would
    think that a cancel has been sent when it hasn't.)
    
    Hence, redefine CancelRequested as simply meaning that SIGINT was
    received.  We could add a second flag with the other meaning, but
    in the absence of any compelling argument why such a flag is needed,
    I think it would just offer an opportunity for future callers to
    get it wrong.  Also remove the consumeQueryResult call in
    ParallelSlotsGetIdle's failure exit.  In passing, simplify the
    API of select_loop.
    
    It would now be possible to re-unify psql's cancel_pressed with
    CancelRequested, partly undoing 5d43c3c5.  But I'm not really
    convinced that that's worth the trouble, so I left psql alone,
    other than fixing a misleading comment.
    
    This code is new in v13 (cf a4fd3aa7), so no need for back-patch.
    
    Per investigation of a complaint from Andres Freund.
    
    Discussion: https://postgr.es/m/20200603201242.ofvm4jztpqytwfye@alap3.anarazel.de
    92f33bb7
scripts_parallel.c 6.18 KB