Re: Proposal for Signal Detection Refactoring

On Wed, 6 Mar 2019 at 17:38, Chris Travers <chris(dot)travers(at)adjust(dot)com> wrote:

>
> Here's a new patch. No rush on it. I am moving it to next commitfest
> anyway because as code documentation I think this is a low priority late in
> the release cycle.
>

While you're looking at signal detection changes I suggest making sure you
get them right for the contribs that use explicit signal handling,
like src/test/modules/worker_spi/ .

I'm actually pretty sure worker_spi is incorrect as it stands in the
current codebase. It defines its own worker_spi_sighup
and worker_spi_sigterm handlers. worker_spi_sigterm() sets a static
bool got_sighup that's scoped to worker_spi.c . Importantly it does NOT set
ipc.c's InterruptPending or ProcDiePending so the CHECK_FOR_INTERRUPTS()
macro will not notice when a backend running worker_spi gets a SIGTERM. So
long as worker_spi's own main loop is serviced regularly that's fine, but
it means worker_spi won't react to signals at all if it's busy in a query
or somewhere else in postgres code.

It's also worth paying attention to the walsender, which has its own signal
handling, and pretty much anywhere else that has a pqsignal(...) call that
isn't SIG_IGN, die, quickdie, startup_die or procsignal_sigusr1_handler:

git grep -P 'pqsignal\(SIG(INT|HUP|TERM|QUIT|USR1|USR2),
(?!die|startup_die|quickdie|procsignal_sigusr1_handler|SIG_IGN)'
src/backend/

I actually found a walsender signal handling issue recently, per
/message-id/CAMsr%2BYEuz4XwZX_QmnX_-2530XhyAmnK%3DzCmicEnq1vLr0aZ-g%40mail.gmail.com
.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise