Fujii Masao wrote:
> On Thu, Dec 24, 2009 at 1:39 PM, Fujii Masao <masao.fu...@gmail.com> wrote:
>> On Wed, Dec 23, 2009 at 7:50 PM, Heikki Linnakangas
>> <heikki.linnakan...@enterprisedb.com> wrote:
>>> Ok. How about writing the history file in pg_stop_backup() for
>>> informational purposes only. Ie. never read it, but rely on the WAL
>>> records instead.
>> Sounds good. I'll make such change as a self-contained patch.
> 
> Done. Please see the attached patch.
> 
> Design:
> 
> * pg_stop_backup writes the backup-end xlog record which contains
>   the backup starting point.
> 
> * In archive recovery, the startup process doesn't mark the database
>   as consistent until it has read the backup-end record.
> 
> * A backup history file is still created as in the past, but is never
>   used.

As the patch stands, reachedBackupEnd is never set to true if starting
from a restore point after the end-of-backup. We'll need to store the
information that we've reached end-of-backup somewhere on disk.

Here's is modified patch that adds a new backupStartPoint field to
pg_control for that + some other minor editorialization.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 783725c..63884eb 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -515,8 +515,7 @@ static void xlog_outrec(StringInfo buf, XLogRecord *record);
 #endif
 static void issue_xlog_fsync(void);
 static void pg_start_backup_callback(int code, Datum arg);
-static bool read_backup_label(XLogRecPtr *checkPointLoc,
-				  XLogRecPtr *minRecoveryLoc);
+static bool read_backup_label(XLogRecPtr *checkPointLoc);
 static void rm_redo_error_callback(void *arg);
 static int	get_sync_bit(int method);
 
@@ -5355,7 +5354,6 @@ StartupXLOG(void)
 	bool		haveBackupLabel = false;
 	XLogRecPtr	RecPtr,
 				checkPointLoc,
-				backupStopLoc,
 				EndOfLog;
 	uint32		endLogId;
 	uint32		endLogSeg;
@@ -5454,7 +5452,7 @@ StartupXLOG(void)
 						recoveryTargetTLI,
 						ControlFile->checkPointCopy.ThisTimeLineID)));
 
-	if (read_backup_label(&checkPointLoc, &backupStopLoc))
+	if (read_backup_label(&checkPointLoc))
 	{
 		/*
 		 * When a backup_label file is present, we want to roll forward from
@@ -5597,11 +5595,23 @@ StartupXLOG(void)
 		ControlFile->prevCheckPoint = ControlFile->checkPoint;
 		ControlFile->checkPoint = checkPointLoc;
 		ControlFile->checkPointCopy = checkPoint;
-		if (backupStopLoc.xlogid != 0 || backupStopLoc.xrecoff != 0)
+		if (InArchiveRecovery)
+		{
+			/* initialize minRecoveryPoint if not set yet */
+			if (XLByteLT(ControlFile->minRecoveryPoint, checkPoint.redo))
+				ControlFile->minRecoveryPoint = checkPoint.redo;
+		}
+		else
 		{
-			if (XLByteLT(ControlFile->minRecoveryPoint, backupStopLoc))
-				ControlFile->minRecoveryPoint = backupStopLoc;
+			XLogRecPtr	InvalidXLogRecPtr = {0, 0};
+			ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
 		}
+		/*
+		 * set backupStartupPoint if we're starting archive recovery from a
+		 * base backup.
+		 */
+		if (haveBackupLabel)
+			ControlFile->backupStartPoint = checkPoint.redo;
 		ControlFile->time = (pg_time_t) time(NULL);
 		/* No need to hold ControlFileLock yet, we aren't up far enough */
 		UpdateControlFile();
@@ -5703,15 +5713,9 @@ StartupXLOG(void)
 
 			InRedo = true;
 
