Re: Updated backup APIs for non-exclusive backups

Greetings,

* Laurenz Albe (laurenz.albe@cybertec.at) wrote:
> On Sun, 2016-04-24 at 11:49 -0400, Stephen Frost wrote:
> > * Magnus Hagander (magnus@hagander.net) wrote:
> > > On Sun, Apr 24, 2016 at 5:37 AM, Bruce Momjian <bruce@momjian.us> wrote:
> > > > On Fri, Apr 22, 2016 at 11:53:46AM -0400, Robert Haas wrote:
> > > > >
> > > > > Frankly, I think that's right.  It is one thing to say that the new
> > > > > method is preferred - +1 for that.  But the old method is going to
> > > > > continue to be used by many people for a long time, and in some cases
> > > > > will be superior.  That's not something we can deprecate, unless I'm
> > > > > misunderstanding the situation.
> > > >
> > > > I agree with Robert.  One the one hand we are saying pg_stop_backup()
> > > > doesn't work well in psql because you get those two file contents output
> > > > that you have to write, and on the other hand we are saying we are going
> > > > to deprecate the method that does work well in psql?  I must be missing
> > > > something too, as that makes no sense.
> > >
> > > I don't agree. I don't see how "making a backup using psql" is more
> > > important than "making a backup without potentially dangerous sideeffects".
> > > But if others don't agree, could one of you at least provide an example of
> > > how you'd like the docs to read in a way that doesn't deprecate the unsafe
> > > way but still informs the user properly?
> >
> > I'm with Magnus on this, primairly because I've come to understand just
> > how dangerous the old backup method is.  That method *should* be
> > deprecated and discouraged.  A backup method which means your database
> > doesn't restart properly if the system crashes during the backup is
> > *bad*.
> >
> > Fixing that means using something more complicated than the old method
> > and that's a bit of a pain in psql, but that doesn't mean we should tell
> > people that the old method is an acceptable approach.
> >
> > Perhaps we can look at improving psql to make it easier to use it for
> > the new backup method but, honestly, all these hackish scripts to do
> > backups aren't doing a lot of things that a real backup solution needs
> > to do.  Improving psql for this is a bit like new paint on the titanic.
>
> I guess that the danger mentioned above is that the database may not
> restart if it crashes while in exclusive backup mode, because the
> WAL containing the starting checkpoint has already been purged.

The database would think it's trying to do recovery from a backup, not
doing crash recovery like it should be doing, so, yes, that's pretty
ugly.

> I would say the typical use case for the exclusive backup method is
> the following (and I have seen it often):
>
> You have some kind of backup software that does file system backups
> and is able to run a "pre-backup" and "post-backup" script.
> The backup is triggered by the backup software.

Seeing it often doesn't make it a good solution.  Running just
pre-backup and post-backup scripts and copying the filesystem isn't
enough to perform an online PostgreSQL backup- the WAL needs to be
collected as well, and you need to make sure that you have all of the
WAL before the backup can be considered complete.  On restore, you're
going to need to create a recovery.conf (at least in released versions)
which provides a restore command (needed even in HEAD today) to get the
old WAL, so having to also create the backup_label file shouldn't be
that difficult.

Lastly, if you really want, you can extract out the data from
pg_stop_backup in whatever your post-backup script is.

> Another thing that is problematic with non-exclusive backups is that
> you have to write the backup_label file into the backup after the
> backup has been taken.  With a technique like the above, you cannot
> easily do that.

... why not?  You need to create the recovery.conf anyway, and you need
to be archiving WAL somewhere, so it certainly seems like you could put
the backup_label there too.

> So in essence, all backup methods that work like the above won't be
> possible any more once the exclusive backup is gone.

That's why it's been deprecated and not removed...  but hopefully we
will be able to remove it in the future as it really isn't a good API.

> I expect that that will make a lot of users unhappy.

If it means that they implement a better backup strategy, then it's
probably a good thing, which is the goal of this.

> Would it be an option to change the semantics so that "backup_label"
> is ignored if there is no "recovery.conf"?  Of course that opens
> another way to corrupt your database (by starting it from a backup
> without first creating "recovery.conf"), but we could add another
> big warning.

My recollection is that other approaches were considered to deal with
this but none of them were all that good, though that was two years ago
at this point.  Certainly the issue with the above where a restored
backup without a recovery.conf could unwittingly be treated as if it
was crash recovery and end up with a very incorrect database as a
result, without any clear indication of that being the case, is quite
bad.

And, really, the arguments for keeping this API around and figuring out
how to 'fix' it would have to outweigh the cost of asking users to
reevaluate their existing solutions and implementing ones that use the
exclusive API, which is pretty clearly better.  What's more, any change
we make to the deprecated API has the chance of breaking existing users
too.  If we're going to make some kind of breaking change, let's make it
in a way that eliminates as many possible pitfalls as we can, and that's
basically what the non-exclusive API does.

I don't see any compelling argument for trying to do something half-way
any more today than I did two years ago when this was being discussed.

Thanks!

Stephen