Tweak heap_update/delete so that we do not hold the buffer context lock

on the old tuple's page while we are doing TOAST pushups.

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index db443218b4def352bf3b2d5939a26dc85f9cc137..c8436b709d1da6099e17bab7a0c05b4a495a4abf 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.107 2001/01/12 21:53:54 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.108 2001/01/15 05:29:19 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1534,18 +1534,20 @@ l1:
        }
        END_CRIT_SECTION();
 
+       LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+
 #ifdef TUPLE_TOASTER_ACTIVE
        /* ----------
         * If the relation has toastable attributes, we need to delete
-        * no longer needed items there too.
+        * no longer needed items there too.  We have to do this before
+        * WriteBuffer because we need to look at the contents of the tuple,
+        * but it's OK to release the context lock on the buffer first.
         * ----------
         */
        if (HeapTupleHasExtended(&tp))
                heap_tuple_toast_attrs(relation, NULL, &(tp));
 #endif
 
-       LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
-
        /*
         * Mark tuple for invalidation from system caches at next command boundary.
         * We have to do this before WriteBuffer because we need to look at the
@@ -1568,7 +1570,10 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
        ItemId          lp;
        HeapTupleData oldtup;
        PageHeader      dp;
-       Buffer          buffer, newbuf;
+       Buffer          buffer,
+                               newbuf;
+       bool            need_toast,
+                               already_marked;
        int                     result;
 
        /* increment access statistics */
@@ -1645,7 +1650,7 @@ l2:
                return result;
        }
 
-       /* XXX order problems if not atomic assignment ??? */
+       /* Fill in OID and transaction status data for newtup */
        newtup->t_data->t_oid = oldtup.t_data->t_oid;
        TransactionIdStore(GetCurrentTransactionId(), &(newtup->t_data->t_xmin));
        newtup->t_data->t_cmin = GetCurrentCommandId();
@@ -1653,69 +1658,84 @@ l2:
        newtup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
        newtup->t_data->t_infomask |= (HEAP_XMAX_INVALID | HEAP_UPDATED);
 
-#ifdef TUPLE_TOASTER_ACTIVE
-       /* ----------
-        * If this relation is enabled for toasting, let the toaster
-        * delete any no-longer-needed entries and create new ones to
-        * make the new tuple fit again.  Also, if there are already-
-        * toasted values from some other relation, the toaster must
-        * fix them.
-        * ----------
+       /*
+        * If the toaster needs to be activated, OR if the new tuple will not
+        * fit on the same page as the old, then we need to release the context
+        * lock (but not the pin!) on the old tuple's buffer while we are off
+        * doing TOAST and/or table-file-extension work.  We must mark the old
+        * tuple to show that it's already being updated, else other processes
+        * may try to update it themselves. To avoid second XLOG log record,
+        * we use xact mgr hook to unlock old tuple without reading log if xact
+        * will abort before update is logged. In the event of crash prio logging,
+        * TQUAL routines will see HEAP_XMAX_UNLOGGED flag...
+        *
+        * NOTE: this trick is useless currently but saved for future
+        * when we'll implement UNDO and will re-use transaction IDs
+        * after postmaster startup.
+        *
+        * We need to invoke the toaster if there are already any toasted values
+        * present, or if the new tuple is over-threshold.
         */
-       if (HeapTupleHasExtended(&oldtup) || 
-               HeapTupleHasExtended(newtup) ||
-               (MAXALIGN(newtup->t_len) > TOAST_TUPLE_THRESHOLD))
-               heap_tuple_toast_attrs(relation, newtup, &oldtup);
-#endif
+       need_toast = (HeapTupleHasExtended(&oldtup) || 
+                                 HeapTupleHasExtended(newtup) ||
+                                 (MAXALIGN(newtup->t_len) > TOAST_TUPLE_THRESHOLD));
 
