Skip site navigation (1) Skip section navigation (2)

Re: Hot Standy introduced problem with query cancel behavior

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org, Joachim Wieland <joe(at)mcknight(dot)de>, Kris Jurka <books(at)ejurka(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Subject: Re: Hot Standy introduced problem with query cancel behavior
Date: 2009-12-30 19:25:02
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackers
On Wed, Dec 30, 2009 at 2:06 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On Wednesday 30 December 2009 01:13:01 Simon Riggs wrote:
>> On Tue, 2009-12-29 at 11:13 -0500, Tom Lane wrote:
>> > Andres Freund <andres(at)anarazel(dot)de> writes:
>> > > On Tuesday 29 December 2009 16:22:54 Tom Lane wrote:
>> > >> This seems like a fairly bad idea.  One of the intended use-cases is
>> > >> to be able to manually "kill -INT" a misbehaving backend.  Assuming
>> > >> that there will be valid info about the signal in shared memory will
>> > >> break that.
>> > >
>> > > Well. That already is the case now. MyProc->recoveryConflictMode is
>> > > checked to recognize what kind of conflict is being resolved...
>> >
>> > In that case, HS has already broken it, and we need to fix it not make
>> > it worse.
>> >
>> > My humble opinion is that SIGINT should not be overloaded with multiple
>> > meanings.  We already have a multiplexed signal mechanism, which is what
>> > should be used for any additional signal reasons HS may need to
>> > introduce.
>> It's a revelation to me, but yes, I see it now and agree.
>> I'm looking at Fujii-san's multiplexing patch from Jul 31 to rewrite
>> this code using that mechanism. It sounds like it's a neat fit and it
>> should get around the bug report from Kris also if it all works.
> 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 ;-)
> I guess locking procarray during sending the signal should be enough?

I think the idea is that you define the behavior of the signal to be
"look at this other piece of state to see whether you should cancel
yourself" rather than just "cancel yourself".  Then if a signal is
delivered by mistake, it's no big deal - you just look at the other
piece of state and decide that you don't need to do anything.


In response to


pgsql-hackers by date

Next:From: Andrew DunstanDate: 2009-12-30 19:26:32
Subject: Re: PATCH: Add hstore_to_json()
Previous:From: Robert HaasDate: 2009-12-30 19:23:03
Subject: Re: PATCH: Add hstore_to_json()

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group