Allowing multiple concurrent base backups

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

I implemented this in two ways, and can't decide which I like better:

1. The contents of the backup label file are returned to the caller of
do_pg_start_backup() as a palloc'd string.

2. do_pg_start_backup() creates a temporary file that the backup label
is written to (instead of "backup_label").

Implementation 1 changes more code, as pg_start/stop_backup() need to be
changed to write/read from memory instead of file, but the result isn't
any more complicated. Nevertheless, I somehow feel more comfortable with 2.

Seems like either one of these is fairly problematic in that you have to
have some monstrous kluge to get the backup_label file to appear with
the right name in the tarfile. How badly do we actually need this?
I don't think the use-case for concurrent base backups is all that large
in practice given the I/O hit it's going to involve.

regards, tom lane

On Tue, Jan 11, 2011 at 19:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

I implemented this in two ways, and can't decide which I like better:

1. The contents of the backup label file are returned to the caller of
do_pg_start_backup() as a palloc'd string.

2. do_pg_start_backup() creates a temporary file that the backup label
is written to (instead of "backup_label").

Implementation 1 changes more code, as pg_start/stop_backup() need to be
changed to write/read from memory instead of file, but the result isn't
any more complicated. Nevertheless, I somehow feel more comfortable with 2.

Seems like either one of these is fairly problematic in that you have to
have some monstrous kluge to get the backup_label file to appear with
the right name in the tarfile.  How badly do we actually need this?
I don't think the use-case for concurrent base backups is all that large
in practice given the I/O hit it's going to involve.

I think it can be done cleaner in the tar file injection - I've been
chatting with Heikki offlist about that. Not sure, but I have a
feeling it does.

As for the use-case, it doesn't necessarily involve a huge I/O hit if
either the entire DB is in RAM (not all that uncommon - though then
the backup finishes quickly as well), or if the *client* is slow, thus
making the server backlogged on sending the base backup.

Having it possible to do it concurrently also makes for *much* easier
use of the feature. It might be just about overlapping with a few
seconds, for example - which would probably have no major effect, but
takes a lot more code on the other end to work around.

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

On 11.01.2011 20:51, Tom Lane wrote:

Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes:

I implemented this in two ways, and can't decide which I like better:

1. The contents of the backup label file are returned to the caller of
do_pg_start_backup() as a palloc'd string.

2. do_pg_start_backup() creates a temporary file that the backup label
is written to (instead of "backup_label").

Implementation 1 changes more code, as pg_start/stop_backup() need to be
changed to write/read from memory instead of file, but the result isn't
any more complicated. Nevertheless, I somehow feel more comfortable with 2.

Seems like either one of these is fairly problematic in that you have to
have some monstrous kluge to get the backup_label file to appear with
the right name in the tarfile.

Oh. I'm surprised you feel that way - that part didn't feel ugly or
kludgey at all to me.

How badly do we actually need this?
I don't think the use-case for concurrent base backups is all that large
in practice given the I/O hit it's going to involve.

It makes it very convenient to set up standbys, without having to worry
that you'll conflict e.g with a nightly backup. I don't imagine people
will use streaming base backups for very large databases anyway.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

It makes it very convenient to set up standbys, without having to worry
that you'll conflict e.g with a nightly backup. I don't imagine people
will use streaming base backups for very large databases anyway.

Also, imagine that you're provisioning a 10-node replication cluster on
EC2. This would make that worlds easier.

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

Magnus Hagander <magnus@hagander.net> writes:

On Tue, Jan 11, 2011 at 19:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Seems like either one of these is fairly problematic in that you have to
have some monstrous kluge to get the backup_label file to appear with
the right name in the tarfile. �How badly do we actually need this?
I don't think the use-case for concurrent base backups is all that large
in practice given the I/O hit it's going to involve.

I think it can be done cleaner in the tar file injection - I've been
chatting with Heikki offlist about that. Not sure, but I have a
feeling it does.

One point that I'm particularly interested to see how you'll kluge it
is ensuring that the tarball contains only the desired temp data and not
also the "real" $PGDATA/backup_label, should there be a normal base
backup being done concurrently with the streamed one.

The whole thing just seems too fragile and dangerous to be worth dealing
with given that actual usage will be a corner case. *I* sure wouldn't
trust it to work when the chips were down.

