Subject: Re: Postgres restart in the middle of exclusive backup and the presence of backup_label file

From: Chapman Flack <[email-masked]>
Date: Tue, 8 Mar 2022 20:24:13 -0500

On 03/08/22 17:12, Nathan Bossart wrote:
> I spent some time trying to come up with a workable script to replace the
> existing one.  I think the main problem is that you need to write out both
> the backup label file and the tablespace map file, but I didn't find an
> easy way to write the different output columns of pg_backup_stop() to
> separate files via psql.

Something like this might work:

SELECT * FROM pg_backup_stop(true) \gset

\out /tmp/backup_label   \qecho :labelfile
\out /tmp/tablespace_map \qecho :spcmapfile
\out
\! ... tar command adding /tmp/{backup_label,tablespace_map} to the tarball

I notice the \qecho adds a final newline (and so if :spcmapfile is empty,
a file containing a single newline is made). In a quick test with a bogus
restore_command, I did not see any error messages specific to the format
of the backup_label or tablespace_map files, so maybe the final newline
isn't a problem.

Assuming the newline isn't a problem, that might be simple enough to
use in an example, and maybe it's not a bad thing that it highlights a few
psql capabilities the reader might not have stumbled on before. Or, maybe
it is just too confusing to bother.

While agreeing that pg_basebackup is the production-ready thing that
does it all for you (with tests for likely errors and so on), I think
there is also some value in a dead-simple example that concretely
shows you what "it" is, what the basic steps are that happen beneath
pg_basebackup's chrome.

If the added newline is a problem, I haven't thought of a way to exclude
it that doesn't take the example out of the realm of dead-simple.

Regards,
-Chap

---

From: Stephen Frost <[email-masked]>
Date: Wed, 9 Mar 2022 10:42:10 -0500

Greetings,

> > easy way to write the different output columns of pg_backup_stop() to

Let's not confuse ourselves here- the existing script *doesn't* work in
any reasonable way when we're talking about everything that needs to be
done to perform a backup.  That a lot of people are using it because
it's in the documentation is an actively bad thing.

The same goes for the archive command example.

> \! ... tar command adding /tmp/{backup_label,tablespace_map} to the tarball

... this doesn't do what's needed either.  We could try to write down
some minimum set of things that are needed for a backup tool to do but
it's not something that a 3 line script is going to cover.  Indeed, it's
a lot more like pg_basebackup and if we want to give folks a script to
use, it should be "run pg_basebackup".

> it is just too confusing to bother.

It's more than just too confusing, it's actively bad because people will
actually use it and then end up with backups that don't work.

> shows you what "it" is, what the basic steps are that happen beneath

Documenting everything that pg_basebackup does to make sure that the
backup is viable might be something to work on if someone is really
excited about this, but it's not 'dead-simple' and it's darn close to
the bare minimum, something that none of these simple scripts will come
anywhere close to being and instead they'll be far less than the
minimum.

> it that doesn't take the example out of the realm of dead-simple.

I disagree that there's really a way to provide 'dead-simple' backups
with what's built into core without using pg_basebackup.  If we want a
'dead-simple' solution in core then we'd need to write an appropriate
backup tool that does all the basic things and include and maintain
that.  Writing a shell script isn't enough and we shouldn't encourage
our users to do exactly that by having it in our documentation because
then they'll think it's enough.

Thanks,

Stephen

---

From: Magnus Hagander <[email-masked]>
Date: Wed, 9 Mar 2022 17:22:56 +0100

> actually use it and then end up with backups that don't work.

+1.

Or even worse, backups that sometimes work, but not reliably and not every time.

> > shows you what "it" is, what the basic steps are that happen beneath

I agree that having a dead simple script would be good.

The *only* dead simple script that's going to be possible is one that
calls pg_basebackup.

The current APIs don't make it *possible* to drive them directly with
a dead simple script.

Pretending something is simple when it's not, is not doing anybody a favor.

> anywhere close to being and instead they'll be far less than the

Yeah, having the full set of steps required documented certainly
wouldn't be a bad thing. But it's a very *different* thing.

> then they'll think it's enough.

+1.

We need to accept that the current APIs are far too low level to be
driven by a shellscript. No matter how much documentation we write is
not going to change that fact.

