From 5541abee0dd56669ef0e9a87121e449d905c3efd Mon Sep 17 00:00:00 2001
From: "Vadim B. Mikheev" <vadim4o@yahoo.com>
Date: Thu, 3 Jun 1999 13:33:13 +0000
Subject: [PATCH] 1. Additional fix against ERROR:  Child itemid marked as
 unused    in CommitTransaction(). 2. Changes in GetSnapshotData().

---
 src/backend/access/transam/xact.c | 36 ++++++++++++++++++-----------
 src/backend/storage/ipc/shmem.c   | 38 +++++++++++++++++++++----------
 2 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 6884e63590..6bde05733a 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.38 1999/06/03 04:41:41 vadim Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.39 1999/06/03 13:33:12 vadim Exp $
  *
  * NOTES
  *		Transaction aborts can now occur two ways:
@@ -646,18 +646,6 @@ RecordTransactionCommit()
 	FlushBufferPool(!TransactionFlushEnabled());
 	if (leak)
 		ResetBufferPool();
-
-	/*
-	 * Let others know about no transaction in progress.
-	 * Note that this must be done _before_ releasing locks
-	 * we hold or bad (too high) XmaxRecent value might be
-	 * used by vacuum.
-	 */
-	if (MyProc != (PROC *) NULL)
-	{
-		MyProc->xid = InvalidTransactionId;
-		MyProc->xmin = InvalidTransactionId;
-	}
 }
 
 
@@ -950,6 +938,28 @@ CommitTransaction()
 	DestroyNoNameRels();
 	AtEOXact_portals();
 	RecordTransactionCommit();
+
+	/*
+	 * Let others know about no transaction in progress by me.
+	 * Note that this must be done _before_ releasing locks we hold 
+	 * and SpinAcquire(ShmemIndexLock) is required - or bad (too high) 
+	 * XmaxRecent value might be used by vacuum: UPDATE with xid 0 is 
+	 * blocked by xid 1' UPDATE, xid 1 is doing commit while xid 2 
+	 * gets snapshot - if xid 2' GetSnapshotData sees xid 1 as running
+	 * then it must see xid 0 as running as well or XmaxRecent = 1
+	 * might be used by concurrent vacuum causing
+	 *         ERROR:  Child itemid marked as unused
+	 * This bug was reported by Hiroshi Inoue and I was able to reproduce
+	 * it with 3 sessions and gdb.	- vadim 06/03/99
+	 */
+	if (MyProc != (PROC *) NULL)
+	{
+		SpinAcquire(ShmemIndexLock);
+		MyProc->xid = InvalidTransactionId;
+		MyProc->xmin = InvalidTransactionId;
+		SpinRelease(ShmemIndexLock);
+	}
+
 	RelationPurgeLocalRelation(true);
 	AtCommit_Cache();
 	AtCommit_Locks();
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index eafe53b0bb..04cfa1a812 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/storage/ipc/shmem.c,v 1.40 1999/05/25 16:11:11 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/storage/ipc/shmem.c,v 1.41 1999/06/03 13:33:13 vadim Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -593,6 +593,10 @@ ShmemInitStruct(char *name, unsigned long size, bool *foundPtr)
  *
  * Strange place for this func, but we have to lookup process data structures
  * for all running backends. - vadim 11/26/96
+ *
+ * We should keep all PROC structs not in ShmemIndex - this is too
+ * general hash table...
+ *
  */
 bool
 TransactionIdIsInProgress(TransactionId xid)
@@ -648,10 +652,7 @@ GetSnapshotData(bool serializable)
 
 	snapshot->xip = (TransactionId *) malloc(have * sizeof(TransactionId));
 	snapshot->xmin = cid;
-	if (serializable)
-		snapshot->xmax = cid;
-	else
-		ReadNewTransactionId(&(snapshot->xmax));
+	ReadNewTransactionId(&(snapshot->xmax));
 
 	SpinAcquire(ShmemIndexLock);
 
@@ -660,8 +661,10 @@ GetSnapshotData(bool serializable)
 	{
 		if (result == (ShmemIndexEnt *) TRUE)
 		{
-			if (MyProc->xmin == InvalidTransactionId)
+			if (serializable)
 				MyProc->xmin = snapshot->xmin;
+			/* Serializable snapshot must be computed before any other... */
+			Assert(MyProc->xmin != InvalidTransactionId);
 			SpinRelease(ShmemIndexLock);
 			snapshot->xcnt = count;
 			return snapshot;
@@ -670,13 +673,24 @@ GetSnapshotData(bool serializable)
 			strncmp(result->key, "PID ", 4) != 0)
 			continue;
 		proc = (PROC *) MAKE_PTR(result->location);
-		xid = proc->xid;		/* we don't use spin-locking in xact.c ! */
-		if (proc == MyProc || xid < FirstTransactionId)
+		/* 
+		 * We don't use spin-locking when changing proc->xid 
+		 * in GetNewTransactionId() and in AbortTransaction() !..
+		 */
+		xid = proc->xid;
+		if (proc == MyProc || 
+			xid < FirstTransactionId || xid >= snapshot->xmax)
+		{
+			/*
+			 * Seems that there is no sense to store xid >= snapshot->xmax
+			 * (what we got from ReadNewTransactionId above) in snapshot->xip 
+			 * - we just assume that all xacts with such xid-s are running 
+			 * and may be ignored.
+			 */
 			continue;
+		}
 		if (xid < snapshot->xmin)
 			snapshot->xmin = xid;
-		else if (xid > snapshot->xmax)
-			snapshot->xmax = xid;
 		if (have == 0)
 		{
 			snapshot->xip = (TransactionId *) realloc(snapshot->xip,
@@ -712,7 +726,7 @@ GetXmaxRecent(TransactionId *XmaxRecent)
 
 	Assert(ShmemIndex);
 
-	ReadNewTransactionId(XmaxRecent);
+	*XmaxRecent = GetCurrentTransactionId();
 
 	SpinAcquire(ShmemIndexLock);
 
@@ -728,7 +742,7 @@ GetXmaxRecent(TransactionId *XmaxRecent)
 			strncmp(result->key, "PID ", 4) != 0)
 			continue;
 		proc = (PROC *) MAKE_PTR(result->location);
-		xmin = proc->xmin;		/* we don't use spin-locking in xact.c ! */
+		xmin = proc->xmin;	/* we don't use spin-locking in AbortTransaction() ! */
 		if (proc == MyProc || xmin < FirstTransactionId)
 			continue;
 		if (xmin < *XmaxRecent)