regards, tom lane

On 11.01.2011 20:17, Heikki Linnakangas wrote:

Patches for both approaches attached. They're also available in my
github repository at git@github.com:hlinnaka/postgres.git.

Just so people won't report the same issues again, a couple of bugs have
already cropped up (thanks Magnus):

* a backup_label file in the data directory should now not be tarred up
- we're injecting a different backup_label file there.

* pg_start_backup_callback needs to be updated to just decrement
forcePageWrites, not reset it to 'false'

* pg_stop_backup() decrements forcePageWrites twice. oops.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

Now that we have a basic over-the-wire base backup capability in walsender,
it would be nice to allow taking multiple base backups at the same time.

I would prefer to be able to take a base backup from a standby, or is
that already possible? What about cascading the wal shipping? Maybe
those ideas are much bigger projects, but if I don't ask, I don't get to
know :)

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On 11.01.2011 21:50, Dimitri Fontaine wrote:

Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes:

Now that we have a basic over-the-wire base backup capability in walsender,
it would be nice to allow taking multiple base backups at the same time.

I would prefer to be able to take a base backup from a standby, or is
that already possible? What about cascading the wal shipping? Maybe
those ideas are much bigger projects, but if I don't ask, I don't get to
know :)

Yeah, those are bigger projects..

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, 2011-01-11 at 20:17 +0200, Heikki Linnakangas wrote:

So, this patch modifies the internal do_pg_start/stop_backup functions
so that in addition to the traditional mode of operation, where a
backup_label file is created in the data directory where it's backed up
along with all other files, the backup label file is be returned to the
caller, and the caller is responsible for including it in the backup.
The code in replication/basebackup.c includes it in the tar file that's
streamed the client, as "backup_label".

Perhaps we can use this more intelligent form of base backup to
differentiate between:
a. a primary that has crashed while a backup was in progress; and
b. an online backup that is being restored.

Allowing the user to do an unrestricted file copy as a base backup
doesn't allow us to make that differentiation. That lead to the two bugs
that we fixed in StartupXLOG(). And right now there are still two
problems remaining (albeit less severe):

1. If it's a primary recovering from a crash, and there is a
backup_label file, and the WAL referenced in the backup_label exists,
then it does a bunch of extra work during recovery; and
2. In the same situation, if the WAL referenced in the backup_label
does not exist, then it PANICs with a HINT to tell you to remove the
backup_label.

Is this an opportunity to solve these problems and simplify the code?

Regards,
Jeff Davis

On 11.01.2011 22:16, Jeff Davis wrote:

On Tue, 2011-01-11 at 20:17 +0200, Heikki Linnakangas wrote:

So, this patch modifies the internal do_pg_start/stop_backup functions
so that in addition to the traditional mode of operation, where a
backup_label file is created in the data directory where it's backed up
along with all other files, the backup label file is be returned to the
caller, and the caller is responsible for including it in the backup.
The code in replication/basebackup.c includes it in the tar file that's
streamed the client, as "backup_label".

Perhaps we can use this more intelligent form of base backup to
differentiate between:
a. a primary that has crashed while a backup was in progress; and
b. an online backup that is being restored.

Allowing the user to do an unrestricted file copy as a base backup
doesn't allow us to make that differentiation. That lead to the two bugs
that we fixed in StartupXLOG(). And right now there are still two
problems remaining (albeit less severe):

1. If it's a primary recovering from a crash, and there is a
backup_label file, and the WAL referenced in the backup_label exists,
then it does a bunch of extra work during recovery; and
2. In the same situation, if the WAL referenced in the backup_label
does not exist, then it PANICs with a HINT to tell you to remove the
backup_label.

Is this an opportunity to solve these problems and simplify the code?

It won't change the situation for pg_start_backup(), but with the patch
the base backups done via streaming won't have those issues, because
backup_label is not created (with that name) in the master.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, 2011-01-11 at 22:56 +0200, Heikki Linnakangas wrote:

1. If it's a primary recovering from a crash, and there is a
backup_label file, and the WAL referenced in the backup_label exists,
then it does a bunch of extra work during recovery; and
2. In the same situation, if the WAL referenced in the backup_label
does not exist, then it PANICs with a HINT to tell you to remove the
backup_label.