For the people who want to drive their backups from a shellscript and
for some reason *don't* want to use pg_basebackup, we need to come up
with a different API or a different set of tools. That is not a
documentation task. That is a "start from a list of which things
pg_basebackup cannot do that are still simple, or that tools like
pgbackrest cannot do if they're complicated".  And then design an API
that's actually safe and easy to use *for that usecase*.

For example, if the use case is "i want to use filesystemor SAN
snapshots for my backups", we shouldn't try to write workarounds using
bash coprocs or whatever. Instead, we could write a tool that
interacts with the current api to start the backup, then launches a
shellscript that interacts with the snapshot system, and then stops
the backup after. With a well defined set of rules for how that shell
script should work and interact.

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

---

From: Chapman Flack <[email-masked]>
Date: Wed, 9 Mar 2022 12:12:07 -0500

On 03/09/22 11:22, Magnus Hagander wrote:
> ...

Okay, I bow to this reasoning, for the purpose of this patch. Let's
just lose the example.

> wouldn't be a bad thing.

I'd say that qualifies as an understatement. While it certainly doesn't
have to be part of this patch, if the claim is that an admin who relies
on pg_basebackup is relying on essential things pg_basebackup does that
have not been enumerated in our documentation yet, I would argue they
should be.

> that's actually safe and easy to use *for that usecase*.

That might also be a good thing, but I don't see it as a substitute
for documenting the present reality of what the irreducibly essential
behaviors of pg_basebackup (or of third-party tools like pgbackrest)
are, and why they are so.

Regards,
-Chap

---

From: Stephen Frost <[email-masked]>
Date: Wed, 9 Mar 2022 12:19:23 -0500

Greetings,

> Okay, I bow to this reasoning, for the purpose of this patch. Let's

Great.

> have not been enumerated in our documentation yet, I would argue they

It doesn't have to be part of this patch and we should move forward with
this patch.  Let's avoid hijacking this thread, which is about this
patch, for an independent debate about what our documentation should or
shouldn't include.

> are, and why they are so.

I disagree.  If we provided a tool then we'd document that tool and how
users can use it, not every single step that it does (see also:
pg_basebackup).

Thanks,

Stephen

---

From: Chapman Flack <[email-masked]>
Date: Wed, 9 Mar 2022 14:32:24 -0500

On 03/09/22 12:19, Stephen Frost wrote:
> Let's avoid hijacking this thread, which is about this

Agreed. New thread here:

https://www.postgresql.org/message-id/6228FFE4.3050309%40anastigmatix.net

Regards,
-Chap

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 9 Mar 2022 14:21:24 -0800

> https://www.postgresql.org/message-id/6228FFE4.3050309%40anastigmatix.net

Great.  Is there any additional feedback on this patch?  Should we add an
example of using pg_basebackup in the "Standalone Hot Backups" section, or
should we leave all documentation additions like this for Chap's new
thread?

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

---

From: Chapman Flack <[email-masked]>
Date: Wed, 9 Mar 2022 18:11:19 -0500

> should we leave all documentation additions like this for Chap's new

I'm composing something longer for the new thread, but on the way
I noticed something we might fit into this one.

I think the listitem

  In the same connection as before, issue the command:
  SELECT * FROM pg_backup_stop(true);

would be clearer if it used named-parameter form, wait_for_archive => true.

This is not strictly necessary, of course, for a function with a single
IN parameter, but it's good documentation (and also could save us headaches
like these if there is ever another future need to give it more parameters).

That listitem doesn't say anything about what the parameter means, which
is a little weird, but probably ok because the next listitem does go into
it in some detail. I don't think a larger reorg is needed to bring that
language one listitem earlier. Just naming the parameter is probably
enough to make it less puzzling (or adding in that listitem, at most,
"the effect of the wait_for_archive parameter is explained below").

For consistency (and the same futureproofing benefit), I'd go to
fast => false in the earlier pg_backup_start as well.

I'm more ambivalent about label => 'label'. It would be consistent,
but should we just agree for conciseness that there will always be
a label and it will always be first?

You can pretty much tell in a call what's a label; it's those anonymous
trues and falses that are easier to read with named notation.

Regards,
-Chap