While trying to grok heap_update I came again across the
_heap_unlock_tuple function.  This code apparently tries to save a XLog
round while trying to mark a tuple for update, by registering a
"rollback callback", which would unmark the tuple in case the
transaction is rolled back.

Turns out the callback is never called at all.  So the code is dead
code.

Also, it claims that by marking t_infomask with a special 
HEAP_XMAX_UNLOGGED bit, this trick would not suffer across a system
crash, because tqual routines would check this bit (comments in the code
don't specify what would the routines do with it).

Turns out tqual routines never check the bit.  In fact, the bit is never
checked at all, nowhere in the code: heap_update sets it and then
unsets it, but that's all.

So I think this is dead code.  The attached patch removes it.

-- 
Alvaro Herrera (<[EMAIL PROTECTED]>)
Maybe there's lots of data loss but the records of data loss are also lost.
(Lincoln Yeoh)
Index: backend/access/heap/heapam.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/heap/heapam.c,v
retrieving revision 1.184
diff -c -r1.184 heapam.c
*** backend/access/heap/heapam.c        20 Mar 2005 23:40:23 -0000      1.184
--- backend/access/heap/heapam.c        27 Mar 2005 19:19:34 -0000
***************
*** 53,61 ****
  #include "pgstat.h"
  
  
- /* comments are in heap_update */
- static xl_heaptid _locked_tuple_;
- static void _heap_unlock_tuple(void *data);
  static XLogRecPtr log_heap_update(Relation reln, Buffer oldbuf,
           ItemPointerData from, Buffer newbuf, HeapTuple newtup, bool move);
  
--- 53,58 ----
***************
*** 1603,1617 ****
         * 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 out-of-line
         * toasted values present, or if the new tuple is over-threshold.
--- 1600,1606 ----
         * 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.
         *
         * We need to invoke the toaster if there are already any out-of-line
         * toasted values present, or if the new tuple is over-threshold.
***************
*** 1625,1639 ****
  
        if (need_toast || newtupsize > pagefree)
        {
-               _locked_tuple_.node = relation->rd_node;
-               _locked_tuple_.tid = oldtup.t_self;
-               XactPushRollback(_heap_unlock_tuple, (void *) &_locked_tuple_);
- 
                oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
                                                                           
HEAP_XMAX_INVALID |
                                                                           
HEAP_MARKED_FOR_UPDATE |
                                                                           
HEAP_MOVED);
-               oldtup.t_data->t_infomask |= HEAP_XMAX_UNLOGGED;
                HeapTupleHeaderSetXmax(oldtup.t_data, xid);
                HeapTupleHeaderSetCmax(oldtup.t_data, cid);
                already_marked = true;
--- 1614,1623 ----
***************
*** 1714,1725 ****
  
        RelationPutHeapTuple(relation, newbuf, newtup);         /* insert new 
tuple */
  
!       if (already_marked)
!       {
!               oldtup.t_data->t_infomask &= ~HEAP_XMAX_UNLOGGED;
!               XactPopRollback();
!       }
!       else
        {
                oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
                                                                           
HEAP_XMAX_INVALID |
--- 1698,1704 ----
  
        RelationPutHeapTuple(relation, newbuf, newtup);         /* insert new 
tuple */
  
!       if (!already_marked)
        {
                oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
                                                                           
HEAP_XMAX_INVALID |
***************
*** 2568,2615 ****
  
  }
  
- static void
- _heap_unlock_tuple(void *data)
- {
-       TransactionId xid = GetCurrentTransactionId();
-       xl_heaptid *xltid = (xl_heaptid *) data;
-       Relation        reln = XLogOpenRelation(false, RM_HEAP_ID, xltid->node);
-       Buffer          buffer;
-       Page            page;
-       OffsetNumber offnum;
-       ItemId          lp;
-       HeapTupleHeader htup;
- 
-       if (!RelationIsValid(reln))
-               elog(PANIC, "_heap_unlock_tuple: can't open relation");
- 
-       buffer = XLogReadBuffer(false, reln,
-                                                       
ItemPointerGetBlockNumber(&(xltid->tid)));
-       if (!BufferIsValid(buffer))
-               elog(PANIC, "_heap_unlock_tuple: can't read buffer");
- 
-       page = (Page) BufferGetPage(buffer);
-       if (PageIsNew((PageHeader) page))
-               elog(PANIC, "_heap_unlock_tuple: uninitialized page");
- 
-       offnum = ItemPointerGetOffsetNumber(&(xltid->tid));
-       if (offnum > PageGetMaxOffsetNumber(page))
-               elog(PANIC, "_heap_unlock_tuple: invalid itemid");
-       lp = PageGetItemId(page, offnum);
- 
-       if (!ItemIdIsUsed(lp) || ItemIdDeleted(lp))
-               elog(PANIC, "_heap_unlock_tuple: unused/deleted tuple in 
rollback");
- 
-       htup = (HeapTupleHeader) PageGetItem(page, lp);
- 
-       if (!TransactionIdEquals(HeapTupleHeaderGetXmax(htup), xid))
-               elog(PANIC, "_heap_unlock_tuple: invalid xmax in rollback");
-       htup->t_infomask &= ~HEAP_XMAX_UNLOGGED;
-       htup->t_infomask |= HEAP_XMAX_INVALID;
-       LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
-       WriteBuffer(buffer);
- }
- 
  void
  heap_redo(XLogRecPtr lsn, XLogRecord *record)
  {
--- 2547,2552 ----
Index: backend/access/transam/xact.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.197
diff -c -r1.197 xact.c
*** backend/access/transam/xact.c       20 Feb 2005 21:46:48 -0000      1.197
--- backend/access/transam/xact.c       27 Mar 2005 18:59:07 -0000
***************
*** 196,205 ****
  
  static SubXactCallbackItem *SubXact_callbacks = NULL;
  
- static void (*_RollbackFunc) (void *) = NULL;
- static void *_RollbackData = NULL;
- 
- 
  /* local function prototypes */
  static void AssignSubTransactionId(TransactionState s);
  static void AbortTransaction(void);
--- 196,201 ----
***************
*** 3902,3922 ****
        else
                strcat(buf, "UNKNOWN");
  }
- 
- void
-                       XactPushRollback(void (*func) (void *), void *data)
- {
- #ifdef XLOG_II
-       if (_RollbackFunc != NULL)
-               elog(PANIC, "XactPushRollback: already installed");
- #endif
- 
-       _RollbackFunc = func;
-       _RollbackData = data;
- }
- 
- void
- XactPopRollback(void)
- {
-       _RollbackFunc = NULL;
- }
--- 3898,3900 ----
Index: include/access/xact.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/access/xact.h,v
retrieving revision 1.74
diff -c -r1.74 xact.h
*** include/access/xact.h       31 Dec 2004 22:03:21 -0000      1.74
--- include/access/xact.h       27 Mar 2005 18:58:40 -0000
***************
*** 145,153 ****
  
  extern int    xactGetCommittedChildren(TransactionId **ptr);
  
- extern void XactPushRollback(void (*func) (void *), void *data);
- extern void XactPopRollback(void);
- 
  extern void xact_redo(XLogRecPtr lsn, XLogRecord *record);
  extern void xact_undo(XLogRecPtr lsn, XLogRecord *record);
  extern void xact_desc(char *buf, uint8 xl_info, char *rec);
--- 145,150 ----
Index: include/access/htup.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/access/htup.h,v
retrieving revision 1.72
diff -c -r1.72 htup.h
*** include/access/htup.h       31 Dec 2004 22:03:21 -0000      1.72
--- include/access/htup.h       27 Mar 2005 19:00:37 -0000
***************
*** 152,160 ****
                                                                                
 * attribute(s) */
  #define HEAP_HASEXTENDED              0x000C  /* the two above combined */
  #define HEAP_HASOID                           0x0010  /* has an object-id 
field */
! /* 0x0020 and 0x0040 are unused */
! #define HEAP_XMAX_UNLOGGED            0x0080  /* to lock tuple for update
!                                                                               
 * without logging */
  #define HEAP_XMIN_COMMITTED           0x0100  /* t_xmin committed */
  #define HEAP_XMIN_INVALID             0x0200  /* t_xmin invalid/aborted */
  #define HEAP_XMAX_COMMITTED           0x0400  /* t_xmax committed */
--- 152,158 ----
                                                                                
 * attribute(s) */
  #define HEAP_HASEXTENDED              0x000C  /* the two above combined */
  #define HEAP_HASOID                           0x0010  /* has an object-id 
field */
! /* 0x0020, 0x0040 and 0x080 are unused */
  #define HEAP_XMIN_COMMITTED           0x0100  /* t_xmin committed */
  #define HEAP_XMIN_INVALID             0x0200  /* t_xmin invalid/aborted */
  #define HEAP_XMAX_COMMITTED           0x0400  /* t_xmax committed */
---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
    (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])

Reply via email to