-			if (minRecoveryPoint.xlogid == 0 && minRecoveryPoint.xrecoff == 0)
-				ereport(LOG,
-						(errmsg("redo starts at %X/%X",
-								ReadRecPtr.xlogid, ReadRecPtr.xrecoff)));
-			else
-				ereport(LOG,
-						(errmsg("redo starts at %X/%X, consistency will be reached at %X/%X",
-								ReadRecPtr.xlogid, ReadRecPtr.xrecoff,
-						minRecoveryPoint.xlogid, minRecoveryPoint.xrecoff)));
+			ereport(LOG,
+					(errmsg("redo starts at %X/%X",
+							ReadRecPtr.xlogid, ReadRecPtr.xrecoff)));
 
 			/*
 			 * Let postmaster know we've started redo now, so that it can
@@ -5771,7 +5775,8 @@ StartupXLOG(void)
 				 * Have we passed our safe starting point?
 				 */
 				if (!reachedMinRecoveryPoint &&
-					XLByteLE(minRecoveryPoint, EndRecPtr))
+					XLByteLE(minRecoveryPoint, EndRecPtr) &&
+					XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
 				{
 					reachedMinRecoveryPoint = true;
 					ereport(LOG,
@@ -5877,7 +5882,9 @@ StartupXLOG(void)
 	 * be further ahead --- ControlFile->minRecoveryPoint cannot have been
 	 * advanced beyond the WAL we processed.
 	 */
-	if (InRecovery && XLByteLT(EndOfLog, minRecoveryPoint))
+	if (InArchiveRecovery &&
+		(XLByteLT(EndOfLog, minRecoveryPoint) ||
+		 !XLogRecPtrIsInvalid(ControlFile->backupStartPoint)))
 	{
 		if (reachedStopPoint)	/* stopped because of stop request */
 			ereport(FATAL,
@@ -7310,6 +7317,32 @@ xlog_redo(XLogRecPtr lsn, XLogRecord *record)
 	{
 		/* nothing to do here */
 	}
+	else if (info == XLOG_BACKUP_END)
+	{
+		XLogRecPtr	startpoint;
+		memcpy(&startpoint, XLogRecGetData(record), sizeof(startpoint));
+
+		if (XLByteEQ(ControlFile->backupStartPoint, startpoint))
+		{
+			/*
+			 * We have reached the end of base backup, the point where
+			 * pg_stop_backup() was done. The data on disk is now consistent.
+			 * Reset backupStartPoint, and update minRecoveryPoint to make
+			 * sure we don't allow starting up at an earlier point even if
+			 * recovery is stopped and restarted soon after this.
+			 */
+			elog(DEBUG1, "end of backup reached");
+
+			LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
+
+			if (XLByteLT(ControlFile->minRecoveryPoint, lsn))
+				ControlFile->minRecoveryPoint = lsn;
+			MemSet(&ControlFile->backupStartPoint, 0, sizeof(XLogRecPtr));
+			UpdateControlFile();
+
+			LWLockRelease(ControlFileLock);
+		}
+	}
 }
 
 void
@@ -7351,6 +7384,14 @@ xlog_desc(StringInfo buf, uint8 xl_info, char *rec)
 	{
 		appendStringInfo(buf, "xlog switch");
 	}
+	else if (info == XLOG_BACKUP_END)
+	{
+		XLogRecPtr startpoint;
+
+		memcpy(&startpoint, rec, sizeof(XLogRecPtr));
+		appendStringInfo(buf, "backup end: %X/%X",
+						 startpoint.xlogid, startpoint.xrecoff);
+	}
 	else
 		appendStringInfo(buf, "UNKNOWN");
 }
@@ -7686,10 +7727,14 @@ pg_start_backup_callback(int code, Datum arg)
 /*
  * pg_stop_backup: finish taking an on-line backup dump
  *
- * We remove the backup label file created by pg_start_backup, and instead
- * create a backup history file in pg_xlog (whence it will immediately be
- * archived).  The backup history file contains the same info found in
- * the label file, plus the backup-end time and WAL location.
+ * We write an end-of-backup WAL record, and remove the backup label file
+ * created by pg_start_backup, creating a backup history file in pg_xlog
+ * instead (whence it will immediately be archived). The backup history file
+ * contains the same info found in the label file, plus the backup-end time
+ * and WAL location. Before 8.5, the backup-end time was read from the backup
+ * history file at the beginning of archive recovery, but we now use the WAL
+ * record for that and the file is for informational and debug purposes only.
+ *
  * Note: different from CancelBackup which just cancels online backup mode.
  */
 Datum
@@ -7697,6 +7742,7 @@ pg_stop_backup(PG_FUNCTION_ARGS)
 {
 	XLogRecPtr	startpoint;
 	XLogRecPtr	stoppoint;
+	XLogRecData	rdata;
 	pg_time_t	stamp_time;
 	char		strfbuf[128];
 	char		histfilepath[MAXPGPATH];
@@ -7738,22 +7784,6 @@ pg_stop_backup(PG_FUNCTION_ARGS)
 	LWLockRelease(WALInsertLock);
 
 	/*
-	 * Force a switch to a new xlog segment file, so that the backup is valid
-	 * as soon as archiver moves out the current segment file. We'll report
-	 * the end address of the XLOG SWITCH record as the backup stopping point.
-	 */
-	stoppoint = RequestXLogSwitch();
-
-	XLByteToSeg(stoppoint, _logId, _logSeg);
-	XLogFileName(stopxlogfilename, ThisTimeLineID, _logId, _logSeg);
-
-	/* Use the log timezone here, not the session timezone */
-	stamp_time = (pg_time_t) time(NULL);
-	pg_strftime(strfbuf, sizeof(strfbuf),
-				"%Y-%m-%d %H:%M:%S %Z",
-				pg_localtime(&stamp_time, log_timezone));
-
-	/*
 	 * Open the existing label file
 	 */
 	lfp = AllocateFile(BACKUP_LABEL_FILE, "r");
@@ -7781,6 +7811,30 @@ pg_stop_backup(PG_FUNCTION_ARGS)
 				 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
 
 	/*
+	 * Write the backup-end xlog record
+	 */
+	rdata.data = (char *) (&startpoint);
+	rdata.len = sizeof(startpoint);
+	rdata.buffer = InvalidBuffer;
+	rdata.next = NULL;
+	stoppoint = XLogInsert(RM_XLOG_ID, XLOG_BACKUP_END, &rdata);
+
+	/*
+	 * Force a switch to a new xlog segment file, so that the backup is valid
+	 * as soon as archiver moves out the current segment file.
+	 */
+	RequestXLogSwitch();
+
+	XLByteToSeg(stoppoint, _logId, _logSeg);
+	XLogFileName(stopxlogfilename, ThisTimeLineID, _logId, _logSeg);
+
+	/* Use the log timezone here, not the session timezone */
+	stamp_time = (pg_time_t) time(NULL);
+	pg_strftime(strfbuf, sizeof(strfbuf),
+				"%Y-%m-%d %H:%M:%S %Z",
+				pg_localtime(&stamp_time, log_timezone));
+
+	/*
 	 * Write the backup history file
 	 */
 	XLByteToSeg(startpoint, _logId, _logSeg);
@@ -8086,33 +8140,18 @@ pg_xlogfile_name(PG_FUNCTION_ARGS)
  * later than the start of the dump, and so if we rely on it as the start
  * point, we will fail to restore a consistent database state.
  *
- * We also attempt to retrieve the corresponding backup history file.
- * If successful, set *minRecoveryLoc to constrain valid PITR stopping
- * points.
- *
  * Returns TRUE if a backup_label was found (and fills the checkpoint
  * location into *checkPointLoc); returns FALSE if not.
  */
 static bool
-read_backup_label(XLogRecPtr *checkPointLoc, XLogRecPtr *minRecoveryLoc)
+read_backup_label(XLogRecPtr *checkPointLoc)
 {
 	XLogRecPtr	startpoint;
-	XLogRecPtr	stoppoint;
-	char		histfilename[MAXFNAMELEN];
-	char		histfilepath[MAXPGPATH];
 	char		startxlogfilename[MAXFNAMELEN];
-	char		stopxlogfilename[MAXFNAMELEN];
 	TimeLineID	tli;
-	uint32		_logId;
-	uint32		_logSeg;
 	FILE	   *lfp;
-	FILE	   *fp;
 	char		ch;
 
-	/* Default is to not constrain recovery stop point */
-	minRecoveryLoc->xlogid = 0;
-	minRecoveryLoc->xrecoff = 0;
-
 	/*
 	 * See if label file is present
 	 */
@@ -8150,45 +8189,6 @@ read_backup_label(XLogRecPtr *checkPointLoc, XLogRecPtr *minRecoveryLoc)
 				 errmsg("could not read file \"%s\": %m",
 						BACKUP_LABEL_FILE)));
 
-	/*
-	 * Try to retrieve the backup history file (no error if we can't)
-	 */
-	XLByteToSeg(startpoint, _logId, _logSeg);
-	BackupHistoryFileName(histfilename, tli, _logId, _logSeg,
-						  startpoint.xrecoff % XLogSegSize);
-
-	if (InArchiveRecovery)
-		RestoreArchivedFile(histfilepath, histfilename, "RECOVERYHISTORY", 0);
-	else
-		BackupHistoryFilePath(histfilepath, tli, _logId, _logSeg,
-							  startpoint.xrecoff % XLogSegSize);
-
-	fp = AllocateFile(histfilepath, "r");
-	if (fp)
-	{
-		/*
-		 * Parse history file to identify stop point.
-		 */
-		if (fscanf(fp, "START WAL LOCATION: %X/%X (file %24s)%c",
-				   &startpoint.xlogid, &startpoint.xrecoff, startxlogfilename,
-				   &ch) != 4 || ch != '\n')
-			ereport(FATAL,
-					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("invalid data in file \"%s\"", histfilename)));
-		if (fscanf(fp, "STOP WAL LOCATION: %X/%X (file %24s)%c",
-				   &stoppoint.xlogid, &stoppoint.xrecoff, stopxlogfilename,
-				   &ch) != 4 || ch != '\n')
-			ereport(FATAL,
-					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("invalid data in file \"%s\"", histfilename)));
-		*minRecoveryLoc = stoppoint;
-		if (ferror(fp) || FreeFile(fp))
-			ereport(FATAL,
-					(errcode_for_file_access(),
-					 errmsg("could not read file \"%s\": %m",
-							histfilepath)));
-	}
-
 	return true;
 }
 
diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h
index 2435f27..31cc537 100644
--- a/src/include/catalog/pg_control.h
+++ b/src/include/catalog/pg_control.h
@@ -62,6 +62,7 @@ typedef struct CheckPoint
 #define XLOG_NOOP						0x20
 #define XLOG_NEXTOID					0x30
 #define XLOG_SWITCH						0x40
+#define XLOG_BACKUP_END					0x50
 
 
 /* System status indicator */
@@ -117,7 +118,27 @@ typedef struct ControlFileData
 
 	CheckPoint	checkPointCopy; /* copy of last check point record */
 
-	XLogRecPtr	minRecoveryPoint;		/* must replay xlog to here */
+	/*
+	 * These two values determine the minimum point we must recover up to
+	 * before starting up:
+	 *
+	 * minRecoveryPoint is updated to the latest replayed LSN whenever we
+	 * flush a data change during archive recovery. That guards against
+	 * starting archive recovery, aborting it, and restarting with an earlier
+	 * stop location. If we've already flushed to disk data changes from WAL
+	 * record, we mustn't start up until we reach X again. Should be zero
+	 * when we're not doing archive recovery.
+	 *
+	 * backupStartPoint is the redo pointer of the backup start checkpoint,
+	 * if we are recovering from an online backup and haven't reached the end
+	 * of backup yet. It is reset to 0 when the end of backup is reached, and
+	 * we mustn't start up before that. A boolean would suffice otherwise, but
+	 * we use the redo location as a cross-check when we see an end-of-backup
+	 * record, to make sure the end-of-backup record is the counterpart of
+	 * the base backup actually used.
+	 */
+	XLogRecPtr	minRecoveryPoint;
+	XLogRecPtr	backupStartPoint;
 
 	/*
 	 * This data is used to check for hardware-architecture compatibility of
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to