pgsql: Handle SIGTERM in pg_receivewal and pg_recvlogical

Started by Daniel Gustafssonalmost 4 years ago5 messagescomitters
Jump to latest
#1Daniel Gustafsson
daniel@yesql.se

Handle SIGTERM in pg_receivewal and pg_recvlogical

In pg_receivewal, compressed output is only flushed on clean exits. The
reason to support SIGTERM as well as SIGINT (which is currently handled)
is that pg_receivewal might well be running as a daemon, and systemd's
default KillSignal is SIGTERM.

Since pg_recvlogical is also supposed to run as a daemon, teach it about
SIGTERM as well and update the documentation to match. While in there,
change pg_receivewal's time_to_stop to be sig_atomic_t like it is in
pg_recvlogical.

Author: Christoph Berg <myon@debian.org>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: /messages/by-id/Yvo/5No5S0c4EFMj@msg.df7cb.de

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/8b60db774356117fab2eb53fb37160fa3e173cdb

Modified Files
--------------
doc/src/sgml/ref/pg_receivewal.sgml | 8 +++++---
doc/src/sgml/ref/pg_recvlogical.sgml | 18 ++++++++++++++++++
src/bin/pg_basebackup/pg_receivewal.c | 11 ++++++-----
src/bin/pg_basebackup/pg_recvlogical.c | 9 +++++----
4 files changed, 34 insertions(+), 12 deletions(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#1)
Re: pgsql: Handle SIGTERM in pg_receivewal and pg_recvlogical

Daniel Gustafsson <dgustafsson@postgresql.org> writes:

Since pg_recvlogical is also supposed to run as a daemon, teach it about
SIGTERM as well and update the documentation to match. While in there,
change pg_receivewal's time_to_stop to be sig_atomic_t like it is in
pg_recvlogical.

While looking at this commit, I wondered why both of those programs
are declaring their signal handlers like

static void
sigint_handler(int signum)

rather than our standard convention

static void
pmdie(SIGNAL_ARGS)

Evidently we don't (any longer?) have any platforms where SIGNAL_ARGS
is non-default, but that still doesn't make this good coding. More
than once I've grepped for SIGNAL_ARGS to locate signal handlers.

Hmm, looks like same mistake in pg_waldump ... barring objection,
I'm going to run around and fix those.

I also notice that port.h has

typedef void (*pqsigfunc) (int signo);
extern pqsigfunc pqsignal(int signo, pqsigfunc func);

which is a bit inconsistent with the idea that SIGNAL_ARGS
might be different from that. Shouldn't we declare it as

typedef void (*pqsigfunc) (SIGNAL_ARGS);

?? I'm definitely going to do that for testing, because then
I can alter SIGNAL_ARGS to help me find any other stragglers.

regards, tom lane

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#2)
Re: pgsql: Handle SIGTERM in pg_receivewal and pg_recvlogical

On 14 Sep 2022, at 17:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Daniel Gustafsson <dgustafsson@postgresql.org> writes:

Since pg_recvlogical is also supposed to run as a daemon, teach it about
SIGTERM as well and update the documentation to match. While in there,
change pg_receivewal's time_to_stop to be sig_atomic_t like it is in
pg_recvlogical.

While looking at this commit, I wondered why both of those programs
are declaring their signal handlers like

static void
sigint_handler(int signum)

rather than our standard convention

static void
pmdie(SIGNAL_ARGS)

Sorry, I had missed that convention.

Evidently we don't (any longer?) have any platforms where SIGNAL_ARGS
is non-default, but that still doesn't make this good coding. More
than once I've grepped for SIGNAL_ARGS to locate signal handlers.

Hmm, looks like same mistake in pg_waldump ...

pg_test_fsync seems to have the same error.

barring objection,
I'm going to run around and fix those.

No objections, unless you have better things to do and wan't me to clean up
then I'll take care of it.

I also notice that port.h has

typedef void (*pqsigfunc) (int signo);
extern pqsigfunc pqsignal(int signo, pqsigfunc func);

which is a bit inconsistent with the idea that SIGNAL_ARGS
might be different from that. Shouldn't we declare it as

typedef void (*pqsigfunc) (SIGNAL_ARGS);

Agreed, that makes sense.

--
Daniel Gustafsson https://vmware.com/

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#3)
Re: pgsql: Handle SIGTERM in pg_receivewal and pg_recvlogical

Daniel Gustafsson <dgustafsson@postgresql.org> writes:

On 14 Sep 2022, at 17:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

barring objection,
I'm going to run around and fix those.

No objections, unless you have better things to do and wan't me to clean up
then I'll take care of it.

Nah, not your bug. There are a bunch of these, I'm finding ...

regards, tom lane

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#4)
Re: pgsql: Handle SIGTERM in pg_receivewal and pg_recvlogical

On 14 Sep 2022, at 17:37, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Daniel Gustafsson <dgustafsson@postgresql.org> writes:

On 14 Sep 2022, at 17:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

barring objection,
I'm going to run around and fix those.

No objections, unless you have better things to do and wan't me to clean up
then I'll take care of it.

Nah, not your bug. There are a bunch of these, I'm finding ...

Thanks for tackling this!

--
Daniel Gustafsson https://vmware.com/