From 86e33648992cfc104836ab1fbb6e654308beb4a5 Mon Sep 17 00:00:00 2001
From: Simon Riggs <simon@2ndQuadrant.com>
Date: Wed, 2 Nov 2011 08:54:56 +0000
Subject: [PATCH] Derive oldestActiveXid at correct time for Hot Standby. There
 was a timing window between when oldestActiveXid was derived and when it
 should have been derived that only shows itself under heavy load. Move code
 around to ensure correct timing of derivation. No change to StartupSUBTRANS()
 code, which is where this failed.

Bug report by Chris Redekop
---
 src/backend/access/transam/xlog.c   | 14 +++++--
 src/backend/storage/ipc/procarray.c | 57 +++++++++++++++++++++++++++++
 src/backend/storage/ipc/standby.c   |  3 +-
 src/include/storage/procarray.h     |  1 +
 src/include/storage/standby.h       |  2 +-
 5 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index bf57b3bf49e..0944aa4162d 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7636,6 +7636,16 @@ CreateCheckPoint(int flags)
 	MemSet(&checkPoint, 0, sizeof(checkPoint));
 	checkPoint.time = (pg_time_t) time(NULL);
 
+	/*
+	 * For Hot Standby, derive the oldestActiveXid before we fix the redo pointer.
+	 * This allows us to begin accumulating changes to assemble our starting
+	 * snapshot of locks and transactions.
+	 */
+	if (!shutdown && XLogStandbyInfoActive())
+		checkPoint.oldestActiveXid = GetOldestActiveTransactionId();
+	else
+		checkPoint.oldestActiveXid = InvalidTransactionId;
+
 	/*
 	 * We must hold WALInsertLock while examining insert state to determine
 	 * the checkpoint REDO pointer.
@@ -7822,9 +7832,7 @@ CreateCheckPoint(int flags)
 	 * Update checkPoint.nextXid since we have a later value
 	 */
 	if (!shutdown && XLogStandbyInfoActive())
-		LogStandbySnapshot(&checkPoint.oldestActiveXid, &checkPoint.nextXid);
-	else
-		checkPoint.oldestActiveXid = InvalidTransactionId;
+		LogStandbySnapshot(&checkPoint.nextXid);
 
 	START_CRIT_SECTION();
 
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 0284ac40823..1a48485f970 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -1620,6 +1620,63 @@ GetRunningTransactionData(void)
 	return CurrentRunningXacts;
 }
 
+/*
+ * GetOldestActiveTransactionId()
+ *
+ * Similar to GetSnapshotData but returns just oldestActiveXid. We include
+ * all PGPROCs with an assigned TransactionId, even VACUUM processes.
+ * We look at all databases, though there is no need to include WALSender
+ * since this has no effect on hot standby conflicts.
+ *
+ * This is never executed during recovery so there is no need to look at
+ * KnownAssignedXids.
+ *
+ * We don't worry about updating other counters, we want to keep this as
+ * simple as possible and leave GetSnapshotData() as the primary code for
+ * that bookkeeping.
+ */
+TransactionId
+GetOldestActiveTransactionId(void)
+{
+	ProcArrayStruct *arrayP = procArray;
+	TransactionId oldestRunningXid;
+	int			index;
+
+	Assert(!RecoveryInProgress());
+
+	LWLockAcquire(ProcArrayLock, LW_SHARED);
+
+	oldestRunningXid = ShmemVariableCache->nextXid;
+
+	/*
+	 * Spin over procArray collecting all xids and subxids.
+	 */
+	for (index = 0; index < arrayP->numProcs; index++)
+	{
+		volatile PGPROC *proc = arrayP->procs[index];
+		TransactionId xid;
+
+		/* Fetch xid just once - see GetNewTransactionId */
+		xid = proc->xid;
+
+		if (!TransactionIdIsNormal(xid))
+			continue;
+
+		if (TransactionIdPrecedes(xid, oldestRunningXid))
+			oldestRunningXid = xid;
+
+		/*
+		 * Top-level XID of a transaction is always less than any of its
+		 * subxids, so we don't need to check if any of the subxids are
+		 * smaller than oldestRunningXid
+		 */
+	}
+
+	LWLockRelease(ProcArrayLock);
+
+	return oldestRunningXid;
+}
+
 /*
  * GetTransactionsInCommit -- Get the XIDs of transactions that are committing
  *
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 72c6b97b225..3f2fc813681 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -815,7 +815,7 @@ standby_desc(StringInfo buf, uint8 xl_info, char *rec)
  * making WAL entries.
  */
 void
-LogStandbySnapshot(TransactionId *oldestActiveXid, TransactionId *nextXid)
+LogStandbySnapshot(TransactionId *nextXid)
 {
 	RunningTransactions running;
 	xl_standby_lock *locks;
@@ -845,7 +845,6 @@ LogStandbySnapshot(TransactionId *oldestActiveXid, TransactionId *nextXid)
 	/* GetRunningTransactionData() acquired XidGenLock, we must release it */
 	LWLockRelease(XidGenLock);
 
-	*oldestActiveXid = running->oldestRunningXid;
 	*nextXid = running->nextXid;
 }
 
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index 71c82437cdf..3e80cc5c075 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -50,6 +50,7 @@ extern RunningTransactions GetRunningTransactionData(void);
 extern bool TransactionIdIsInProgress(TransactionId xid);
 extern bool TransactionIdIsActive(TransactionId xid);
 extern TransactionId GetOldestXmin(bool allDbs, bool ignoreVacuum);
+extern TransactionId GetOldestActiveTransactionId(void);
 
 extern int	GetTransactionsInCommit(TransactionId **xids_p);
 extern bool HaveTransactionsInCommit(TransactionId *xids, int nxids);
diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
index 6ebac62db54..e587a5c4a4d 100644
--- 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 *oldestActiveXid, TransactionId *nextXid);
+extern void LogStandbySnapshot(TransactionId *nextXid);
 
 #endif   /* STANDBY_H */
-- 
2.39.5