From ed78384acd691e3336e170aa18af69d15d088027 Mon Sep 17 00:00:00 2001
From: Simon Riggs <simon@2ndQuadrant.com>
Date: Mon, 29 Nov 2010 01:08:02 +0000
Subject: [PATCH] Move call to GetTopTransactionId() earlier in LockAcquire(),
 removing an infrequently occurring race condition in Hot Standby. An xid must
 be assigned before a lock appears in shared memory, rather than immediately
 after, else GetRunningTransactionLocks() may see InvalidTransactionId,
 causing assertion failures during lock processing on standby.

Bug report and diagnosis by Fujii Masao, fix by me.
---
 src/backend/storage/ipc/standby.c | 29 +++++++++++++++++++++--------
 src/backend/storage/lmgr/lock.c   | 29 +++++++++++++++++++++--------
 src/include/storage/standby.h     |  1 +
 3 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 82da926156..5e0d1d067e 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -953,14 +953,6 @@ LogAccessExclusiveLock(Oid dbOid, Oid relOid)
 {
 	xl_standby_lock xlrec;
 
-	/*
-	 * Ensure that a TransactionId has been assigned to this transaction. We
-	 * don't actually need the xid yet but if we don't do this then
-	 * RecordTransactionCommit() and RecordTransactionAbort() will optimise
-	 * away the transaction completion record which recovery relies upon to
-	 * release locks. It's a hack, but for a corner case not worth adding code
-	 * for into the main commit path.
-	 */
 	xlrec.xid = GetTopTransactionId();
 
 	/*
@@ -973,3 +965,24 @@ LogAccessExclusiveLock(Oid dbOid, Oid relOid)
 
 	LogAccessExclusiveLocks(1, &xlrec);
 }
+
+/*
+ * Prepare to log an AccessExclusiveLock, for use during LockAcquire()
+ */
+void
+LogAccessExclusiveLockPrepare(void)
+{
+	/*
+	 * Ensure that a TransactionId has been assigned to this transaction,
+	 * for two reasons, both related to lock release on the standby.
+	 * First, we must assign an xid so that RecordTransactionCommit() and
+	 * RecordTransactionAbort() do not optimise away the transaction
+	 * completion record which recovery relies upon to release locks. It's
+	 * a hack, but for a corner case not worth adding code for into the
+	 * main commit path. Second, must must assign an xid before the lock
+	 * is recorded in shared memory, otherwise a concurrently executing
+	 * GetRunningTransactionLocks() might see a lock associated with an
+	 * InvalidTransactionId which we later assert cannot happen.
+	 */
+	(void) GetTopTransactionId();
+}
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 45cfbc0710..e7cf1cd7ad 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -499,6 +499,7 @@ LockAcquireExtended(const LOCKTAG *locktag,
 	int			partition;
 	LWLockId	partitionLock;
 	int			status;
+	bool		log_lock = false;
 
 	if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
 		elog(ERROR, "unrecognized lock method: %d", lockmethodid);
@@ -579,6 +580,24 @@ LockAcquireExtended(const LOCKTAG *locktag,
 		return LOCKACQUIRE_ALREADY_HELD;
 	}
 
+	/*
+	 * Emit a WAL record if acquisition of this lock needs to be replayed in a
+	 * standby server. Only AccessExclusiveLocks can conflict with lock types
+	 * that read-only transactions can acquire in a standby server.
+	 *
+	 * Make sure this definition matches the one in GetRunningTransactionLocks().
+	 *
+	 * First we prepare to log, then after lock acquired we issue log record.
+	 */
+	if (lockmode >= AccessExclusiveLock &&
+		locktag->locktag_type == LOCKTAG_RELATION &&
+		!RecoveryInProgress() &&
+		XLogStandbyInfoActive())
+	{
+		LogAccessExclusiveLockPrepare();
+		log_lock = true;
+	}
+
 	/*
 	 * Otherwise we've got to mess with the shared lock table.
 	 */
@@ -868,15 +887,9 @@ LockAcquireExtended(const LOCKTAG *locktag,
 
 	/*
 	 * Emit a WAL record if acquisition of this lock need to be replayed in a
-	 * standby server. Only AccessExclusiveLocks can conflict with lock types
-	 * that read-only transactions can acquire in a standby server.
-	 *
-	 * Make sure this definition matches the one GetRunningTransactionLocks().
+	 * standby server.
 	 */
-	if (lockmode >= AccessExclusiveLock &&
-		locktag->locktag_type == LOCKTAG_RELATION &&
-		!RecoveryInProgress() &&
-		XLogStandbyInfoActive())
+	if (log_lock)
 	{
 		/*
 		 * Decode the locktag back to the original values, to avoid sending
diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h
index 553415c2d3..dec1d71323 100644
--- a/src/include/storage/standby.h
+++ b/src/include/storage/standby.h
@@ -109,6 +109,7 @@ typedef struct RunningTransactionsData
 typedef RunningTransactionsData *RunningTransactions;
 
 extern void LogAccessExclusiveLock(Oid dbOid, Oid relOid);
+extern void LogAccessExclusiveLockPrepare(void);
 
 extern void LogStandbySnapshot(TransactionId *oldestActiveXid, TransactionId *nextXid);