Update low-level backup documentation to match actual behavior

Started by David Steeleover 8 years ago20 messages
#1David Steele
david@pgmasters.net
1 attachment(s)

As discussed in [1]/messages/by-id/20170814152816.GF4628@tamriel.snowman.net 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]/messages/by-id/CAB7nPqQvVpMsqJExSVXHUwpXFRwojsb-jb4BYnxEQbjJLfw-yQ@mail.gmail.com 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]: /messages/by-id/20170814152816.GF4628@tamriel.snowman.net
/messages/by-id/20170814152816.GF4628@tamriel.snowman.net
[2]: /messages/by-id/CAB7nPqQvVpMsqJExSVXHUwpXFRwojsb-jb4BYnxEQbjJLfw-yQ@mail.gmail.com
/messages/by-id/CAB7nPqQvVpMsqJExSVXHUwpXFRwojsb-jb4BYnxEQbjJLfw-yQ@mail.gmail.com

Attachments:

low-level-backup-docs-v1.patchtext/plain; charset=UTF-8; name=low-level-backup-docs-v1.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 0e7c6e2051..cfffea3919 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -890,7 +890,10 @@ SELECT pg_start_backup('label', false, false);
 SELECT * FROM pg_stop_backup(false, true);
 </programlisting>
      This terminates the backup mode and performs an automatic switch to
-     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
      the last WAL segment file written during the backup interval to be
      ready to archive.
     </para>
@@ -908,9 +911,12 @@ SELECT * FROM pg_stop_backup(false, true);
      Once the WAL segment files active during the backup are archived, you are
      done.  The file identified by <function>pg_stop_backup</>'s first return
      value is the last segment that is required to form a complete set of
-     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.
      Archiving of these files happens automatically since you have
      already configured <varname>archive_command</>. In most cases this
      happens quickly, but you are advised to monitor your archive
@@ -940,11 +946,12 @@ SELECT * FROM pg_stop_backup(false, true);
     </para>
    </sect3>
    <sect3 id="backup-lowlevel-base-backup-exclusive">
-    <title>Making an exclusive low level backup</title>
+    <title>Making an exclusive low level backup on a primary</title>
     <para>
      The process for an exclusive backup is mostly the same as for a
      non-exclusive one, but it differs in a few key steps. It does not allow
-     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
      the server if it crashes during the backup. Prior to PostgreSQL 9.6, this
      was the only low-level method available, but it is now recommended that
      all users upgrade their scripts to use non-exclusive backups if possible.
@@ -1012,15 +1019,10 @@ SELECT pg_start_backup('label', true);
 <programlisting>
 SELECT pg_stop_backup();
 </programlisting>
-     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.
     </para>
    </listitem>
    <listitem>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index b43ec30a4e..28eda97273 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18606,7 +18606,8 @@ postgres=# select pg_start_backup('label_goes_here');
    </para>
 
    <para>
-    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
     archive area. The history file includes the label given to
     <function>pg_start_backup</>, the starting and ending write-ahead log locations for
     the backup, and the starting and ending times of the backup.  The return
