Re: Hot Standy introduced problem with query cancel behavior

On Thursday 31 December 2009 12:25:19 Simon Riggs wrote:
> On Wed, 2009-12-30 at 20:06 +0100, Andres Freund wrote:
> > Hm. I just read a bit of that multiplexing facility (out of a different
> > reason) and I have some doubt about it being used unmodified for
> > canceling backends:
> >
> > procsignal.c:
> > /*
> > * Note: Since there's no locking, it's possible that the target
> > * process detaches from shared memory and exits right after this
> > * test, before we set the flag and send signal. And the signal slot
> > * might even be recycled by a new process, so it's remotely possible
> > * that we set a flag for a wrong process. That's OK, all the signals
> > * are such that no harm is done if they're mistakenly fired.
> > */
> > procsignal.h:
> > ...
> > * Also, because of race conditions, it's important that all the signals
> > be * defined so that no harm is done if a process mistakenly receives
> > one. */
> >
> > When cancelling a backend that behaviour could be a bit annoying ;-)
>
> Reading comments alone doesn't show the full situation here.
>
> Before we send signal we check pid also, so the chances of this
> happening are fairly small. i.e. we would need to have a backend slot
> reused by a new backend with exactly same pid as the last slot holder.
Well. The problem does not occur for critical errors (i.e. session death)
only. As signal delivery is asynchronous it can very well happen for
transaction cancellation as well.

> I'm proposing to use this for killing transactions and connections, so I
> don't think there's too much harm done there. If the backend is leaving
> anyway, that's what we wanted. If its a new guy that is wearing the same
> boots then a little collateral damage doesn't leave the server in a bad
> place. HS cancellations aren't yet so precise that this matters.
Building racy infrastructure when it can be avoided with a little care still
seems not to be the best path to me.

Andres