Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Michael Paquier]

On Fri, Sep 1, 2017 at 5:11 AM, David Steele  wrote:
> On 8/31/17 4:04 PM, Robert Haas wrote:
>> On Wed, Aug 30, 2017 at 8:37 PM, Michael Paquier
>>  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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

On 8/31/17 4:04 PM, Robert Haas wrote:
> On Wed, Aug 30, 2017 at 8:37 PM, Michael Paquier
>  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
da...@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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Robert Haas]

On Wed, Aug 30, 2017 at 8:37 PM, Michael Paquier
 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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Michael Paquier]

On Wed, Aug 30, 2017 at 8:02 PM, David Steele  wrote:
> On 8/29/17 9:44 PM, Michael Paquier wrote:
>> On Tue, Aug 29, 2017 at 10:59 PM, David Steele  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
>> + pg_switch_wal 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 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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

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  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
> + pg_switch_wal 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 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
da...@pgmasters.net
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);
 
 SELECT * FROM pg_stop_backup(false);
 
- 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
+ pg_switch_xlog 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.
 
@@ -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 pg_stop_backup's first return
  value is the last segment that is required to form a complete set of
- backup files.  If archive_mode is enabled,
+ backup files.  On a primary, if archive_mode is enabled,
  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);
  pg_stop_backup terminates because of this your backup
  may not be valid.
 
+
+
+ Note that on a standby 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.
+
+

   
 
@@ -932,9 +942,9 @@ SELECT * FROM pg_stop_backup(false);
 Making an exclusive low level backup
 
  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.
 
@@ -992,6 +1002,11 @@ SELECT pg_start_backup('label', true);
   for things to
  consider during this backup.
 
+
+  Note that if the server crashes during the backup it may not be
+  possible to restart until the backup_label file has been
+  manually deleted from the PGDATA directory.
+


 
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');
 pg_start_backup. In a non-exclusive backup, the contents of
 the backup_label and 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 

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Michael Paquier]

On Tue, Aug 29, 2017 at 10:59 PM, David Steele  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 
>>> 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
+ pg_switch_wal 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 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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

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 
>> 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
da...@pgmasters.net
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);
 
 SELECT * FROM pg_stop_backup(false);
 
- 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
+ pg_switch_wal 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.
 
@@ -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 pg_stop_backup's first return
  value is the last segment that is required to form a complete set of
- backup files.  If archive_mode is enabled,
+ backup files.  On a primary, if archive_mode is enabled,
  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);
  pg_stop_backup terminates because of this your backup
  may not be valid.
 
+
+
+ Note that on a standby 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.
+
+

   
 
@@ -932,9 +942,9 @@ SELECT * FROM pg_stop_backup(false);
 Making an exclusive low level backup
 
  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.
 
@@ -992,6 +1002,11 @@ SELECT pg_start_backup('label', true);
   for things to
  consider during this backup.
 
+
+  Note that if the server crashes during the backup it may not be
+  possible to restart until the backup_label file has been
+  manually deleted from the PGDATA directory.
+


 
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');
 pg_start_backup. In a non-exclusive backup, the contents of
 the backup_label and 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
+pg_stop_backup will wait for WAL to be archived when archiving
+is enabled.  On a standby 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 pg_switch_wal on the primary in order to
+trigger an immediate segment switch of the last required WAL.

 

-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
 pg_start_backup, the starting and ending transaction log 
locations for
 the backup, and the starting and ending times of the backup.  The return

-- 
Sent via pgsql-hackers mailing list 

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

On 8/25/17 3:26 PM, Robert Haas wrote:

On Fri, Aug 25, 2017 at 3:21 PM, David Steele  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
da...@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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Robert Haas]

On Fri, Aug 25, 2017 at 3:21 PM, David Steele  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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

On 8/25/17 3:13 PM, Robert Haas wrote:
> On Fri, Aug 25, 2017 at 3:10 PM, David Steele  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
da...@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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Robert Haas]

On Fri, Aug 25, 2017 at 3:10 PM, David Steele  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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

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
da...@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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Michael Paquier]

On Thu, Aug 24, 2017 at 10:49 PM, David Steele  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  wrote:
>>
>> + Prior to PostgreSQL 9.6, this
>> Markup ?
>
> Fixed.
>
>> +  Note well that if the server crashes during the backup it may not be
>> +  possible to restart until the backup_label file has been
>> +  manually deleted from the PGDATA directory.
>> Missing a markup  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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

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  wrote:
> 
> + Prior to PostgreSQL 9.6, this
> Markup ?

Fixed.

> +  Note well that if the server crashes during the backup it may not be
> +  possible to restart until the backup_label file has been
> +  manually deleted from the PGDATA directory.
> Missing a markup  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, pg_switch_wal 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
da...@pgmasters.net
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);
 
 SELECT * FROM pg_stop_backup(false, true);
 
