On Wed, 6 Mar 2019 at 17:38, Chris Travers <chris.trav...@adjust.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 https://www.postgresql.org/message-id/CAMsr%2BYEuz4XwZX_QmnX_-2530XhyAmnK%3DzCmicEnq1vLr0aZ-g%40mail.gmail.com . -- Craig Ringer http://www.2ndQuadrant.com/ 2ndQuadrant - PostgreSQL Solutions for the Enterprise