Thread: SIGHUP not received by custom bgworkers if postmaster is notified

Hi all,

While playing with custom background workers, I noticed that postmaster does not notify its registered bgworkers if it receives SIGHUP,
so you have to send a SIGHUP directly to the bgworker process to notify it. Signal handling is correctly done for SIGQUIT and SIGTERM for shutdown only.
Attached is a patch fixing that, I simply added a call to SignalUnconnectedWorkers in SIGHUP_handler:postmaster.c.

Regards,
--
Michael

On 21-03-2013 05:06, Michael Paquier wrote:
> While playing with custom background workers, I noticed that postmaster does
> not notify its registered bgworkers if it receives SIGHUP,
> so you have to send a SIGHUP directly to the bgworker process to notify it.
> Signal handling is correctly done for SIGQUIT and SIGTERM for shutdown only.
> Attached is a patch fixing that, I simply added a call to
> SignalUnconnectedWorkers in SIGHUP_handler:postmaster.c.
> 
Per this discussion [1], it seems it is as is by design. AFAICS controlling
when change configuration parameters is a feature not a bug. Alvaro said that
will include SIGHUP handle in worker_spi (see [2] for how to process
configurantion file).


[1] http://www.postgresql.org/message-id/20121231140353.GC4363@alvh.no-ip.org
[2]
http://www.postgresql.org/message-id/1357210591.1964.22.camel@localhost.localdomain


--   Euler Taveira de Oliveira - Timbira       http://www.timbira.com.br/  PostgreSQL: Consultoria, Desenvolvimento,
Suporte24x7 e Treinamento
 

Euler Taveira escribió:
> On 21-03-2013 05:06, Michael Paquier wrote:
> > While playing with custom background workers, I noticed that postmaster does
> > not notify its registered bgworkers if it receives SIGHUP,
> > so you have to send a SIGHUP directly to the bgworker process to notify it.
> > Signal handling is correctly done for SIGQUIT and SIGTERM for shutdown only.
> > Attached is a patch fixing that, I simply added a call to
> > SignalUnconnectedWorkers in SIGHUP_handler:postmaster.c.
> >
> Per this discussion [1], it seems it is as is by design. AFAICS controlling
> when change configuration parameters is a feature not a bug. Alvaro said that
> will include SIGHUP handle in worker_spi (see [2] for how to process
> configurantion file).

They are opposite ends of the problem.  Worker code needs a SIGHUP
signal handler, whatever that is (most likely something that causes the
configuration to be reread), which is what Guillaume's patch is about;
but postmaster needs to *send* a SIGHUP to its bgworker children, which
is what Michael is on about.  Currently postmaster signals children that
are connected to shmem, but it's not considering those that aren't
connected.

At least that's how I understand the issue at hand, without actually
looking deeper into it.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

-- 
Michael

Hi all,

Please find attached a simple example of bgworker that logs a message each time a SIGTERM or SIGHUP signal is received by it:
- "hello signal: processed SIGHUP" when SIGHUP is handled by my example
- "hello signal: processed SIGTERM" when SIGTERM is handled by my example

With the current master code, here is what I get:
$ for i in {1..5}; do pg_ctl reload -D ~/bin/pgsql/master/; sleep 1; done
server signaled
server signaled
server signaled
server signaled
server signaled
$ cat ~/bin/pgsql/master/pg_log/postgresql-2013-03-23_112246.log
LOG:  starting background worker process "hello signal worker"
LOG:  database system was shut down at 2013-03-23 11:22:46 JST
LOG:  database system is ready to accept connections
LOG:  autovacuum launcher started
   
LOG:  received SIGHUP, reloading configuration files
LOG:  received SIGHUP, reloading configuration files
LOG:  received SIGHUP, reloading configuration files
LOG:  received SIGHUP, reloading configuration files
LOG:  received SIGHUP, reloading configuration files

SIGHUP is not received by my bgworker. But SIGTERM is:

$ tail -n 5 ~/bin/pgsql/master/pg_log/postgresql-2013-03-23_112246.log
LOG:  autovacuum launcher shutting down
LOG:  hello signal: processed SIGTERM
LOG:  worker process: hello signal worker (PID 2873) exited with exit code 0
LOG:  shutting down
LOG:  database system is shut down

Now, if I apply my fix and redo the same tests, here is what I get:

$ for i in {1..5}; do pg_ctl reload -D ~/bin/pgsql/master/; sleep 1; done
server signaled
server signaled
server signaled
server signaled
server signaled
$ cat ~/bin/pgsql/master/pg_log/postgresql-2013-03-23_113315.log
LOG:  starting background worker process "hello signal worker"
LOG:  database system was shut down at 2013-03-23 11:33:14 JST
LOG:  database system is ready to accept connections
LOG:  autovacuum launcher started
   
LOG:  received SIGHUP, reloading configuration files
LOG:  hello signal: processed SIGHUP
LOG:  received SIGHUP, reloading configuration files
LOG:  hello signal: processed SIGHUP
LOG:  received SIGHUP, reloading configuration files
LOG:  hello signal: processed SIGHUP
LOG:  received SIGHUP, reloading configuration files
LOG:  hello signal: processed SIGHUP
LOG:  received SIGHUP, reloading configuration files
LOG:  hello signal: processed SIGHUP

So SIGHUP is now correctly managed by the bgworker. As well as SIGTERM:

$ pg_ctl st: pg_ctl stop -D ~/bin/pgsql/master/
waiting for server to shut down.... done
server stopped
ioltas@nukkle:~/bin/extra(linux OK)$ tail -n 5 ~/bin/pgsql/master/pg_log/postgresql-2013-03-23_113315.log
LOG:  hello signal: processed SIGTERM
LOG:  autovacuum launcher shutting down
LOG:  worker process: hello signal worker (PID 13781) exited with exit code 0
LOG:  shutting down
LOG:  database system is shut down

It would be great to get that fixed.
Thanks.
--
Michael

Michael Paquier escribió:
> Hi all,
>
> While playing with custom background workers, I noticed that postmaster
> does not notify its registered bgworkers if it receives SIGHUP,
> so you have to send a SIGHUP directly to the bgworker process to notify it.
> Signal handling is correctly done for SIGQUIT and SIGTERM for shutdown only.
> Attached is a patch fixing that, I simply added a call to
> SignalUnconnectedWorkers in SIGHUP_handler:postmaster.c.

Thanks, applied.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Michael Paquier escribió:
> Hi all,
>
> Please find attached a simple example of bgworker that logs a message each
> time a SIGTERM or SIGHUP signal is received by it:
> - "hello signal: processed SIGHUP" when SIGHUP is handled by my example
> - "hello signal: processed SIGTERM" when SIGTERM is handled by my example

I committed some improvements to worker_spi this morning that I think
enough demostrate signal handling capabilities, which I think is what
your submitted code would do.  If you see more use for a separate body
of sample worker code, by all means do submit that.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

<div dir="ltr">Thanks for committing the fix!<br /><div class="gmail_extra"><br /><div class="gmail_quote">On Thu, Apr
11,2013 at 4:11 AM, Alvaro Herrera <span dir="ltr"><<a href="mailto:alvherre@2ndquadrant.com"
target="_blank">alvherre@2ndquadrant.com</a>></span>wrote:<br /><blockquote class="gmail_quote" style="margin:0px
0px0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Michael Paquier escribió:<br /><div
class="im">>Hi all,<br /> ><br /> > Please find attached a simple example of bgworker that logs a message
each<br/> > time a SIGTERM or SIGHUP signal is received by it:<br /> > - "hello signal: processed SIGHUP" when
SIGHUPis handled by my example<br /> > - "hello signal: processed SIGTERM" when SIGTERM is handled by my example<br
/><br/></div>I committed some improvements to worker_spi this morning that I think<br /> enough demostrate signal
handlingcapabilities, which I think is what<br /> your submitted code would do.  If you see more use for a separate
body<br/> of sample worker code, by all means do submit that.<br /></blockquote></div>Sure.<br />-- <br />Michael<br
/></div></div>