- 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
+ pg_switch_wal 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.
 
@@ -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 pg_stop_backup's first return
  value is the last segment that is required to form a complete set of
- backup files.  If archive_mode is enabled,
+ backup files.  On a primary, if archive_mode is enabled and 
the
+ wait_for_archive parameter is true,
  pg_stop_backup does not return until the last segment has
  been archived.
+ On a standby, archive_mode must be always in order
+ for pg_stop_backup to wait.
  Archiving of these files happens automatically since you have
  already configured 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);
 
 
  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
+ wait_for_archive parameter (which defaults to true) can be set
+ to false to have
  pg_stop_backup return as soon as the stop backup record is
  written to the WAL.  By default, 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);
 Making an exclusive low level backup
 
  The process for an exclusive 

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Michael Paquier]

On Fri, Aug 18, 2017 at 3:35 AM, David Steele  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  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
+ pg_switch_wal 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 archive_mode is enabled,
+ backup files.  On a primary, if archive_mode is
enabled and the
+ wait_for_archive parameter is true,
  pg_stop_backup does not return until the last segment has
  been archived.
+ On a standby, archive_mode must be always in order
+ for pg_stop_backup to wait.

This level of details is good to have. Thanks.

+ Prior to PostgreSQL 9.6, this
Markup ?

+  Note well that if the server crashes during the backup it may not be
+  possible to restart until the backup_label file has been
+  manually deleted from the PGDATA directory.
Missing a markup  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, pg_switch_wal 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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

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
da...@pgmasters.net
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);
 
 SELECT * FROM pg_stop_backup(false, true);
 
- 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
+ pg_switch_wal 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.
 
@@ -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 pg_stop_backup's first return
  value is the last segment that is required to form a complete set of
- backup files.  If archive_mode is enabled,
+ backup files.  On a primary, if archive_mode is enabled and 
the
+ wait_for_archive parameter is true,
  pg_stop_backup does not return until the last segment has
  been archived.
+ On a standby, archive_mode must be always in order
+ for pg_stop_backup to wait.
  Archiving of these files happens automatically since you have
  already configured 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);
 Making an exclusive low level backup
 
  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.
 
@@ -1003,6 +1009,11 @@ SELECT pg_start_backup('label', true);
   for things to
  consider during this backup.
 
+
+  Note well that if the server crashes during the backup it may not be
+  possible to restart until the backup_label file has been
+  manually deleted from the PGDATA directory.
+


 
@@ -1012,15 +1023,10 @@ SELECT pg_start_backup('label', true);
 
 SELECT pg_stop_backup();
 
- 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, pg_switch_wal should be executed on
- the primary.
+ interval to be ready to archive.
 


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');

 

-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
 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

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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Robert Haas]

On Fri, Aug 18, 2017 at 2:58 PM, David Steele  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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

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
> + pg_switch_wal 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 pg_switch_wal on the primary to
> perform a manual switch.

Looks good.

> 
> -Making an exclusive low level backup
> +Making an exclusive low level backup on a primary
> 
> 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, pg_switch_wal 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
da...@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

Re: [HACKERS] Update low-level backup documentation to match actual behavior

[Robert Haas]

- 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
+ pg_switch_wal 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 pg_switch_wal on the primary to
perform a manual switch.

- backup files.  If archive_mode is enabled,
+ backup files.  On a primary, if archive_mode is
enabled and the
+ wait_for_archive parameter is true,
  pg_stop_backup does not return until the last segment has
  been archived.
+ On a standby, archive_mode must be always in order
+ for pg_stop_backup to wait.

Looks good.

-Making an exclusive low level backup
+Making an exclusive low level backup on a primary

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, pg_switch_wal 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

[HACKERS] Update low-level backup documentation to match actual behavior

[David Steele]

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
da...@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
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);
 
  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
+ pg_switch_wal 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.
 
@@ -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 pg_stop_backup's first return
  value is the last segment that is required to form a complete set of
- backup files.  If archive_mode is enabled,
+ backup files.  On a primary, if archive_mode is enabled and 
the
+ wait_for_archive parameter is true,
  pg_stop_backup does not return until the last segment has
  been archived.
+ On a standby, archive_mode must be always in order
+ for pg_stop_backup to wait.
  Archiving of these files happens automatically since you have
  already configured 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);
 


-Making an exclusive low level backup
+Making an exclusive low level backup on a primary
 
  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);
 
 SELECT pg_stop_backup();
 
- 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, pg_switch_wal should be executed on
- the primary.
+ interval to be ready to archive.
 


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');

 

-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
 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

-- 
Sent via pgsql-hackers mailing