Re: [HACKERS] Creating backup history files for backups taken from standbys

[Michael Paquier]

On Fri, Feb 02, 2018 at 12:47:26AM +0900, Fujii Masao wrote:
> The patch basically looks good to me. Here are some small comments.
> 
>    <para>
>       The backup history file is not created in the database cluster backed 
> up.
>    </para>
> 
> The above should be deleted in pg_basebackup.sgml.
> 
>     * During recovery, since we don't use the end-of-backup WAL
>     * record and don't write the backup history file, the
> 
> This comment needs to be updated in xlog.c.

Thanks Fujii-san for the review.  Indeed those portions need a refresh.. 
--
Michael

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 487c7ff750..5dcee53dd2 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -18657,9 +18657,9 @@ postgres=# select pg_start_backup('label_goes_here');
    </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
+    The function also creates a backup history file in the write-ahead log
+    archive area when archiving is enabled.
+    The history file includes the label given to
     <function>pg_start_backup</function>, the starting and ending write-ahead log locations for
     the backup, and the starting and ending times of the backup.  The return
     value is the backup's ending write-ahead log location (which again
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index e8921b1bb4..1c37cecf72 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -78,11 +78,6 @@ PostgreSQL documentation
    Note that there are some limitations in an online backup from the standby:
 
    <itemizedlist>
-    <listitem>
-     <para>
-      The backup history file is not created in the database cluster backed up.
-     </para>
-    </listitem>
     <listitem>
      <para>
       If you are using <literal>-X none</literal>, there is no guarantee that all
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e42b828edf..f2ec7441f3 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -10389,10 +10389,10 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 
 				/*
 				 * During recovery, since we don't use the end-of-backup WAL
-				 * record and don't write the backup history file, the
-				 * starting WAL location doesn't need to be unique. This means
-				 * that two base backups started at the same time might use
-				 * the same checkpoint as starting locations.
+				 * record, the starting WAL location doesn't need to be unique.
+				 * This means that two base backups started at the same time
+				 * might use the same checkpoint as starting locations, and
+				 * write a backup history file with the same name.
 				 */
 				gotUniqueStartpoint = true;
 			}
@@ -10730,11 +10730,7 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 	XLogRecPtr	startpoint;
 	XLogRecPtr	stoppoint;
 	TimeLineID	stoptli;
-	pg_time_t	stamp_time;
-	char		strfbuf[128];
-	char		histfilepath[MAXPGPATH];
 	char		startxlogfilename[MAXFNAMELEN];
-	char		stopxlogfilename[MAXFNAMELEN];
 	char		lastxlogfilename[MAXFNAMELEN];
 	char		histfilename[MAXFNAMELEN];
 	char		backupfrom[20];
@@ -10940,12 +10936,6 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 	 * location. Note that it can be greater than the exact backup end
 	 * location if the minimum recovery point is updated after the backup of
 	 * pg_control. This is harmless for current uses.
-	 *
-	 * XXX currently a backup history file is for informational and debug
-	 * purposes only. It's not essential for an online backup. Furthermore,
-	 * even if it's created, it will not be archived during recovery because
-	 * an archiver is not invoked. So it doesn't seem worthwhile to write a
-	 * backup history file during recovery.
 	 */
 	if (backup_started_in_recovery)
 	{
@@ -10990,9 +10980,21 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 		 * valid as soon as archiver moves out the current segment file.
 		 */
 		RequestXLogSwitch(false);
+	}
 
-		XLByteToPrevSeg(stoppoint, _logSegNo, wal_segment_size);
-		XLogFileName(stopxlogfilename, stoptli, _logSegNo, wal_segment_size);
+	/*
+	 * Write the backup history file if archiving is enabled.
+	 *
+	 * XXX currently a backup history file is for informational and debug
+	 * purposes only. It's not essential for an online backup.
+	 */
+	if ((!backup_started_in_recovery && XLogArchivingActive()) ||
+		(backup_started_in_recovery && XLogArchivingAlways()))
+	{
+		char		strfbuf[128];
+		char		histfilepath[MAXPGPATH];
+		char		stopxlogfilename[MAXFNAMELEN];
+		pg_time_t	stamp_time;
 
 		/* Use the log timezone here, not the session timezone */
 		stamp_time = (pg_time_t) time(NULL);
@@ -11000,9 +11002,8 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 					"%Y-%m-%d %H:%M:%S %Z",
 					pg_localtime(&stamp_time, log_timezone));
 
-		/*
-		 * Write the backup history file
-		 */
+		XLByteToPrevSeg(stoppoint, _logSegNo, wal_segment_size);
+		XLogFileName(stopxlogfilename, stoptli, _logSegNo, wal_segment_size);
 		XLByteToSeg(startpoint, _logSegNo, wal_segment_size);
 		BackupHistoryFilePath(histfilepath, stoptli, _logSegNo,
 							  startpoint, wal_segment_size);

signature.asc
Description: PGP signature