Ensure correct minimum consistent point on standbys

Startup process has improved its calculation of incorrect minimum
consistent point in 8d68ee6, which ensures that all WAL available gets
replayed when doing crash recovery, and has introduced an incorrect
calculation of the minimum recovery point for non-startup processes,
which can cause incorrect page references on a standby when for example
the background writer flushed a couple of pages on-disk but was not
updating the control file to let a subsequent crash recovery replay to
where it should have.

The only case where this has been reported to be a problem is when a
standby needs to calculate the latest removed xid when replaying a btree
deletion record, so one would need connections on a standby that happen
just after recovery has thought it reached a consistent point.  Using a
background worker which is started after the consistent point is reached
would be the easiest way to get into problems if it connects to a
database.  Having clients which attempt to connect periodically could
also be a problem, but the odds of seeing this problem are much lower.

The fix used is pretty simple, as the idea is to give access to the
minimum recovery point written in the control file to non-startup
processes so as they use a reference, while the startup process still
initializes its own references of the minimum consistent point so as the
original problem with incorrect page references happening post-promotion
with a crash do not show up.

Reported-by: Alexander Kukushkin
Diagnosed-by: Alexander Kukushkin
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Alexander Kukushkin
Discussion: https://postgr.es/m/153492341830.1368.3936905691758473953@wrigleys.postgresql.org
Backpatch-through: 9.3

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index cfef58d69dfa5475ef00607cf6deff9f413f6060..e0d3a93adee4c71a49a2c0a3be0362f54101d59e 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2722,9 +2722,13 @@ UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force)
         * i.e., we're doing crash recovery.  We never modify the control file's
         * value in that case, so we can short-circuit future checks here too. The
         * local values of minRecoveryPoint and minRecoveryPointTLI should not be
-        * updated until crash recovery finishes.
+        * updated until crash recovery finishes.  We only do this for the startup
+        * process as it should not update its own reference of minRecoveryPoint
+        * until it has finished crash recovery to make sure that all WAL
+        * available is replayed in this case.  This also saves from extra locks
+        * taken on the control file from the startup process.
         */
-       if (XLogRecPtrIsInvalid(minRecoveryPoint))
+       if (XLogRecPtrIsInvalid(minRecoveryPoint) && InRecovery)
        {
                updateMinRecoveryPoint = false;
                return;
@@ -2736,7 +2740,9 @@ UpdateMinRecoveryPoint(XLogRecPtr lsn, bool force)
        minRecoveryPoint = ControlFile->minRecoveryPoint;
        minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
 
-       if (force || minRecoveryPoint < lsn)
+       if (XLogRecPtrIsInvalid(minRecoveryPoint))
+               updateMinRecoveryPoint = false;
+       else if (force || minRecoveryPoint < lsn)
        {
                XLogRecPtr      newMinRecoveryPoint;
                TimeLineID      newMinRecoveryPointTLI;
@@ -3126,9 +3132,11 @@ XLogNeedsFlush(XLogRecPtr record)
                 * An invalid minRecoveryPoint means that we need to recover all the
                 * WAL, i.e., we're doing crash recovery.  We never modify the control
                 * file's value in that case, so we can short-circuit future checks
-                * here too.
+                * here too.  This triggers a quick exit path for the startup process,
+                * which cannot update its local copy of minRecoveryPoint as long as
+                * it has not replayed all WAL available when doing crash recovery.
                 */
-               if (XLogRecPtrIsInvalid(minRecoveryPoint))
+               if (XLogRecPtrIsInvalid(minRecoveryPoint) && InRecovery)
                        updateMinRecoveryPoint = false;
 
                /* Quick exit if already known to be updated or cannot be updated */
@@ -3145,8 +3153,19 @@ XLogNeedsFlush(XLogRecPtr record)
                minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
                LWLockRelease(ControlFileLock);
 
+               /*
+                * Check minRecoveryPoint for any other process than the startup
+                * process doing crash recovery, which should not update the control
+                * file value if crash recovery is still running.
+                */
+               if (XLogRecPtrIsInvalid(minRecoveryPoint))
+                       updateMinRecoveryPoint = false;
+
                /* check again */
-               return record > minRecoveryPoint;
+               if (record <= minRecoveryPoint || !updateMinRecoveryPoint)
+                       return false;
+               else
+                       return true;
        }
 
        /* Quick exit if already known flushed */