Thread: [HACKERS] shm_mq_wait_internal gets stuck forever on fast shutdown
Hi all
I've noticed a possible bug / design limitation where shm_mq_wait_internal sleep in a latch wait forever, and the postmaster gets stuck waiting for the bgworker the wait is running in to exit.
I've noticed a possible bug / design limitation where shm_mq_wait_internal sleep in a latch wait forever, and the postmaster gets stuck waiting for the bgworker the wait is running in to exit.
This happens when the shm_mq does not have an associated bgworker handle registered because the other end is not known at mq creation time or is a normal backend not a bgworker. So a BGW handle cannot be passed.
* In a proc's signal handler, use globals set before entry and after exit from shm_mq operations to detect if we're currently in shm_mq and promote SIGTERM to SIGQUIT by sending a new signal to ourselves. Or set up state so CHECK_FOR_INTERRUPTS() will notice when the handler returns.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
shm_mq_wait_internal() will CHECK_FOR_INTERRUPTS() when its latch wait is interrupted by a SIGTERM. But it doesn't actually respond to SIGTERM in any way; it just merrily resets its latch and keeps looping.
It will bail out correctly on SIGQUIT.
If the proc waiting to attach was known at queue creation time and was a bgworker, we'd pass a bgworker handle and the mq would notice it failed to start and stop waiting. There's only a problem if no bgworker handle can be supplied.
The underlying problem is that CHECK_FOR_INTERRUPTS() doesn't care about SIGTERM or have any way to test for it. And we don't have any global management of SIGTERM like we do SIGQUIT so the shm_mq_wait_internal loop can't test for it.
The only ways I can see to fix this are:
* Generalize SIGTERM handling across postgres, so there's a global "got_SIGTERM" global that shm_mq_wait_internal can test to break out of its loop, and every backend's signal handler must set it. Lots of churn.
* In a proc's signal handler, use globals set before entry and after exit from shm_mq operations to detect if we're currently in shm_mq and promote SIGTERM to SIGQUIT by sending a new signal to ourselves. Or set up state so CHECK_FOR_INTERRUPTS() will notice when the handler returns.
* Allow passing of a *bool that tests for SIGTERM, or a function pointer called on each iteration to test whether looping should continue, to be passed to shm_mq_attach. So if you can't supply a bgw handle, you supply that instead. Provide a shm_mq_set_handle equivalent for it too.
Any objections to the last approach?
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On 21 August 2017 at 10:57, Craig Ringer <craig@2ndquadrant.com> wrote:
Hi all
I've noticed a possible bug / design limitation where shm_mq_wait_internal sleep in a latch wait forever, and the postmaster gets stuck waiting for the bgworker the wait is running in to exit.This happens when the shm_mq does not have an associated bgworker handle registered because the other end is not known at mq creation time or is a normal backend not a bgworker. So a BGW handle cannot be passed.shm_mq_wait_internal() will CHECK_FOR_INTERRUPTS() when its latch wait is interrupted by a SIGTERM. But it doesn't actually respond to SIGTERM in any way; it just merrily resets its latch and keeps looping.It will bail out correctly on SIGQUIT.If the proc waiting to attach was known at queue creation time and was a bgworker, we'd pass a bgworker handle and the mq would notice it failed to start and stop waiting. There's only a problem if no bgworker handle can be supplied.The underlying problem is that CHECK_FOR_INTERRUPTS() doesn't care about SIGTERM or have any way to test for it. And we don't have any global management of SIGTERM like we do SIGQUIT so the shm_mq_wait_internal loop can't test for it.The only ways I can see to fix this are:* Generalize SIGTERM handling across postgres, so there's a global "got_SIGTERM" global that shm_mq_wait_internal can test to break out of its loop, and every backend's signal handler must set it. Lots of churn.
* In a proc's signal handler, use globals set before entry and after exit from shm_mq operations to detect if we're currently in shm_mq and promote SIGTERM to SIGQUIT by sending a new signal to ourselves. Or set up state so CHECK_FOR_INTERRUPTS() will notice when the handler returns.* Allow passing of a *bool that tests for SIGTERM, or a function pointer called on each iteration to test whether looping should continue, to be passed to shm_mq_attach. So if you can't supply a bgw handle, you supply that instead. Provide a shm_mq_set_handle equivalent for it too.Any objections to the last approach?
BTW, you can work around it in extension code for existing versions with something like this in your bgworker:
volatile bool in_shm_mq = false;
void
my_handle_sigterm(SIGNAL_ARGS)
{
...
if (in_shm_mq)
{
/*
* shm_mq can get stuck in shm_mq_wait_internal on SIGTERM; see
*
* To work around this we keep track of whether we're in shmem_mq
* and generate a fake interrupt for CHECK_FOR_INTERRUPTS() to
* process if so.
*
* The guard around in_shm_mq may not be necessary, but without
* it any SIGTERM will likely cause ereport(FATAL) with
* "terminating connection due to administrator command"
* which isn't ideal.
*/
InterruptPending = true;
ProcDiePending = true;
}
....
}
inline static shm_mq_handle *
myext_shm_mq_attach(shm_mq *mq, dsm_segment *seg, BackgroundWorkerHandle *handle)
{
shm_mq_handle *ret;
in_shm_mq = true;
ret = shm_mq_attach(mq, seg, handle);
in_shm_mq = false;
return ret;
}
/* repeated for shm_mq_receive, shm_mq_send, shm_mq_sendv, shm_mq_wait_for_attach */
You can instead use non-blocking sends instead, and sleep on your own latch, doing the same work as shm_mq_wait_internal yourself.
On Sun, Aug 20, 2017 at 10:57 PM, Craig Ringer <craig@2ndquadrant.com> wrote: > I've noticed a possible bug / design limitation where shm_mq_wait_internal > sleep in a latch wait forever, and the postmaster gets stuck waiting for the > bgworker the wait is running in to exit. > > This happens when the shm_mq does not have an associated bgworker handle > registered because the other end is not known at mq creation time or is a > normal backend not a bgworker. So a BGW handle cannot be passed. Well, right. The reason the API lets you pass a bgworker handle is to avoid the problem that happens if you don't pass a bgworker handle. :-) > shm_mq_wait_internal() will CHECK_FOR_INTERRUPTS() when its latch wait is > interrupted by a SIGTERM. But it doesn't actually respond to SIGTERM in any > way; it just merrily resets its latch and keeps looping. > > It will bail out correctly on SIGQUIT. There are a couple of ways to avoid this in the existing code structure. One is to not use blocking operations. If you pass nowait as true then you can put the latch wait loop in the caller and inject whatever logic you want. Another approach is to use die() or something else that sets ProcDiePending as your SIGTERM handler. > The only ways I can see to fix this are: > > * Generalize SIGTERM handling across postgres, so there's a global > "got_SIGTERM" global that shm_mq_wait_internal can test to break out of its > loop, and every backend's signal handler must set it. Lots of churn. Not a bad idea otherwise, though. > * In a proc's signal handler, use globals set before entry and after exit > from shm_mq operations to detect if we're currently in shm_mq and promote > SIGTERM to SIGQUIT by sending a new signal to ourselves. Or set up state so > CHECK_FOR_INTERRUPTS() will notice when the handler returns. Sounds ugly. > * Allow passing of a *bool that tests for SIGTERM, or a function pointer > called on each iteration to test whether looping should continue, to be > passed to shm_mq_attach. So if you can't supply a bgw handle, you supply > that instead. Provide a shm_mq_set_handle equivalent for it too. While this would work, I don't really see the need for it given the availability of nonblocking operations. See mq_putmessage() for an example. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 21 August 2017 at 21:44, Robert Haas <robertmhaas@gmail.com> wrote:
-- While this would work, I don't really see the need for it given the
availability of nonblocking operations. See mq_putmessage() for an
example.
Makes sense, and I'm not especially concerned. If the expected solution to such usage is to use non-blocking calls, that's fine with me.
I partly wanted to put this out there to help the next person looking into it. Or myself, when I've forgotten and go looking again ;) . But also, to ensure that this was in fact fully expected behaviour not an oversight re applying shm_mq to non-bgworker endpoints.
On Mon, Aug 21, 2017 at 10:07 AM, Craig Ringer <craig@2ndquadrant.com> wrote: > Makes sense, and I'm not especially concerned. If the expected solution to > such usage is to use non-blocking calls, that's fine with me. > > I partly wanted to put this out there to help the next person looking into > it. Or myself, when I've forgotten and go looking again ;) . But also, to > ensure that this was in fact fully expected behaviour not an oversight re > applying shm_mq to non-bgworker endpoints. Yep, it's expected. It's possible I should have designed it differently, so if someone does feel concerned at some point we can certainly debate how to change things, but what you're describing matches my expectations and it seems OK to me, pretty much. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company