Don't advance checkPoint.nextXid near the end of a checkpoint sequence.

This reverts commit c11130690d6dca64267201a169cfb38c1adec5ef in favor of
actually fixing the problem: namely, that we should never have been
modifying the checkpoint record's nextXid at this point to begin with.
The nextXid should match the state as of the checkpoint's logical WAL
position (ie the redo point), not the state as of its physical position.
It's especially bogus to advance it in some wal_levels and not others.
In any case there is no need for the checkpoint record to carry the
same nextXid shown in the XLOG_RUNNING_XACTS record just emitted by
LogStandbySnapshot, as any replay operation will already have adopted
that value as current.

This fixes bug #7710 from Tarvi Pillessaar, and probably also explains bug
#6291 from Daniel Farina, in that if a checkpoint were in progress at the
instant of XID wraparound, the epoch bump would be lost as reported.
(And, of course, these days there's at least a 50-50 chance of a checkpoint
being in progress at any given instant.)

Diagnosed by me and independently by Andres Freund.  Back-patch to all
branches supporting hot standby.

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5a06ae2a4a3560c15c9fd9fb07f61d8db21a8a63..50e2b22dd70e35de9f84e88fbdd4566c9b2f033b 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8019,18 +8019,9 @@ CreateCheckPoint(int flags)
         *
         * If we are shutting down, or Startup process is completing crash
         * recovery we don't need to write running xact data.
-        *
-        * Update checkPoint.nextXid since we may have a later value. If we
-        * do update the value, and we have wrapped, increment epoch also.
         */
        if (!shutdown && XLogStandbyInfoActive())
-       {
-               TransactionId prevXid = checkPoint.nextXid;
-
-               LogStandbySnapshot(&checkPoint.nextXid);
-               if (checkPoint.nextXid < prevXid)
-                       checkPoint.nextXidEpoch++;
-       }
+               LogStandbySnapshot();
 
        START_CRIT_SECTION();
 

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 0658feb076cafcd262b029a6b0cbef76d8189fac..8b3b8331aa9616cbe8d6fde943b23f3284b76a29 100644 (file)
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -865,7 +865,7 @@ standby_desc(StringInfo buf, uint8 xl_info, char *rec)
  * from a time when they were possible.
  */
 void
-LogStandbySnapshot(TransactionId *nextXid)
+LogStandbySnapshot(void)
 {
        RunningTransactions running;
        xl_standby_lock *locks;
@@ -894,8 +894,6 @@ LogStandbySnapshot(TransactionId *nextXid)
        LogCurrentRunningXacts(running);
        /* GetRunningTransactionData() acquired XidGenLock, we must release it */
        LWLockRelease(XidGenLock);
-
-       *nextXid = running->nextXid;
 }
 
 /*

diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
index ed3b66b35df07553a77a17619e54d52b4a981562..4b69be077b1bc2a6f5b617e74c1b0f43b8590a0c 100644 (file)
--- a/src/include/storage/standby.h
+++ b/src/include/storage/standby.h
@@ -110,6 +110,6 @@ typedef RunningTransactionsData *RunningTransactions;
 extern void LogAccessExclusiveLock(Oid dbOid, Oid relOid);
 extern void LogAccessExclusiveLockPrepare(void);
 
-extern void LogStandbySnapshot(TransactionId *nextXid);
+extern void LogStandbySnapshot(void);
 
 #endif   /* STANDBY_H */