Thread

Commits

  1. Improve low-level backup documentation.

  1. Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-17T18:35:52Z

    As discussed in [1] our low-level backup documentation does not quite
    match the actual behavior of the functions on primary vs. standby.
    Since it appears we have decided that the remaining behavioral
    differences after 52f8a59dd953c68 are bugs in the documentation, the
    attached is a first pass at bringing the documentation up to date.
    
    The biggest change is to recognize that exclusive backups can only be
    run on a primary and to adjust the text accordingly.  Also, I did not
    mention the wait_for_archive param in the exclusive instructions since
    they are deprecated.
    
    This patch should be sufficient for 10/11 but will need some minor
    changes for 9.6 to remove the reference to wait_for_archive.  Note that
    this patch ignores Michael's patch [2] to create WAL history files on a
    standby as this will likely only be applied to master.
    
    In addition, I have formatted the text to produce minimal diffs for
    review, but it could be tightened up before commit.
    
    -- 
    -David
    david@pgmasters.net
    
    [1]
    https://www.postgresql.org/message-id/20170814152816.GF4628%40tamriel.snowman.net
    [2]
    https://www.postgresql.org/message-id/CAB7nPqQvVpMsqJExSVXHUwpXFRwojsb-jb4BYnxEQbjJLfw-yQ%40mail.gmail.com
    
  2. Re: Update low-level backup documentation to match actual behavior

    Robert Haas <robertmhaas@gmail.com> — 2017-08-18T18:45:28Z

    -     the next WAL segment.  The reason for the switch is to arrange for
    +     the next WAL segment when run on a primary.  On a standby you can call
    +     <function>pg_switch_wal</function> on the primary to perform a manual
    +     switch.
    +     The reason for the switch is to arrange for
    
    Tacking on "when run on a primary" onto the end of the existing
    sentence is a little ambiguous: does that clause apply only to the
    last part, or to the whole sentence?  I suggest something like: This
    terminates the backup mode.  On a primary, it also performs an
    automatic switch to the next WAL segment.  On a standby, it is not
    possible to automatically switch WAL segments, so you may wish to
    consider running <function>pg_switch_wal</function> on the primary to
    perform a manual switch.
    
    -     backup files.  If <varname>archive_mode</> is enabled,
    +     backup files.  On a primary, if <varname>archive_mode</> is
    enabled and the
    +     <literal>wait_for_archive</> parameter is <literal>true</>,
          <function>pg_stop_backup</> does not return until the last segment has
          been archived.
    +     On a standby, <varname>archive_mode</> must be <literal>always</> in order
    +     for <function>pg_stop_backup</> to wait.
    
    Looks good.
    
    -    <title>Making an exclusive low level backup</title>
    +    <title>Making an exclusive low level backup on a primary</title>
    
    I'd omit this hunk.
    
    -     more than one concurrent backup to run, and there can be some issues on
    +     more than one concurrent backup to run, must be run on a
    primary, and there
    +     can be some issues on
    
    Maybe this would be clearer: This type of backup can only be taken on
    a primary, does not allow more than one ...
    
    -     This function, when called on a primary, terminates the backup mode and
    +     This function terminates the backup mode and
          performs an automatic switch to the next WAL segment. The reason for the
          switch is to arrange for the last WAL segment written during the backup
    -     interval to be ready to archive.  When called on a standby, this function
    -     only terminates backup mode.  A subsequent WAL segment switch will be
    -     needed in order to ensure that all WAL files needed to restore the backup
    -     can be archived; if the primary does not have sufficient write activity
    -     to trigger one, <function>pg_switch_wal</function> should be executed on
    -     the primary.
    +     interval to be ready to archive.
    
    Why do you want to delete all that text?  It seems like good text to me.
    
    -    The function also creates a backup history file in the write-ahead log
    +    When executed on a primary, the function also creates a backup history file
    +    in the write-ahead log
    
    Looks good.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
    
  3. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-18T18:58:38Z

    Robert,
    
    Thanks for reviewing!
    
    On 8/18/17 2:45 PM, Robert Haas wrote:
    > -     the next WAL segment.  The reason for the switch is to arrange for
    > +     the next WAL segment when run on a primary.  On a standby you can call
    > +     <function>pg_switch_wal</function> on the primary to perform a manual
    > +     switch.
    > +     The reason for the switch is to arrange for
    > 
    > Tacking on "when run on a primary" onto the end of the existing
    > sentence is a little ambiguous: does that clause apply only to the
    > last part, or to the whole sentence?  I suggest something like: This
    > terminates the backup mode.  On a primary, it also performs an
    > automatic switch to the next WAL segment.  On a standby, it is not
    > possible to automatically switch WAL segments, so you may wish to
    > consider running <function>pg_switch_wal</function> on the primary to
    > perform a manual switch.
    
    Looks good.
    
    > 
    > -    <title>Making an exclusive low level backup</title>
    > +    <title>Making an exclusive low level backup on a primary</title>
    > 
    > I'd omit this hunk.
    
    OK, but I was trying to make it very clear that this backup method only
    works on a primary.  If you think the text is in the first paragraph is
    enough then I'm willing to go with that, though.
    
    > -     more than one concurrent backup to run, and there can be some issues on
    > +     more than one concurrent backup to run, must be run on a
    > primary, and there
    > +     can be some issues on
    > 
    > Maybe this would be clearer: This type of backup can only be taken on
    > a primary, does not allow more than one ...
    
    Looks good.
    
    > -     This function, when called on a primary, terminates the backup mode and
    > +     This function terminates the backup mode and
    >       performs an automatic switch to the next WAL segment. The reason for the
    >       switch is to arrange for the last WAL segment written during the backup
    > -     interval to be ready to archive.  When called on a standby, this function
    > -     only terminates backup mode.  A subsequent WAL segment switch will be
    > -     needed in order to ensure that all WAL files needed to restore the backup
    > -     can be archived; if the primary does not have sufficient write activity
    > -     to trigger one, <function>pg_switch_wal</function> should be executed on
    > -     the primary.
    > +     interval to be ready to archive.
    > 
    > Why do you want to delete all that text?  It seems like good text to me.
    
    Since the exclusive method only works on a primary...
    
    -- 
    -David
    david@pgmasters.net
    
    
    
  4. Re: Update low-level backup documentation to match actual behavior

    Robert Haas <robertmhaas@gmail.com> — 2017-08-18T19:00:20Z

    On Fri, Aug 18, 2017 at 2:58 PM, David Steele <david@pgmasters.net> wrote:
    > OK, but I was trying to make it very clear that this backup method only
    > works on a primary.  If you think the text is in the first paragraph is
    > enough then I'm willing to go with that, though.
    
    Yeah, I think the text is enough.
    
    > Since the exclusive method only works on a primary...
    
    Oh, right.  Duh.
    
    If you update the patch I'll apply it to 11 and 10.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
    
  5. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-18T22:46:12Z

    On 8/18/17 3:00 PM, Robert Haas wrote:
    > 
    > If you update the patch I'll apply it to 11 and 10.
    
    Attached is the updated patch.
    
    I didn't like the vague "there can be some issues on the server if it
    crashes during the backup" so I added a new paragraph at the appropriate
    step to give a more detailed explanation of the problem.
    
    Thanks,
    -- 
    -David
    david@pgmasters.net
    
  6. Re: Update low-level backup documentation to match actual behavior

    Michael Paquier <michael.paquier@gmail.com> — 2017-08-21T02:22:17Z

    On Fri, Aug 18, 2017 at 3:35 AM, David Steele <david@pgmasters.net> wrote:
    > This patch should be sufficient for 10/11 but will need some minor
    > changes for 9.6 to remove the reference to wait_for_archive.  Note that
    > this patch ignores Michael's patch [2] to create WAL history files on a
    > standby as this will likely only be applied to master.
    
    I'll just rebase as needed the other patch, this documentation update
    is very important in itself, so let's not worry about that.
    
    On Sat, Aug 19, 2017 at 7:46 AM, David Steele <david@pgmasters.net> wrote:
    > On 8/18/17 3:00 PM, Robert Haas wrote:
    >>
    >> If you update the patch I'll apply it to 11 and 10.
    >
    > Attached is the updated patch.
    >
    > I didn't like the vague "there can be some issues on the server if it
    > crashes during the backup" so I added a new paragraph at the appropriate
    > step to give a more detailed explanation of the problem.
    
    Thanks for the patch.
    
    -     This terminates the backup mode and performs an automatic switch to
    -     the next WAL segment.  The reason for the switch is to arrange for
    +     This terminates backup mode. On a primary, it also performs an automatic
    +     switch to the next WAL segment.  On a standby, it is not possible to
    +     automatically switch WAL segments, so you may wish to run
    +     <function>pg_switch_wal</function> on the primary to perform a manual
    +     switch. The reason for the switch is to arrange for
          the last WAL segment file written during the backup interval to be
          ready to archive.
    [...]
    -     backup files.  If <varname>archive_mode</> is enabled,
    +     backup files.  On a primary, if <varname>archive_mode</> is
    enabled and the
    +     <literal>wait_for_archive</> parameter is <literal>true</>,
          <function>pg_stop_backup</> does not return until the last segment has
          been archived.
    +     On a standby, <varname>archive_mode</> must be <literal>always</> in order
    +     for <function>pg_stop_backup</> to wait.
    
    This level of details is good to have. Thanks.
    
    +     Prior to PostgreSQL 9.6, this
    Markup <productname>?
    
    +      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.
    s/Note well/Note as well/, no?
    
    -     This function, when called on a primary, terminates the backup mode and
    +     This function terminates backup mode and
          performs an automatic switch to the next WAL segment. The reason for the
          switch is to arrange for the last WAL segment written during the backup
    -     interval to be ready to archive.  When called on a standby, this function
    -     only terminates backup mode.  A subsequent WAL segment switch will be
    -     needed in order to ensure that all WAL files needed to restore the backup
    -     can be archived; if the primary does not have sufficient write activity
    -     to trigger one, <function>pg_switch_wal</function> should be executed on
    -     the primary.
    +     interval to be ready to archive.
    pg_stop_backup() still waits for the last WAL segment to be archived
    on the primary. Perhaps we'd want to mention that?
    
    Documentation does not state yet that the use of low-level APIs for
    exclusive backups are not supported on standbys.
    
    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". 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!
    -- 
    Michael
    
    
    
  7. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-24T13:49:04Z

    Hi Michael,
    
    Thanks for reviewing!  Sorry for the late response, those eclipses don't
    just chase themselves...
    
    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.
    
    > -     This function, when called on a primary, terminates the backup mode and
    > +     This function terminates backup mode and
    >       performs an automatic switch to the next WAL segment. The reason for the
    >       switch is to arrange for the last WAL segment written during the backup
    > -     interval to be ready to archive.  When called on a standby, this function
    > -     only terminates backup mode.  A subsequent WAL segment switch will be
    > -     needed in order to ensure that all WAL files needed to restore the backup
    > -     can be archived; if the primary does not have sufficient write activity
    > -     to trigger one, <function>pg_switch_wal</function> should be executed on
    > -     the primary.
    > +     interval to be ready to archive.
    > pg_stop_backup() still waits for the last WAL segment to be archived
    > on the primary. Perhaps we'd want to mention that?
    
    That's detailed in the next paragraph, ISTM.
    
    > 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".
    
    > 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.
    
    Thanks!
    -- 
    -David
    david@pgmasters.net
    
  8. Re: Update low-level backup documentation to match actual behavior

    Michael Paquier <michael.paquier@gmail.com> — 2017-08-24T23:36:47Z

    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
    
    
    
  9. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-25T19:10:48Z

    On 8/24/17 7:36 PM, Michael Paquier wrote:
    > 
    > 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.
    
    Robert said he would commit this so I expect he'll do that if he doesn't
    have any objections to the changes.
    
    Robert, if you would prefer me to submit this to the CF I'm happy to do so.
    
    Thanks,
    -- 
    -David
    david@pgmasters.net
    
    
    
  10. Re: Update low-level backup documentation to match actual behavior

    Robert Haas <robertmhaas@gmail.com> — 2017-08-25T19:13:14Z

    On Fri, Aug 25, 2017 at 3:10 PM, David Steele <david@pgmasters.net> wrote:
    > On 8/24/17 7:36 PM, Michael Paquier wrote:
    >>
    >> 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.
    >
    > Robert said he would commit this so I expect he'll do that if he doesn't
    > have any objections to the changes.
    >
    > Robert, if you would prefer me to submit this to the CF I'm happy to do so.
    
    Ha, this note arrived just as I was working on getting this committed.
    I'll commit this to 11 and 10 presently; can you produce a version for
    9.6?
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
    
  11. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-25T19:21:16Z

    On 8/25/17 3:13 PM, Robert Haas wrote:
    > On Fri, Aug 25, 2017 at 3:10 PM, David Steele <david@pgmasters.net> wrote:
    >>
    >> Robert said he would commit this so I expect he'll do that if he doesn't
    >> have any objections to the changes.
    >>
    >> Robert, if you would prefer me to submit this to the CF I'm happy to do so.
    > 
    > Ha, this note arrived just as I was working on getting this committed.
    > I'll commit this to 11 and 10 presently; can you produce a version for
    > 9.6?
    
    No problem.  I'll base it on your commit to capture any changes you made.
    
    -- 
    -David
    david@pgmasters.net
    
    
    
  12. Re: Update low-level backup documentation to match actual behavior

    Robert Haas <robertmhaas@gmail.com> — 2017-08-25T19:26:05Z

    On Fri, Aug 25, 2017 at 3:21 PM, David Steele <david@pgmasters.net> wrote:
    > No problem.  I'll base it on your commit to capture any changes you made.
    
    Thanks, but you incorporated everything I wanted in response to my
    first review -- so I didn't tweak it any further.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
    
  13. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-25T20:03:31Z

    On 8/25/17 3:26 PM, Robert Haas wrote:
    > On Fri, Aug 25, 2017 at 3:21 PM, David Steele <david@pgmasters.net> wrote:
    >> No problem.  I'll base it on your commit to capture any changes you made.
    > 
    > Thanks, but you incorporated everything I wanted in response to my
    > first review -- so I didn't tweak it any further.
    
    Thank you for committing that.  I'll get the 9.6 patch to you early next 
    week.
    
    -- 
    -David
    david@pgmasters.net
    
    
    
  14. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-29T13:59:46Z

    Hi Robert,
    
    On 8/25/17 4:03 PM, David Steele wrote:
    > On 8/25/17 3:26 PM, Robert Haas wrote:
    >> On Fri, Aug 25, 2017 at 3:21 PM, David Steele <david@pgmasters.net>
    >> wrote:
    >>> No problem.  I'll base it on your commit to capture any changes you
    >>> made.
    >>
    >> Thanks, but you incorporated everything I wanted in response to my
    >> first review -- so I didn't tweak it any further.
    > 
    > Thank you for committing that.  I'll get the 9.6 patch to you early next
    > week.
    
    Attached is the 9.6 patch.  It required a bit more work in func.sgml
    than I was expecting so have a close look at that.  The rest was mostly
    removing irrelevant hunks.
    
    Thanks,
    -- 
    -David
    david@pgmasters.net
    
  15. Re: Update low-level backup documentation to match actual behavior

    Michael Paquier <michael.paquier@gmail.com> — 2017-08-30T01:44:54Z

    On Tue, Aug 29, 2017 at 10:59 PM, David Steele <david@pgmasters.net> wrote:
    > Hi Robert,
    >
    > On 8/25/17 4:03 PM, David Steele wrote:
    >> On 8/25/17 3:26 PM, Robert Haas wrote:
    >>> On Fri, Aug 25, 2017 at 3:21 PM, David Steele <david@pgmasters.net>
    >>> wrote:
    >>>> No problem.  I'll base it on your commit to capture any changes you
    >>>> made.
    >>>
    >>> Thanks, but you incorporated everything I wanted in response to my
    >>> first review -- so I didn't tweak it any further.
    >>
    >> Thank you for committing that.  I'll get the 9.6 patch to you early next
    >> week.
    >
    > Attached is the 9.6 patch.  It required a bit more work in func.sgml
    > than I was expecting so have a close look at that.  The rest was mostly
    > removing irrelevant hunks.
    
    +     switch to the next WAL segment.  On a standby, it is not possible to
    +     automatically switch WAL segments, so you may wish to run
    +     <function>pg_switch_wal</function> on the primary to perform a manual
    +     switch. The reason for the switch is to arrange for
    [...]
    +    WAL segments have been archived. If write activity on the primary
    is low, it
    +    may be useful to run <function>pg_switch_wal</> on the primary in order to
    +    trigger an immediate segment switch of the last required WAL
    It seems to me that both portions are wrong. There is no archiving
    wait on standbys for 9.6, and pg_stop_backup triggers by itself the
    segment switch, so saying that enforcing pg_switch_wal on the primary
    is moot. pg_switch_xlog has been renamed to pg_switch_wal in PG10, so
    the former name applies.
    -- 
    Michael
    
    
    
  16. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-30T11:02:35Z

    Hi Michael,
    
    Thanks for reviewing!
    
    On 8/29/17 9:44 PM, Michael Paquier wrote:
    > On Tue, Aug 29, 2017 at 10:59 PM, David Steele <david@pgmasters.net> wrote:
    >>
    >> Attached is the 9.6 patch.  It required a bit more work in func.sgml
    >> than I was expecting so have a close look at that.  The rest was mostly
    >> removing irrelevant hunks.
    > 
    > +     switch to the next WAL segment.  On a standby, it is not possible to
    > +     automatically switch WAL segments, so you may wish to run
    > +     <function>pg_switch_wal</function> on the primary to perform a manual
    > +     switch. The reason for the switch is to arrange for
    > [...]
    > +    WAL segments have been archived. If write activity on the primary
    > is low, it
    > +    may be useful to run <function>pg_switch_wal</> on the primary in order to
    > +    trigger an immediate segment switch of the last required WAL
    > It seems to me that both portions are wrong. There is no archiving
    > wait on standbys for 9.6, and 
    I think its clearly stated here that pg_stop_backup() does not wait for
    WAL to archive on a standby.  Even, it is very important for the backup
    routine to make sure that all the WAL *is* archived.
    
    > pg_stop_backup triggers by itself the
    > segment switch, so saying that enforcing pg_switch_wal on the primary
    > is moot. 
    
    pg_stop_backup() does not perform a WAL switch on the standby which is
    what this sentence is referring to.  I have separated this section out
    into a new paragraph to (hopefully) make it clearer.
    
    > pg_switch_xlog has been renamed to pg_switch_wal in PG10, so
    > the former name applies.
    
    Whoops!
    
    New patch is attached.
    
    Thanks,
    -- 
    -David
    david@pgmasters.net
    
  17. Re: Update low-level backup documentation to match actual behavior

    Michael Paquier <michael.paquier@gmail.com> — 2017-08-31T00:37:08Z

    On Wed, Aug 30, 2017 at 8:02 PM, David Steele <david@pgmasters.net> wrote:
    > On 8/29/17 9:44 PM, Michael Paquier wrote:
    >> On Tue, Aug 29, 2017 at 10:59 PM, David Steele <david@pgmasters.net> wrote:
    >>>
    >>> Attached is the 9.6 patch.  It required a bit more work in func.sgml
    >>> than I was expecting so have a close look at that.  The rest was mostly
    >>> removing irrelevant hunks.
    >>
    >> +     switch to the next WAL segment.  On a standby, it is not possible to
    >> +     automatically switch WAL segments, so you may wish to run
    >> +     <function>pg_switch_wal</function> on the primary to perform a manual
    >> +     switch. The reason for the switch is to arrange for
    >> [...]
    >> +    WAL segments have been archived. If write activity on the primary
    >> is low, it
    >> +    may be useful to run <function>pg_switch_wal</> on the primary in order to
    >> +    trigger an immediate segment switch of the last required WAL
    >> It seems to me that both portions are wrong. There is no archiving
    >> wait on standbys for 9.6, and
    > I think its clearly stated here that pg_stop_backup() does not wait for
    > WAL to archive on a standby.  Even, it is very important for the backup
    > routine to make sure that all the WAL *is* archived.
    
    Yes, it seems that I somewhat missed the "on the primary portion"
    during the first read of the patch.
    
    >> pg_stop_backup triggers by itself the
    >> segment switch, so saying that enforcing pg_switch_wal on the primary
    >> is moot.
    >
    > pg_stop_backup() does not perform a WAL switch on the standby which is
    > what this sentence is referring to.  I have separated this section out
    > into a new paragraph to (hopefully) make it clearer.
    >
    >> pg_switch_xlog has been renamed to pg_switch_wal in PG10, so
    >> the former name applies.
    >
    > Whoops!
    >
    > New patch is attached.
    
    Thanks for the new version. This looks fine to me.
    -- 
    Michael
    
    
    
  18. Re: Update low-level backup documentation to match actual behavior

    Robert Haas <robertmhaas@gmail.com> — 2017-08-31T20:04:45Z

    On Wed, Aug 30, 2017 at 8:37 PM, Michael Paquier
    <michael.paquier@gmail.com> wrote:
    > Thanks for the new version. This looks fine to me.
    
    Committed to REL9_6_STABLE with minor wordsmithing.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
    
  19. Re: Update low-level backup documentation to match actual behavior

    David Steele <david@pgmasters.net> — 2017-08-31T20:11:18Z

    On 8/31/17 4:04 PM, Robert Haas wrote:
    > On Wed, Aug 30, 2017 at 8:37 PM, Michael Paquier
    > <michael.paquier@gmail.com> wrote:
    >> Thanks for the new version. This looks fine to me.
    > 
    > Committed to REL9_6_STABLE with minor wordsmithing.
    
    The edits look good to me. Thanks, Robert!
    
    -- 
    -David
    david@pgmasters.net
    
    
    
  20. Re: Update low-level backup documentation to match actual behavior

    Michael Paquier <michael.paquier@gmail.com> — 2017-09-01T01:30:20Z

    On Fri, Sep 1, 2017 at 5:11 AM, David Steele <david@pgmasters.net> wrote:
    > On 8/31/17 4:04 PM, Robert Haas wrote:
    >> On Wed, Aug 30, 2017 at 8:37 PM, Michael Paquier
    >> <michael.paquier@gmail.com> wrote:
    >>> Thanks for the new version. This looks fine to me.
    >>
    >> Committed to REL9_6_STABLE with minor wordsmithing.
    >
    > The edits look good to me. Thanks, Robert!
    
    Thanks David for the patch, and Robert for the commit! The final
    result is nicely shaped.
    -- 
    Michael