On Wed, Jan 11, 2023 at 05:20:38PM +1300, Thomas Munro wrote:
> Is the problem here that SIGCHLD is processed ...
>
> PG_SETMASK(&UnBlockSig); <--- here?
>
> selres = select(nSockets, &rmask, NULL, NULL, &timeout);
>
> Meanwhile the SIGCHLD handler code says:
>
> * Was it the wal receiver? If exit status is zero (normal) or one
> * (FATAL exit), we assume everything is all right just like normal
> * backends. (If we need a new wal receiver, we'll start one at the
> * next iteration of the postmaster's main loop.)
>
> ... which is true, but that won't be reached for a while in this case
> if the timeout has already been set to 60s. Your patch makes that
> 100ms, in that case, a time delay that by now attracts my attention
> like a red rag to a bull (I don't know why you didn't make it 0).
I think this is right. At the very least, it seems consistent with my
observations.
> I'm not sure, but if I got that right, then I think the whole problem
> might automatically go away with CF #4032. The SIGCHLD processing
> code will run not when signals are unblocked before select() (that is
> gone), but instead *after* the event loop wakes up with WL_LATCH_SET,
> and runs the handler code in the regular user context before dropping
> through to the rest of the main loop.
Yeah, with those patches, the problem goes away. IIUC the key part is that
the postmaster's latch gets set when SIGCHLD is received, so even if
SIGUSR1 and SIGCHLD are processed out of order, WalReceiverPID gets cleared
and we try to restart it shortly thereafter. I find this much easier to
reason about.
I'll go ahead and withdraw this patch from the commitfest. Thanks for
chiming in.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
