postgres.git / summary / log / commit / refs

commit    425bef6ee7f210c991f35dff4b3ed6691818c610
Author:   Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date:     Wed Jan 08 09:39:55 2014 +0000
First released in: 9.4.0, 9.0.16, 9.1.12, 9.2.7, 9.3.3

    Fix bug in determining when recovery has reached consistency.
    
    When starting WAL replay from an online checkpoint, the last replayed WAL
    record variable was initialized using the checkpoint record's location, even
    though the records between the REDO location and the checkpoint record had
    not been replayed yet. That was noted as "slightly confusing" but harmless
    in the comment, but in some cases, it fooled CheckRecoveryConsistency to
    incorrectly conclude that we had already reached a consistent state
    immediately at the beginning of WAL replay. That caused the system to accept
    read-only connections in hot standby mode too early, and also PANICs with
    message "WAL contains references to invalid pages".
    
    Fix by initializing the variables to the REDO location instead.
    
    In 9.2 and above, change CheckRecoveryConsistency() to use
    lastReplayedEndRecPtr variable when checking if backup end location has
    been reached. It was inconsistently using EndRecPtr for that check, but
    lastReplayedEndRecPtr when checking min recovery point. It made no
    difference before this patch, because in all the places where
    CheckRecoveryConsistency was called the two variables were the same, but
    it was always an accident waiting to happen, and would have been wrong
    after this patch anyway.
    
    Report and analysis by Tomonari Katsumata, bug #8686. Backpatch to 9.0,
    where hot standby was introduced.


 src/backend/access/transam/xlog.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index d256524347e..2ea0bb07aaf 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5452,21 +5452,13 @@ StartupXLOG(void)
 		}
 
 		/*
-		 * Initialize shared replayEndRecPtr, lastReplayedEndRecPtr, and
-		 * recoveryLastXTime.
-		 *
-		 * This is slightly confusing if we're starting from an online
-		 * checkpoint; we've just read and replayed the checkpoint record, but
-		 * we're going to start replay from its redo pointer, which precedes
-		 * the location of the checkpoint record itself. So even though the
-		 * last record we've replayed is indeed ReadRecPtr, we haven't
-		 * replayed all the preceding records yet. That's OK for the current
-		 * use of these variables.
+		 * Initialize shared variables for tracking progress of WAL replay,
+		 * as if we had just replayed the record before the REDO location.
 		 */
 		SpinLockAcquire(&xlogctl->info_lck);
-		xlogctl->replayEndRecPtr = ReadRecPtr;
+		xlogctl->replayEndRecPtr = checkPoint.redo;
 		xlogctl->replayEndTLI = ThisTimeLineID;
-		xlogctl->lastReplayedEndRecPtr = EndRecPtr;
+		xlogctl->lastReplayedEndRecPtr = checkPoint.redo;
 		xlogctl->lastReplayedTLI = ThisTimeLineID;
 		xlogctl->recoveryLastXTime = 0;
 		xlogctl->currentChunkStartTime = 0;
@@ -6137,6 +6129,8 @@ StartupXLOG(void)
 static void
 CheckRecoveryConsistency(void)
 {
+	XLogRecPtr lastReplayedEndRecPtr;
+
 	/*
 	 * During crash recovery, we don't reach a consistent state until we've
 	 * replayed all the WAL.
@@ -6144,11 +6138,17 @@ CheckRecoveryConsistency(void)
 	if (XLogRecPtrIsInvalid(minRecoveryPoint))
 		return;
 
+	/*
+	 * assume that we are called in the startup process, and hence don't need
+	 * a lock to read lastReplayedEndRecPtr
+	 */
+	lastReplayedEndRecPtr = XLogCtl->lastReplayedEndRecPtr;
+
 	/*
 	 * Have we reached the point where our base backup was completed?
 	 */
 	if (!XLogRecPtrIsInvalid(ControlFile->backupEndPoint) &&
-		ControlFile->backupEndPoint <= EndRecPtr)
+		ControlFile->backupEndPoint <= lastReplayedEndRecPtr)
 	{
 		/*
 		 * We have reached the end of base backup, as indicated by pg_control.
@@ -6161,8 +6161,8 @@ CheckRecoveryConsistency(void)
 
 		LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
 
-		if (ControlFile->minRecoveryPoint < EndRecPtr)
-			ControlFile->minRecoveryPoint = EndRecPtr;
+		if (ControlFile->minRecoveryPoint < lastReplayedEndRecPtr)
+			ControlFile->minRecoveryPoint = lastReplayedEndRecPtr;
 
 		ControlFile->backupStartPoint = InvalidXLogRecPtr;
 		ControlFile->backupEndPoint = InvalidXLogRecPtr;
@@ -6180,7 +6180,7 @@ CheckRecoveryConsistency(void)
 	 * consistent yet.
 	 */
 	if (!reachedConsistency && !ControlFile->backupEndRequired &&
-		minRecoveryPoint <= XLogCtl->lastReplayedEndRecPtr &&
+		minRecoveryPoint <= lastReplayedEndRecPtr &&
 		XLogRecPtrIsInvalid(ControlFile->backupStartPoint))
 	{
 		/*
@@ -6192,8 +6192,8 @@ CheckRecoveryConsistency(void)
 		reachedConsistency = true;
 		ereport(LOG,
 				(errmsg("consistent recovery state reached at %X/%X",
-						(uint32) (XLogCtl->lastReplayedEndRecPtr >> 32),
-						(uint32) XLogCtl->lastReplayedEndRecPtr)));
+						(uint32) (lastReplayedEndRecPtr >> 32),
+						(uint32) lastReplayedEndRecPtr)));
 	}
 
 	/*


[parent: a826773bf6a4]