Terminating pg_basebackup background streamer

Started by Magnus Haganderabout 12 years ago10 messageshackers
Jump to latest
#1Magnus Hagander
magnus@hagander.net

If an error occurs in the foreground (backup) process of pg_basebackup, and
we exit in a controlled way, the background process (streaming xlog
process) would stay around and keep streaming.

This can happen for example if disk space runs out and there is very low
activity on the server. (If there is activity on the server, the background
streamer will also run out of disk space and exit)

Attached patch kills it off in disconnect_and_exit(), which seems like the
right thing to do to me.

Any objections to applying and backpatching that for the upcoming minor
releases?

Should we perhaps also consider adding a sigterm handler to pg_basebackup,
so if you send it a SIGTERM it kills off the background process as well?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Attachments:

basebackup_kill.patchtext/x-patch; charset=US-ASCII; name=basebackup_kill.patchDownload+28-6
#2Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Magnus Hagander (#1)
Re: Terminating pg_basebackup background streamer

On 02/09/2014 02:17 PM, Magnus Hagander wrote:

If an error occurs in the foreground (backup) process of pg_basebackup, and
we exit in a controlled way, the background process (streaming xlog
process) would stay around and keep streaming.

This can happen for example if disk space runs out and there is very low
activity on the server. (If there is activity on the server, the background
streamer will also run out of disk space and exit)

Attached patch kills it off in disconnect_and_exit(), which seems like the
right thing to do to me.

Any objections to applying and backpatching that for the upcoming minor
releases?

Do you get a different error message with this patch than before? Is the
new one better than the old one?

- Heikki

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

