Re: "stuck spinlock" - Mailing list pgsql-hackers

From Andres Freund
Subject Re: "stuck spinlock"
Date
Msg-id 20131214124210.GD25520@awork2.anarazel.de
Whole thread Raw
In response to Re: "stuck spinlock"  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On 2013-12-13 13:39:42 -0500, Robert Haas wrote:
> On Fri, Dec 13, 2013 at 1:15 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> > Agreed on not going forward like now, but I don't really see how they
> > could usefully use die(). I think we should just mandate that every
> > bgworker conneced to shared memory registers a sigterm handler - we
> > could put a check into BackgroundWorkerUnblockSignals(). We should leave
> > the current handler in for unconnected one though...
> > bgworkers are supposed to be written as a loop around procLatch, so
> > adding a !got_sigterm, probably isn't too hard.
> 
> I think the !got_sigterm thing is complete bunk.  If a background
> worker is running SQL queries, it really ought to honor a query cancel
> or sigterm at the next CHECK_FOR_INTERRUPTS().

I am not convinced by the necessity of that, not in general. After all,
the code is using a bgworker and not a normal backend for a reason. If
you e.g. have queing code, it very well might need to serialize its
state to disk before shutting down. Checking whether the bgworker should
shut down every iteration of the mainloop sounds appropriate to me for
such cases.

But I think we should provide a default handler that does the necessary
things to interrupt queries, so bgworker authors don't have to do it
themselves and, just as importantly, we can more easily add new stuff
there.

> +static void
> +handle_sigterm(SIGNAL_ARGS)
> +{
> +       int                     save_errno = errno;
> +
> +       if (MyProc)
> +               SetLatch(&MyProc->procLatch);
> +
> +       if (!proc_exit_inprogress)
> +       {
> +               InterruptPending = true;
> +               ProcDiePending = true;
> +       }
> +
> +       errno = save_errno;
> +}
> 
> ...but I'm not 100% sure that's right, either.

If you want a bgworker to behave as close as a normal backend we should
probably really do the full dance die() does. Specifically call
ProcessInterrupts() immediately if ImmediateInterruptOK allows it,
otherwise we'd just continue waiting for locks and similar.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: "stuck spinlock"
Next
From: Andres Freund
Date:
Subject: Re: PoC: Partial sort