-       /* Find buffer for new tuple */
-
-       if ((unsigned) MAXALIGN(newtup->t_len) <= PageGetFreeSpace((Page) dp))
-               newbuf = buffer;
-       else
+       if (need_toast ||
+               (unsigned) MAXALIGN(newtup->t_len) > PageGetFreeSpace((Page) dp))
        {
-               /* 
-                * We have to unlock old tuple buffer before extending table
-                * file but have to keep lock on the old tuple. To avoid second
-                * XLOG log record we use xact mngr hook to unlock old tuple
-                * without reading log if xact will abort before update is logged.
-                * In the event of crash prio logging, TQUAL routines will see
-                * HEAP_XMAX_UNLOGGED flag...
-                *
-                * NOTE: this trick is useless currently but saved for future
-                * when we'll implement UNDO and will re-use transaction IDs
-                * after postmaster startup.
-                */
                _locked_tuple_.node = relation->rd_node;
                _locked_tuple_.tid = oldtup.t_self;
                XactPushRollback(_heap_unlock_tuple, (void*) &_locked_tuple_);
 
-               TransactionIdStore(GetCurrentTransactionId(), &(oldtup.t_data->t_xmax));
+               TransactionIdStore(GetCurrentTransactionId(),
+                                                  &(oldtup.t_data->t_xmax));
                oldtup.t_data->t_cmax = GetCurrentCommandId();
                oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
-                                                                HEAP_XMAX_INVALID | HEAP_MARKED_FOR_UPDATE);
+                                                                          HEAP_XMAX_INVALID |
+                                                                          HEAP_MARKED_FOR_UPDATE);
                oldtup.t_data->t_infomask |= HEAP_XMAX_UNLOGGED;
+               already_marked = true;
                LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
-               newbuf = RelationGetBufferForTuple(relation, newtup->t_len);
+
+               /* Let the toaster do its thing */
+               if (need_toast)
+                       heap_tuple_toast_attrs(relation, newtup, &oldtup);
+
+               /* Now, do we need a new page for the tuple, or not? */
+               if ((unsigned) MAXALIGN(newtup->t_len) <= PageGetFreeSpace((Page) dp))
+                       newbuf = buffer;
+               else
+                       newbuf = RelationGetBufferForTuple(relation, newtup->t_len);
+
+               /* Re-acquire the lock on the old tuple's page. */
                /* this seems to be deadlock free... */
                LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
        }
+       else
+       {
+               /* No TOAST work needed, and it'll fit on same page */
+               already_marked = false;
+               newbuf = buffer;
+       }
 
        /* NO ELOG(ERROR) from here till changes are logged */
        START_CRIT_SECTION();
 
        RelationPutHeapTuple(relation, newbuf, newtup); /* insert new tuple */
-       if (buffer == newbuf)
+
+       if (already_marked)
        {
-               TransactionIdStore(GetCurrentTransactionId(), &(oldtup.t_data->t_xmax));
-               oldtup.t_data->t_cmax = GetCurrentCommandId();
-               oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
-                                                                HEAP_XMAX_INVALID | HEAP_MARKED_FOR_UPDATE);
+               oldtup.t_data->t_infomask &= ~HEAP_XMAX_UNLOGGED;
+               XactPopRollback();
        }
        else
        {
-               oldtup.t_data->t_infomask &= ~HEAP_XMAX_UNLOGGED;
-               XactPopRollback();
+               TransactionIdStore(GetCurrentTransactionId(),
+                                                  &(oldtup.t_data->t_xmax));
+               oldtup.t_data->t_cmax = GetCurrentCommandId();
+               oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
+                                                                          HEAP_XMAX_INVALID |
+                                                                          HEAP_MARKED_FOR_UPDATE);
        }
 
        /* record address of new tuple in t_ctid of old one */
@@ -1724,7 +1744,7 @@ l2:
        /* XLOG stuff */
        {
                XLogRecPtr      recptr = log_heap_update(relation, buffer, oldtup.t_self, 
-                                                                                               newbuf, newtup, false);
+                                                                                        newbuf, newtup, false);
 
                if (newbuf != buffer)
                {
@@ -1734,6 +1754,7 @@ l2:
                PageSetLSN(BufferGetPage(buffer), recptr);
                PageSetSUI(BufferGetPage(buffer), ThisStartUpID);
        }
+
        END_CRIT_SECTION();
 
        if (newbuf != buffer)

diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 2787611dd01d7ee820d28c73383379b978e0bdfd..46495404b8148f2155cdcd619179e540ea37f99a 100644 (file)
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/heap/tuptoaster.c,v 1.13 2000/10/23 23:42:04 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/heap/tuptoaster.c,v 1.14 2001/01/15 05:29:19 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -197,10 +197,12 @@ toast_delete(Relation rel, HeapTuple oldtup)
         */
        for (i = 0; i < numAttrs; i++)
        {
-               value = heap_getattr(oldtup, i + 1, tupleDesc, &isnull);
-               if (!isnull && att[i]->attlen == -1)
-                       if (VARATT_IS_EXTERNAL(value))
+               if (att[i]->attlen == -1)
+               {
+                       value = heap_getattr(oldtup, i + 1, tupleDesc, &isnull);
+                       if (!isnull && VARATT_IS_EXTERNAL(value))
                                toast_delete_datum(rel, value);
+               }
        }
 }