#3Magnus Hagander
magnus@hagander.net
In reply to: Heikki Linnakangas (#2)
Re: Terminating pg_basebackup background streamer

On Mon, Feb 10, 2014 at 7:29 PM, Heikki Linnakangas <hlinnakangas@vmware.com

wrote:

On 02/09/2014 02:17 PM, Magnus Hagander wrote:

If an error occurs in the foreground (backup) process of pg_basebackup,
and
we exit in a controlled way, the background process (streaming xlog
process) would stay around and keep streaming.

This can happen for example if disk space runs out and there is very low
activity on the server. (If there is activity on the server, the
background
streamer will also run out of disk space and exit)

Attached patch kills it off in disconnect_and_exit(), which seems like the
right thing to do to me.

Any objections to applying and backpatching that for the upcoming minor
releases?

Do you get a different error message with this patch than before? Is the
new one better than the old one?

Previously you got double error messages - one from the foreground, and a
second one from the background sometime in the future (whenever it
eventually failed, and for whatever reason - so if it was out of disk
space, it would complain about that once it got enough xlog for it to
happen).

With the patch you just get the error message from the first process. The
background process doesn't give an error on SIGTERM, it just exists.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#4Magnus Hagander
magnus@hagander.net
In reply to: Magnus Hagander (#3)
Re: Terminating pg_basebackup background streamer

On Mon, Feb 10, 2014 at 7:39 PM, Magnus Hagander <magnus@hagander.net>wrote:

On Mon, Feb 10, 2014 at 7:29 PM, Heikki Linnakangas <
hlinnakangas@vmware.com> wrote:

On 02/09/2014 02:17 PM, Magnus Hagander wrote:

If an error occurs in the foreground (backup) process of pg_basebackup,
and
we exit in a controlled way, the background process (streaming xlog
process) would stay around and keep streaming.

This can happen for example if disk space runs out and there is very low
activity on the server. (If there is activity on the server, the
background
streamer will also run out of disk space and exit)

Attached patch kills it off in disconnect_and_exit(), which seems like
the
right thing to do to me.

Any objections to applying and backpatching that for the upcoming minor
releases?

Do you get a different error message with this patch than before? Is the
new one better than the old one?

Previously you got double error messages - one from the foreground, and a
second one from the background sometime in the future (whenever it
eventually failed, and for whatever reason - so if it was out of disk
space, it would complain about that once it got enough xlog for it to
happen).

With the patch you just get the error message from the first process. The
background process doesn't give an error on SIGTERM, it just exists.

Since there were no other objections, I've applied this patch.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Magnus Hagander (#4)
Re: Terminating pg_basebackup background streamer

On 2/12/14, 12:47 PM, Magnus Hagander wrote:

Since there were no other objections, I've applied this patch.

I'm getting a compiler warning:

pg_basebackup.c:105:3: error: implicit declaration of function 'kill'
[-Werror=implicit-function-declaration]

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

#6Magnus Hagander
magnus@hagander.net
In reply to: Peter Eisentraut (#5)
Re: Terminating pg_basebackup background streamer

On Wed, Feb 12, 2014 at 10:28 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 2/12/14, 12:47 PM, Magnus Hagander wrote:

Since there were no other objections, I've applied this patch.

I'm getting a compiler warning:

pg_basebackup.c:105:3: error: implicit declaration of function 'kill'
[-Werror=implicit-function-declaration]

What platform is that? And do you know which header the declaration
actually lives in? I don't see it here...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#7Andres Freund
andres@anarazel.de
In reply to: Magnus Hagander (#6)
Re: Terminating pg_basebackup background streamer

On February 12, 2014 10:34:47 PM CET, Magnus Hagander <magnus@hagander.net> wrote:

On Wed, Feb 12, 2014 at 10:28 PM, Peter Eisentraut <peter_e@gmx.net>
wrote:

On 2/12/14, 12:47 PM, Magnus Hagander wrote:

Since there were no other objections, I've applied this patch.

I'm getting a compiler warning:

pg_basebackup.c:105:3: error: implicit declaration of function 'kill'
[-Werror=implicit-function-declaration]

What platform is that? And do you know which header the declaration
actually lives in? I don't see it here...

Should be in the signal.h you added a bit later according to posix.

Andres

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

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

#8Peter Eisentraut
peter_e@gmx.net
In reply to: Magnus Hagander (#6)
Re: Terminating pg_basebackup background streamer

On 2/12/14, 4:34 PM, Magnus Hagander wrote:

On Wed, Feb 12, 2014 at 10:28 PM, Peter Eisentraut <peter_e@gmx.net
<mailto:peter_e@gmx.net>> wrote:

On 2/12/14, 12:47 PM, Magnus Hagander wrote:

Since there were no other objections, I've applied this patch.

I'm getting a compiler warning:

pg_basebackup.c:105:3: error: implicit declaration of function 'kill'
[-Werror=implicit-function-declaration]

What platform is that? And do you know which header the declaration
actually lives in? I don't see it here...

OS X, <signal.h> according to man page

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

#9Magnus Hagander
magnus@hagander.net
In reply to: Peter Eisentraut (#8)
Re: Terminating pg_basebackup background streamer

On Wed, Feb 12, 2014 at 10:53 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 2/12/14, 4:34 PM, Magnus Hagander wrote:

On Wed, Feb 12, 2014 at 10:28 PM, Peter Eisentraut <peter_e@gmx.net
<mailto:peter_e@gmx.net>> wrote:

On 2/12/14, 12:47 PM, Magnus Hagander wrote:

Since there were no other objections, I've applied this patch.

I'm getting a compiler warning:

pg_basebackup.c:105:3: error: implicit declaration of function 'kill'
[-Werror=implicit-function-declaration]

What platform is that? And do you know which header the declaration
actually lives in? I don't see it here...

OS X, <signal.h> according to man page

Are you sure you made that test after my fixup patch (the one suggested by
Andres)? Because that one was at least supposed to add signal.h...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Magnus Hagander (#9)
Re: Terminating pg_basebackup background streamer

On 2/13/14, 6:25 AM, Magnus Hagander wrote:

Are you sure you made that test after my fixup patch (the one suggested
by Andres)? Because that one was at least supposed to add signal.h...

works now

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