#2Robert Haas
robertmhaas@gmail.com
In reply to: David Steele (#1)
Re: Update low-level backup documentation to match actual behavior
-     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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3David Steele
david@pgmasters.net
In reply to: Robert Haas (#2)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Robert Haas
robertmhaas@gmail.com
In reply to: David Steele (#3)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5David Steele
david@pgmasters.net
In reply to: Robert Haas (#4)
1 attachment(s)
Re: Update low-level backup documentation to match actual behavior

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

Attachments:

low-level-backup-docs-v2.patchtext/plain; charset=UTF-8; name=low-level-backup-docs-v2.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 0e7c6e2051..76f81975f1 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -889,8 +889,11 @@ SELECT pg_start_backup('label', false, false);
 <programlisting>
 SELECT * FROM pg_stop_backup(false, true);
 </programlisting>
-     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.
     </para>
@@ -908,9 +911,12 @@ SELECT * FROM pg_stop_backup(false, true);
      Once the WAL segment files active during the backup are archived, you are
      done.  The file identified by <function>pg_stop_backup</>'s first return
      value is the last segment that is required to form a complete set of
-     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.
      Archiving of these files happens automatically since you have
      already configured <varname>archive_command</>. In most cases this
      happens quickly, but you are advised to monitor your archive
@@ -943,9 +949,9 @@ SELECT * FROM pg_stop_backup(false, true);
     <title>Making an exclusive low level backup</title>
     <para>
      The process for an exclusive backup is mostly the same as for a
-     non-exclusive one, but it differs in a few key steps. It does not allow
-     more than one concurrent backup to run, and there can be some issues on
-     the server if it crashes during the backup. Prior to PostgreSQL 9.6, this
+     non-exclusive one, but it differs in a few key steps. This type of backup
+     can only be taken on a primary and does not allow concurrent backups.
+     Prior to PostgreSQL 9.6, this
      was the only low-level method available, but it is now recommended that
      all users upgrade their scripts to use non-exclusive backups if possible.
     </para>
@@ -1003,6 +1009,11 @@ SELECT pg_start_backup('label', true);
      <xref linkend="backup-lowlevel-base-backup-data"> for things to
      consider during this backup.
     </para>
+    <para>
+      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.
+    </para>
    </listitem>
    <listitem>
     <para>
@@ -1012,15 +1023,10 @@ SELECT pg_start_backup('label', true);
 <programlisting>
 SELECT pg_stop_backup();
 </programlisting>
-     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.
     </para>
    </listitem>
    <listitem>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index b43ec30a4e..28eda97273 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18606,7 +18606,8 @@ postgres=# select pg_start_backup('label_goes_here');
    </para>
 
    <para>
-    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
     archive area. The history file includes the label given to
     <function>pg_start_backup</>, the starting and ending write-ahead log locations for
     the backup, and the starting and ending times of the backup.  The return
#6Michael Paquier
michael.paquier@gmail.com
In reply to: David Steele (#5)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7David Steele
david@pgmasters.net
In reply to: Michael Paquier (#6)
1 attachment(s)
Re: Update low-level backup documentation to match actual behavior

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

Attachments:

low-level-backup-docs-v3.patchtext/plain; charset=UTF-8; name=low-level-backup-docs-v3.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 0e7c6e2051..95aeb35507 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -889,8 +889,11 @@ SELECT pg_start_backup('label', false, false);
 <programlisting>
 SELECT * FROM pg_stop_backup(false, true);
 </programlisting>
-     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.
     </para>
@@ -908,9 +911,12 @@ SELECT * FROM pg_stop_backup(false, true);
      Once the WAL segment files active during the backup are archived, you are
      done.  The file identified by <function>pg_stop_backup</>'s first return
      value is the last segment that is required to form a complete set of
-     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.
      Archiving of these files happens automatically since you have
      already configured <varname>archive_command</>. In most cases this
      happens quickly, but you are advised to monitor your archive
@@ -926,8 +932,9 @@ SELECT * FROM pg_stop_backup(false, true);
     </para>
     <para>
      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
+     required for the backup are successfully archived then the
+     <literal>wait_for_archive</> parameter (which defaults to true) can be set
+     to false to have
      <function>pg_stop_backup</> return as soon as the stop backup record is
      written to the WAL.  By default, <function>pg_stop_backup</> will wait
      until all WAL has been archived, which can take some time.  This option
@@ -943,9 +950,9 @@ SELECT * FROM pg_stop_backup(false, true);
     <title>Making an exclusive low level backup</title>
     <para>
      The process for an exclusive backup is mostly the same as for a
-     non-exclusive one, but it differs in a few key steps. It does not allow
-     more than one concurrent backup to run, and there can be some issues on
-     the server if it crashes during the backup. Prior to PostgreSQL 9.6, this
+     non-exclusive one, but it differs in a few key steps. This type of backup
+     can only be taken on a primary and does not allow concurrent backups.
+     Prior to <productname>PostgreSQL</> 9.6, this
      was the only low-level method available, but it is now recommended that
      all users upgrade their scripts to use non-exclusive backups if possible.
     </para>
@@ -1003,6 +1010,11 @@ SELECT pg_start_backup('label', true);
      <xref linkend="backup-lowlevel-base-backup-data"> for things to
      consider during this backup.
     </para>
+    <para>
+      Note 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 <envar>PGDATA</envar> directory.
+    </para>
    </listitem>
    <listitem>
     <para>
@@ -1012,15 +1024,10 @@ SELECT pg_start_backup('label', true);
 <programlisting>
 SELECT pg_stop_backup();
 </programlisting>
-     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.
     </para>
    </listitem>
    <listitem>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index b43ec30a4e..28eda97273 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18606,7 +18606,8 @@ postgres=# select pg_start_backup('label_goes_here');
    </para>
 
    <para>
-    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
     archive area. The history file includes the label given to
     <function>pg_start_backup</>, the starting and ending write-ahead log locations for
     the backup, and the starting and ending times of the backup.  The return
#8Michael Paquier
michael.paquier@gmail.com
In reply to: David Steele (#7)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9David Steele
david@pgmasters.net
In reply to: Michael Paquier (#8)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Robert Haas
robertmhaas@gmail.com
In reply to: David Steele (#9)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11David Steele
david@pgmasters.net
In reply to: Robert Haas (#10)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Robert Haas
robertmhaas@gmail.com
In reply to: David Steele (#11)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13David Steele
david@pgmasters.net
In reply to: Robert Haas (#12)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14David Steele
david@pgmasters.net
In reply to: David Steele (#13)
1 attachment(s)
Re: Update low-level backup documentation to match actual behavior

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

Attachments:

low-level-backup-docs-9.6-v1.patchtext/plain; charset=UTF-8; name=low-level-backup-docs-9.6-v1.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 03c0dbf1cd..456f6f2c98 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -889,8 +889,11 @@ SELECT pg_start_backup('label', false, false);
 <programlisting>
 SELECT * FROM pg_stop_backup(false);
 </programlisting>
-     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.
     </para>
@@ -908,7 +911,7 @@ SELECT * FROM pg_stop_backup(false);
      Once the WAL segment files active during the backup are archived, you are
      done.  The file identified by <function>pg_stop_backup</>'s first return
      value is the last segment that is required to form a complete set of
-     backup files.  If <varname>archive_mode</> is enabled,
+     backup files.  On a primary, if <varname>archive_mode</> is enabled,
      <function>pg_stop_backup</> does not return until the last segment has
      been archived.
      Archiving of these files happens automatically since you have
@@ -924,6 +927,13 @@ SELECT * FROM pg_stop_backup(false);
      <function>pg_stop_backup</> terminates because of this your backup
      may not be valid.
     </para>
+
+    <para>
+     Note that on a standby <function>pg_stop_backup</> does not wait for
+     WAL segments to be archived so the backup process must ensure that all WAL
+     segments required for the backup are successfully archived.
+    </para>
+
    </listitem>
   </orderedlist>
     </para>
@@ -932,9 +942,9 @@ SELECT * FROM pg_stop_backup(false);
     <title>Making an exclusive low level backup</title>
     <para>
      The process for an exclusive backup is mostly the same as for a
-     non-exclusive one, but it differs in a few key steps. It does not allow
-     more than one concurrent backup to run, and there can be some issues on
-     the server if it crashes during the backup. Prior to PostgreSQL 9.6, this
+     non-exclusive one, but it differs in a few key steps. This type of backup
+     can only be taken on a primary and does not allow concurrent backups.
+     Prior to <productname>PostgreSQL</> 9.6, this
      was the only low-level method available, but it is now recommended that
      all users upgrade their scripts to use non-exclusive backups if possible.
     </para>
@@ -992,6 +1002,11 @@ SELECT pg_start_backup('label', true);
      <xref linkend="backup-lowlevel-base-backup-data"> for things to
      consider during this backup.
     </para>
+    <para>
+      Note 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 <envar>PGDATA</envar> directory.
+    </para>
    </listitem>
    <listitem>
     <para>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8482601294..a803e747b2 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18070,11 +18070,18 @@ postgres=# select pg_start_backup('label_goes_here');
     <function>pg_start_backup</>. In a non-exclusive backup, the contents of
     the <filename>backup_label</> and <filename>tablespace_map</> are returned
     in the result of the function, and should be written to files in the
-    backup (and not in the data directory).
+    backup (and not in the data directory).  When executed on a primary
+    <function>pg_stop_backup</> will wait for WAL to be archived when archiving
+    is enabled.  On a standby <function>pg_stop_backup</> will return
+    immediately without waiting so it's important to verify that all required
+    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.
    </para>
 
    <para>
-    The function also creates a backup history file in the transaction log
+    When executed on a primary, the function also creates a backup history file
+    in the write-ahead log
     archive area. The history file includes the label given to
     <function>pg_start_backup</>, the starting and ending transaction log locations for
     the backup, and the starting and ending times of the backup.  The return
#15Michael Paquier
michael.paquier@gmail.com
In reply to: David Steele (#14)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16David Steele
david@pgmasters.net
In reply to: Michael Paquier (#15)
1 attachment(s)
Re: Update low-level backup documentation to match actual behavior

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

Attachments:

low-level-backup-docs-9.6-v2.patchtext/plain; charset=UTF-8; name=low-level-backup-docs-9.6-v2.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 03c0dbf1cd..fd696c38db 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -889,8 +889,11 @@ SELECT pg_start_backup('label', false, false);
 <programlisting>
 SELECT * FROM pg_stop_backup(false);
 </programlisting>
-     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_xlog</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.
     </para>
@@ -908,7 +911,7 @@ SELECT * FROM pg_stop_backup(false);
      Once the WAL segment files active during the backup are archived, you are
      done.  The file identified by <function>pg_stop_backup</>'s first return
      value is the last segment that is required to form a complete set of
-     backup files.  If <varname>archive_mode</> is enabled,
+     backup files.  On a primary, if <varname>archive_mode</> is enabled,
      <function>pg_stop_backup</> does not return until the last segment has
      been archived.
      Archiving of these files happens automatically since you have
@@ -924,6 +927,13 @@ SELECT * FROM pg_stop_backup(false);
      <function>pg_stop_backup</> terminates because of this your backup
      may not be valid.
     </para>
+
+    <para>
+     Note that on a standby <function>pg_stop_backup</> does not wait for
+     WAL segments to be archived so the backup process must ensure that all WAL
+     segments required for the backup are successfully archived.
+    </para>
+
    </listitem>
   </orderedlist>
     </para>
@@ -932,9 +942,9 @@ SELECT * FROM pg_stop_backup(false);
     <title>Making an exclusive low level backup</title>
     <para>
      The process for an exclusive backup is mostly the same as for a
-     non-exclusive one, but it differs in a few key steps. It does not allow
-     more than one concurrent backup to run, and there can be some issues on
-     the server if it crashes during the backup. Prior to PostgreSQL 9.6, this
+     non-exclusive one, but it differs in a few key steps. This type of backup
+     can only be taken on a primary and does not allow concurrent backups.
+     Prior to <productname>PostgreSQL</> 9.6, this
      was the only low-level method available, but it is now recommended that
      all users upgrade their scripts to use non-exclusive backups if possible.
     </para>
@@ -992,6 +1002,11 @@ SELECT pg_start_backup('label', true);
      <xref linkend="backup-lowlevel-base-backup-data"> for things to
      consider during this backup.
     </para>
+    <para>
+      Note 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 <envar>PGDATA</envar> directory.
+    </para>
    </listitem>
    <listitem>
     <para>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8482601294..01ebfb8d90 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18070,11 +18070,22 @@ postgres=# select pg_start_backup('label_goes_here');
     <function>pg_start_backup</>. In a non-exclusive backup, the contents of
     the <filename>backup_label</> and <filename>tablespace_map</> are returned
     in the result of the function, and should be written to files in the
-    backup (and not in the data directory).
+    backup (and not in the data directory).  When executed on a primary
+    <function>pg_stop_backup</> will wait for WAL to be archived when archiving
+    is enabled.
    </para>
 
    <para>
-    The function also creates a backup history file in the transaction log
+    On a standby <function>pg_stop_backup</> will return immediately without
+    waiting so it's important to verify that all required WAL segments have been
+    archived. If write activity on the primary is low, it may be useful to run
+    <function>pg_switch_xlog</> on the primary in order to trigger an immediate
+    segment switch of the last required WAL.
+   </para>
+
+   <para>
+    When executed on a primary, the function also creates a backup history file
+    in the write-ahead log
     archive area. The history file includes the label given to
     <function>pg_start_backup</>, the starting and ending transaction log locations for
     the backup, and the starting and ending times of the backup.  The return
#17Michael Paquier
michael.paquier@gmail.com
In reply to: David Steele (#16)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#17)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#19David Steele
david@pgmasters.net
In reply to: Robert Haas (#18)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#20Michael Paquier
michael.paquier@gmail.com
In reply to: David Steele (#19)
Re: Update low-level backup documentation to match actual behavior

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers