public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chapman Flack <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: David Steele <[email protected]>
Cc: [email protected]
Subject: Re: Postgres restart in the middle of exclusive backup and the presence of backup_label file
Date: Wed, 9 Mar 2022 18:11:19 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <20220309222124.GB180567@nathanxps13>
References: <[email protected]>
	<20220308200107.GA85558@nathanxps13>
	<[email protected]>
	<20220308221253.GA169440@nathanxps13>
	<[email protected]>
	<[email protected]>
	<CABUevExC0kaA9DY67tEgkbLBf6dmRtU9KgrLwSqVQjYPte7erQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<20220309222124.GB180567@nathanxps13>

On 03/09/22 17:21, Nathan Bossart wrote:
> 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?

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

view thread (9+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Postgres restart in the middle of exclusive backup and the presence of backup_label file
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox