SIGHUP not received by custom bgworkers if postmaster is notified

Started by Michael Paquieralmost 13 years ago8 messages

michael.paquier@gmail.com

almost 13 years ago

1 attachment(s)

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

Attachments:

20130321_bgworker_sighup.patchapplication/octet-stream; name=20130321_bgworker_sighup.patchDownload

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 15c2320..a77210b 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2280,6 +2280,7 @@ SIGHUP_handler(SIGNAL_ARGS)
 				(errmsg("received SIGHUP, reloading configuration files")));
 		ProcessConfigFile(PGC_SIGHUP);
 		SignalChildren(SIGHUP);
+		SignalUnconnectedWorkers(SIGHUP);
 		if (StartupPID != 0)
 			signal_child(StartupPID, SIGHUP);
 		if (BgWriterPID != 0)

Euler Taveira

euler@timbira.com

almost 13 years ago

In reply to: Michael Paquier (#1)

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

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]/messages/by-id/20121231140353.GC4363@alvh.no-ip.org, 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]/messages/by-id/1357210591.1964.22.camel@localhost.localdomain for how to process
configurantion file).

[1]: /messages/by-id/20121231140353.GC4363@alvh.no-ip.org
[2]: /messages/by-id/1357210591.1964.22.camel@localhost.localdomain
/messages/by-id/1357210591.1964.22.camel@localhost.localdomain

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera

alvherre@2ndquadrant.com

almost 13 years ago

In reply to: Euler Taveira (#2)

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

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

almost 13 years ago

In reply to: Alvaro Herrera (#3)

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

On Fri, Mar 22, 2013 at 12:15 AM, Alvaro Herrera
<alvherre@2ndquadrant.com>wrote:

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.

Yes, that's exactly the problem. And I believe that the postmaster should
also notify its registered bgworkers if it receives a SIGHUP as it does for
its other backends. Have a look at the 1-line patch I sent to see how I
fixed
that...
--
Michael

Michael Paquier

michael.paquier@gmail.com

almost 13 years ago

In reply to: Michael Paquier (#4)

2 attachment(s)

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

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

Attachments:

hello_signal.tar.gzapplication/x-gzip; name=hello_signal.tar.gzDownload

20130321_bgworker_sighup.patchapplication/octet-stream; name=20130321_bgworker_sighup.patchDownload

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 15c2320..a77210b 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2280,6 +2280,7 @@ SIGHUP_handler(SIGNAL_ARGS)
 				(errmsg("received SIGHUP, reloading configuration files")));
 		ProcessConfigFile(PGC_SIGHUP);
 		SignalChildren(SIGHUP);
+		SignalUnconnectedWorkers(SIGHUP);
 		if (StartupPID != 0)
 			signal_child(StartupPID, SIGHUP);
 		if (BgWriterPID != 0)

Alvaro Herrera

alvherre@2ndquadrant.com

almost 13 years ago

In reply to: Michael Paquier (#1)

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

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera

alvherre@2ndquadrant.com

almost 13 years ago

In reply to: Michael Paquier (#5)

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

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

almost 13 years ago

In reply to: Alvaro Herrera (#7)

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

Thanks for committing the fix!

On Thu, Apr 11, 2013 at 4:11 AM, Alvaro Herrera <alvherre@2ndquadrant.com>wrote:

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.

Sure.
--
Michael