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