Re: Hot Standy introduced problem with query cancel behavior

On Thu, Dec 31, 2009 at 6:40 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> Building racy infrastructure when it can be avoided with a little care
>> still seems not to be the best path to me.
>
> Doing that will add more complexity in an area that is hard to test
> effectively. I think the risk of introducing further bugs while trying
> to fix this rare condition is high. Right now the conflict processing
> needs more work and is often much less precise than this, so improving
> this aspect of it would not be a priority. I've added it to the TODO
> though. Thank you for your research.
>
> Patch implements recovery conflict signalling using SIGUSR1
> multiplexing, then uses a SessionCancelPending mode similar to Joachim's
> TransactionCancelPending.

I have reworked Simon's patch a bit and attach the result.

Quick facts:

- Hot Standby only uses SIGUSR1
- SIGINT behaves as it did before: it only cancels running statements
- pg_cancel_backend() continues to use SIGINT
- I added pg_cancel_idle_transaction() to cancel an idle transaction via
  SIGUSR1
- One central function HandleCancelAction() sets the flags before calling
  ProcessInterrupts(), it is called from the different signal handlers and
  receives parameters about what it should do
- If a SIGUSR1 reason is used that will cancel something, ProcArrayLock is
  acquired until the signal has been sent to make sure that we won't signal the
  wrong backend. Does this sufficiently cover the concerns of Andres Freund
  discussed upthread?

As there were so many boolean SomethingCancelPending variables I changed them
to be bitmasks and merged all of them into a single variable. However I am not
sure if we can change the name and type of especially InterruptPending that
easily as it was marked with PGDLLIMPORT...

@Simon: Is there a reason why you have not yet removed recoveryConflictMode
from PGPROC?


Joachim