1. Additional fix against ERROR: Child itemid marked as unused

   in CommitTransaction().
2. Changes in GetSnapshotData().

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 6884e635904fce8e6d174c7797d476763e321b72..6bde05733ac829b51776bfc4cc8ca09a4f4f7f41 100644 (file)
--- 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 eafe53b0bb13de4cfd788f113cfecb36b6acf085..04cfa1a812eaa49c9e9ecf95f2c707fab0e35431 100644 (file)
--- 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)