Is this an opportunity to solve these problems and simplify the code?

It won't change the situation for pg_start_backup(), but with the patch
the base backups done via streaming won't have those issues, because
backup_label is not created (with that name) in the master.

Do you think we should change the backup protocol for normal base
backups to try to fix this? Or do you think that the simplicity of
unrestricted file copy is worth these problems?

We could probably make some fairly minor changes, like making a file on
the primary and telling users to exclude it from any base backup. The
danger, of course, is that they do copy it, and their backup is
compromised.

Regards,
Jeff Davis

On Jan 11, 2011, at 2:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The whole thing just seems too fragile and dangerous to be worth dealing
with given that actual usage will be a corner case. *I* sure wouldn't
trust it to work when the chips were down.

I hope this assessment proves to be incorrect, because like Magnus and Heikki, I think this will be a major usability improvement. It takes us from "there's a way to do that" to "it just works".

...Robert

On Tue, Jan 11, 2011 at 22:51, Jeff Davis <pgsql@j-davis.com> wrote:

On Tue, 2011-01-11 at 22:56 +0200, Heikki Linnakangas wrote:

  1. If it's a primary recovering from a crash, and there is a
backup_label file, and the WAL referenced in the backup_label exists,
then it does a bunch of extra work during recovery; and
  2. In the same situation, if the WAL referenced in the backup_label
does not exist, then it PANICs with a HINT to tell you to remove the
backup_label.

Is this an opportunity to solve these problems and simplify the code?

It won't change the situation for pg_start_backup(), but with the patch
the base backups done via streaming won't have those issues, because
backup_label is not created (with that name) in the master.

Do you think we should change the backup protocol for normal base
backups to try to fix this? Or do you think that the simplicity of
unrestricted file copy is worth these problems?

We could probably make some fairly minor changes, like making a file on
the primary and telling users to exclude it from any base backup. The
danger, of course, is that they do copy it, and their backup is
compromised.

I think keeping the flexibility is important. If it does add an extra
step I think that's ok once we have pg_basebackup, but it must be
reasonably *safe*. Corrupt backups from forgetting to exclude a file
seems not so.

But if the problem is you forgot to exclude it, can't you just remove
it at a later time?

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

On Tue, Jan 11, 2011 at 05:06:34PM -0500, Robert Haas wrote:

On Jan 11, 2011, at 2:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

The whole thing just seems too fragile and dangerous to be worth dealing
with given that actual usage will be a corner case. *I* sure wouldn't
trust it to work when the chips were down.

I hope this assessment proves to be incorrect, because like Magnus and Heikki, I think this will be a major usability improvement. It takes us from "there's a way to do that" to "it just works".

(It just works)++ :)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Tue, 2011-01-11 at 23:07 +0100, Magnus Hagander wrote:

I think keeping the flexibility is important. If it does add an extra
step I think that's ok once we have pg_basebackup, but it must be
reasonably *safe*. Corrupt backups from forgetting to exclude a file
seems not so.

Agreed.

But if the problem is you forgot to exclude it, can't you just remove
it at a later time?

If you think you are recovering the primary, and it's really the backup,
then you get corruption. It's too late to remove a file after that
(unless you have a backup of your backup ;) ).

If you think you are restoring a backup, and it's really a primary that
crashed, then you run into one of the two problems that I mentioned
(which are less severe than corruption, but very annoying).

Regards,
Jeff Davis

2011/1/11 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>:

On 11.01.2011 21:50, Dimitri Fontaine wrote:

Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  writes:

Now that we have a basic over-the-wire base backup capability in
walsender,
it would be nice to allow taking multiple base backups at the same time.

I would prefer to be able to take a base backup from a standby, or is
that already possible?  What about cascading the wal shipping?  Maybe
those ideas are much bigger projects, but if I don't ask, I don't get to
know :)

Yeah, those are bigger projects..

Way more interesting, IMHO.
Feature to allow multiple backup at the same time looks useless to me.
If DB is small, then it is not that hard to do that before or after a
possible nightly backup.
If DB is large necessary to comment ?

The only case I find interesting is if you can use the bandwith of
more than one ethernet device, else I would use rsync between nodes
after the inital basebackup, pretty sure.

