On Thu, Aug 24, 2017 at 10:49 PM, David Steele <david@pgmasters.net> wrote:
> Thanks for reviewing! Sorry for the late response, those eclipses don't
> just chase themselves...
That's quite something to see.
> On 8/20/17 10:22 PM, Michael Paquier wrote:
>> On Fri, Aug 18, 2017 at 3:35 AM, David Steele <david@pgmasters.net> wrote:
>>
>> + Prior to PostgreSQL 9.6, this
>> Markup <productname>?
>
> Fixed.
>
>> + Note well that if the server crashes during the backup it may not be
>> + possible to restart until the <literal>backup_label</> file has been
>> + manually deleted from the PGDATA directory.
>> Missing a markup <envvar> here for PGDATA.
>
> Fixed.
>
>> s/Note well/Note as well/, no?
>
> This was a literal translation of nota bene but I've changed it to
> simply "Note" as that seems common in the docs.
Oh, OK.
>> Documentation does not state yet that the use of low-level APIs for
>> exclusive backups are not supported on standbys.
>
> The first paragraph of the exclusive section states, "this type of
> backup can only be taken on a primary".
Sorry, missed that.
>> Now in the docs:
>> If the backup process monitors and ensures that all WAL segment files
>> required for the backup are successfully archived then the second
>> parameter (which defaults to true) can be set to false to have
>> I would recommend adding some details here and mention
>> "wait_for_archive" instead of "second parameter".
>
> Done.
>
>> I am wondering as
>> well if this paragraph should be put in red with a warning or
>> something like that. This is really, really important to ensure
>> consistent backups!
>
> Maybe, but this logic could easily apply to a lot of sections in the
> backup docs. I'm not sure where it would end.
True as well. The patch looks good to me. If a committer does not show
up soon, it may be better to register that in the CF and wait. I am
not sure that adding an open item is suited, as docs have the same
problem on 9.6.
--
Michael