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

tglsfdc · tglsfdc · commit 135f4f605517 · 2012-12-02T15:20:15.000-05:00
This reverts commit c111306 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
@@ -7422,18 +7422,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
@@ -869,7 +869,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;
@@ -898,8 +898,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
@@ -111,6 +111,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 */
