On Thu, Aug 11, 2011 at 1:34 AM, Heikki Linnakangas
<heikki.linnakan...@enterprisedb.com> wrote:
> Hmm, that's not possible for the 'tar' output, but would work for 'dir'
> output. Another similar idea would be to withhold the control file in memory
> until the end of backup, and append it to the output as last. The backup
> can't be restored until the control file is written out.
>
> That won't protect from more complicated scenarios, like if you take the
> backup without the -x flag, and copy some but not all of the required WAL
> files manually to the pg_xlog directory. But it'd be much better than
> nothing for 9.1.

We need to skip checking whether we've reached the end backup location
only when the server crashes while base backup using pg_start_backup. Right?
We can do this by *not* initializing ControlFile->backupStartPoint if the server
is doing crash recovery and backupEndRequired is false. What about the attached
patch?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 11035e6..d0d68d4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6329,11 +6329,8 @@ StartupXLOG(void)
 		/*
 		 * set backupStartPoint if we're starting recovery from a base backup
 		 */
-		if (haveBackupLabel)
-		{
+		if ((InArchiveRecovery && haveBackupLabel) || backupEndRequired)
 			ControlFile->backupStartPoint = checkPoint.redo;
-			ControlFile->backupEndRequired = backupEndRequired;
-		}
 		ControlFile->time = (pg_time_t) time(NULL);
 		/* No need to hold ControlFileLock yet, we aren't up far enough */
 		UpdateControlFile();
@@ -6703,20 +6700,13 @@ StartupXLOG(void)
 		 * crashes while an online backup is in progress. We must not treat
 		 * that as an error, or the database will refuse to start up.
 		 */
-		if (InArchiveRecovery || ControlFile->backupEndRequired)
-		{
-			if (ControlFile->backupEndRequired)
-				ereport(FATAL,
-						(errmsg("WAL ends before end of online backup"),
-						 errhint("All WAL generated while online backup was taken must be available at recovery.")));
-			else if (!XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
-				ereport(FATAL,
-						(errmsg("WAL ends before end of online backup"),
-						 errhint("Online backup started with pg_start_backup() must be ended with pg_stop_backup(), and all WAL up to that point must be available at recovery.")));
-			else
-				ereport(FATAL,
-					  (errmsg("WAL ends before consistent recovery point")));
-		}
+		if (!XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
+			ereport(FATAL,
+					(errmsg("WAL ends before end of online backup"),
+					 errhint("Online backup started with pg_start_backup() must be ended with pg_stop_backup(), and all WAL up to that point must be available at recovery.")));
+		else
+			ereport(FATAL,
+					(errmsg("WAL ends before consistent recovery point")));
 	}
 
 	/*
@@ -8540,7 +8530,6 @@ xlog_redo(XLogRecPtr lsn, XLogRecord *record)
 			if (XLByteLT(ControlFile->minRecoveryPoint, lsn))
 				ControlFile->minRecoveryPoint = lsn;
 			MemSet(&ControlFile->backupStartPoint, 0, sizeof(XLogRecPtr));
-			ControlFile->backupEndRequired = false;
 			UpdateControlFile();
 
 			LWLockRelease(ControlFileLock);
@@ -9826,8 +9815,8 @@ read_backup_label(XLogRecPtr *checkPointLoc, bool *backupEndRequired)
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("invalid data in file \"%s\"", BACKUP_LABEL_FILE)));
 	/*
-	 * BACKUP METHOD line is new in 9.0. Don't complain if it doesn't exist,
-	 * in case you're restoring from a backup taken with an 9.0 beta version
+	 * BACKUP METHOD line is new in 9.1. Don't complain if it doesn't exist,
+	 * in case you're restoring from a backup taken with an 9.1 beta version
 	 * that didn't emit it.
 	 */
 	if (fscanf(lfp, "BACKUP METHOD: %19s", backuptype) == 1)
diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h
index 6688c19..9600b50 100644
--- a/src/include/catalog/pg_control.h
+++ b/src/include/catalog/pg_control.h
@@ -137,16 +137,9 @@ typedef struct ControlFileData
 	 * we use the redo pointer as a cross-check when we see an end-of-backup
 	 * record, to make sure the end-of-backup record corresponds the base
 	 * backup we're recovering from.
-	 *
-	 * If backupEndRequired is true, we know for sure that we're restoring
-	 * from a backup, and must see a backup-end record before we can safely
-	 * start up. If it's false, but backupStartPoint is set, a backup_label
-	 * file was found at startup but it may have been a leftover from a stray
-	 * pg_start_backup() call, not accompanied by pg_stop_backup().
 	 */
 	XLogRecPtr	minRecoveryPoint;
 	XLogRecPtr	backupStartPoint;
-	bool		backupEndRequired;
 
 	/*
 	 * Parameter settings that determine if the WAL can be used for archival
-- 
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