>From 35fdab06ceefcd0b4399550d76f9e134a1ae5880 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu, 12 Dec 2013 17:27:54 -0300
Subject: [PATCH 3/4] fixup XidIsInProgress before transam.c tests

---
 src/backend/access/heap/heapam.c |   81 ++++++++++++++++++++++++++------------
 1 file changed, 55 insertions(+), 26 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 9509480..9616c18 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -5381,47 +5381,76 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 
 	for (i = 0; i < nmembers; i++)
 	{
+		/*
+		 * Determine whether to keep this member or ignore it.
+		 */
 		if (ISUPDATE_from_mxstatus(members[i].status))
 		{
+			TransactionId	xid = members[i].xid;
+
 			/*
 			 * It's an update; should we keep it?  If the transaction is known
-			 * aborted then it's okay to ignore it, otherwise not.  (Note this
-			 * is just an optimization and not needed for correctness, so it's
-			 * okay to get this test wrong; for example, in case an updater is
-			 * crashed, or a running transaction in the process of aborting.)
+			 * aborted then it's okay to ignore it, otherwise not.  However,
+			 * if the Xid is older than the cutoff_xid, we must remove it,
+			 * because otherwise we might allow a very old Xid to persist
+			 * which would later cause pg_clog lookups problems, because the
+			 * corresponding SLRU segment might be about to be truncated away.
+			 * (Note that such an old updater cannot possibly be committed,
+			 * because HeapTupleSatisfiesVacuum would have returned
+			 * HEAPTUPLE_DEAD and we would not be trying to freeze the tuple.)
+			 *
+			 * Note the TransactionIdDidAbort() test is just an optimization
+			 * and not strictly necessary for correctness.
+			 *
+			 * As with all tuple visibility routines, it's critical to test
+			 * TransactionIdIsInProgress before the transam.c routines,
+			 * because of race conditions explained in detail in tqual.c.
 			 */
-			if (!TransactionIdDidAbort(members[i].xid))
+			if (TransactionIdIsCurrentTransactionId(xid) ||
+				TransactionIdIsInProgress(xid))
 			{
-				newmembers[nnewmembers++] = members[i];
 				Assert(!TransactionIdIsValid(update_xid));
-
+				update_xid = xid;
+			}
+			else if (!TransactionIdDidAbort(xid))
+			{
 				/*
-				 * Tell caller to set HEAP_XMAX_COMMITTED hint while we have
-				 * the Xid in cache.  Again, this is just an optimization, so
-				 * it's not a problem if the transaction is still running and
-				 * in the process of committing.
+				 * Test whether to tell caller to set HEAP_XMAX_COMMITTED
+				 * while we have the Xid still in cache.  Note this can only
+				 * be done if the transaction is known not running.
 				 */
-				if (TransactionIdDidCommit(update_xid))
+				if (TransactionIdDidCommit(xid))
 					update_committed = true;
-
-				update_xid = newmembers[i].xid;
+				Assert(!TransactionIdIsValid(update_xid));
+				update_xid = xid;
 			}
 
 			/*
-			 * Checking for very old update Xids is critical: if the update
-			 * member of the multi is older than cutoff_xid, we must remove
-			 * it, because otherwise a later liveliness check could attempt
-			 * pg_clog access for a page that was truncated away by the
-			 * current vacuum.	Note that if the update had committed, we
-			 * wouldn't be freezing this tuple because it would have gotten
-			 * removed (HEAPTUPLE_DEAD) by HeapTupleSatisfiesVacuum; so it
-			 * either aborted or crashed.  Therefore, ignore this update_xid.
+			 * If we determined that it's an Xid corresponding to an update
+			 * that must be retained, additionally add it to the list of
+			 * members of the new Multis, in case we end up using that.  (We
+			 * might still decide to use only an update Xid and not a multi,
+			 * but it's easier to maintain the list as we walk the old members
+			 * list.)
+			 *
+			 * It is possible to end up with a very old updater Xid that
+			 * crashed and thus did not mark itself as aborted in pg_clog.
+			 * That would manifest as a pre-cutoff Xid.  Make sure to ignore
+			 * it.
 			 */
-			if (TransactionIdPrecedes(update_xid, cutoff_xid))
+			if (TransactionIdIsValid(update_xid))
 			{
-				update_xid = InvalidTransactionId;
-				update_committed = false;
-
+				if (!TransactionIdPrecedes(update_xid, cutoff_xid))
+				{
+					newmembers[nnewmembers++] = members[i];
+				}
+				else
+				{
+					/* cannot have committed: would be HEAPTUPLE_DEAD */
+					Assert(!TransactionIdDidCommit(update_xid));
+					update_xid = InvalidTransactionId;
+					update_committed = false;
+				}
 			}
 		}
 		else
-- 
1.7.10.4