--
 Heikki Linnakangas
 EnterpriseDB   http://www.enterprisedb.com

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

--
Cédric Villemain               2ndQuadrant
http://2ndQuadrant.fr/     PostgreSQL : Expertise, Formation et Support

On 11.01.2011 23:51, Jeff Davis wrote:

On Tue, 2011-01-11 at 22:56 +0200, Heikki Linnakangas wrote:

1. If it's a primary recovering from a crash, and there is a
backup_label file, and the WAL referenced in the backup_label exists,
then it does a bunch of extra work during recovery; and
2. In the same situation, if the WAL referenced in the backup_label
does not exist, then it PANICs with a HINT to tell you to remove the
backup_label.

Is this an opportunity to solve these problems and simplify the code?

It won't change the situation for pg_start_backup(), but with the patch
the base backups done via streaming won't have those issues, because
backup_label is not created (with that name) in the master.

Do you think we should change the backup protocol for normal base
backups to try to fix this? Or do you think that the simplicity of
unrestricted file copy is worth these problems?

We could probably make some fairly minor changes, like making a file on
the primary and telling users to exclude it from any base backup. The
danger, of course, is that they do copy it, and their backup is
compromised.

Yeah, I don't think it's a good idea to provide such a foot-gun.

Last time we discussed this there was the idea of creating a file in
$PGDATA, and removing read-permissions from it, so that it couldn't be
accidentally included in the backup. Even with that safeguard, it
doesn't feel very safe - a backup running as root or some other special
privileges might still be able to read it.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, Jan 11, 2011 at 8:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Tue, Jan 11, 2011 at 19:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Seems like either one of these is fairly problematic in that you have to
have some monstrous kluge to get the backup_label file to appear with
the right name in the tarfile.  How badly do we actually need this?
I don't think the use-case for concurrent base backups is all that large
in practice given the I/O hit it's going to involve.

I think it can be done cleaner in the tar file injection - I've been
chatting with Heikki offlist about that. Not sure, but I have a
feeling it does.

One point that I'm particularly interested to see how you'll kluge it
is ensuring that the tarball contains only the desired temp data and not
also the "real" $PGDATA/backup_label, should there be a normal base
backup being done concurrently with the streamed one.

The whole thing just seems too fragile and dangerous to be worth dealing
with given that actual usage will be a corner case.  *I* sure wouldn't
trust it to work when the chips were down.

Maybe if pg_start_backup() notices that there is another backup
running should block waiting for another session to run
pg_stop_backup() ? Or have a new function like pg_start_backup_wait()
?

Considering that parallell base backups would be io-bound (or
network-bound), there is little need to actually run them in parallell
.

Greetings
Marcin

On Wed, Jan 12, 2011 at 10:26:05AM +0100, marcin mank wrote:

On Tue, Jan 11, 2011 at 8:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Tue, Jan 11, 2011 at 19:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Seems like either one of these is fairly problematic in that you have to
have some monstrous kluge to get the backup_label file to appear with
the right name in the tarfile. �How badly do we actually need this?
I don't think the use-case for concurrent base backups is all that large
in practice given the I/O hit it's going to involve.

I think it can be done cleaner in the tar file injection - I've been
chatting with Heikki offlist about that. Not sure, but I have a
feeling it does.

One point that I'm particularly interested to see how you'll kluge it
is ensuring that the tarball contains only the desired temp data and not
also the "real" $PGDATA/backup_label, should there be a normal base
backup being done concurrently with the streamed one.

The whole thing just seems too fragile and dangerous to be worth dealing
with given that actual usage will be a corner case. �*I* sure wouldn't
trust it to work when the chips were down.

Maybe if pg_start_backup() notices that there is another backup
running should block waiting for another session to run
pg_stop_backup() ? Or have a new function like pg_start_backup_wait()
?

Considering that parallell base backups would be io-bound (or
network-bound), there is little need to actually run them in parallell

That's not actually true. Backups at the moment are CPU-bound, and
running them in parallel is one way to make them closer to I/O-bound,
which is what they *should* be.

There are other proposals out there, and some work being done, to make
backups less dependent on CPU, among them:

- Making the on-disk representation smaller
- Making COPY more efficient

As far as I know, none of this work is public yet.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate