On 2014-07-26 11:32:24 -0400, Tom Lane wrote:
> "MauMau" <maumau307@gmail.com> writes:
> > [ sinval catchup signal -> ProcessCatchupEvent -> WaitLatch -> deadlock ]
>
> Ugh.
>
> One line of thought is that it's pretty unsafe to be doing anything
> as complicated as transaction start/commit in a signal handler, even one
> that is sure it's not interrupting anything else.
Yea, that's really not nice.
> The only excuse
> ProcessCatchupEvent has for that is that it's "trying to ensure proper
> cleanup from an error". Perhaps with closer analysis we could convince
> ourselves that errors thrown from AcceptInvalidationMessages() wouldn't
> need transaction abort cleanup.
Hm. I'm not convinced that's going to be easy.
Wouldn't it be better to move the catchup interrupt processing out of
the signal handler? For normal backends we only enable when reading from
the client and DoingCommandRead is set. How about setting a variable in
the signal handler and doing the actual catchup processing after the
recv() returned EINTR? That'd require either renegging on SA_RESTART or
using WaitLatchOrSocket() and nonblocking send/recv. I think that'd be
a much safer model and after researching it a bit for the idle in
transaction timeout thing it doesn't look super hard. Even windows seems
to already support everything necessary.
> Or we could try to get rid of the need to do anything beyond setting
> a flag in the interrupt handler itself. But I'm afraid that's probably
> unworkable, especially now that we use SA_RESTART on all signals.
Yea :(. Several platforms actually ignore SA_RESTART for
send/recv/select/... under some circumstances (notably linux), but it'd
probably be hard to get it right for all.
I don't think we can continue to use the blocking calls for much longer
unless we allow them to be interruptible. But I doubt that change would
be backpatchable.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services