On Sun, Jun 20, 2004 at 08:49:22PM -0400, Tom Lane wrote:

> There's a good deal more than that missing :-(.  Here are the modules or
> actions that are called in CommitTransaction and/or AbortTransaction
> that have not yet been touched by the patch:
> 
> localbuf.c (refcounts need fixed same as bufmgr)

Here is a patch against the original versions of these files; cleaned up
bufmgr.c somewhat.  Adds the same logic to local buffers (moving the
BufferRefCount struct declaration to buf_internals.h so it's shared by
both bufmgr.c and localbuf.c).  Needs xact.c and xact.h patched as in
the second patch.

As with the bufmgr.c original patch, I don't really know how to test
that this actually works.  I fooled around with printing what it was
doing during a subtrans commit/abort, and it seems OK, but that's about
it.  In what situations can a transaction roll back with a nonzero
reference count in a local buffer?

-- 
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"I dream about dreams about dreams", sang the nightingale
under the pale moon (Sandman)
Index: src/include/storage/bufmgr.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/include/storage/bufmgr.h,v
retrieving revision 1.82
diff -c -r1.82 bufmgr.h
*** src/include/storage/bufmgr.h        31 May 2004 19:24:05 -0000      1.82
--- src/include/storage/bufmgr.h        21 Jun 2004 20:29:08 -0000
***************
*** 148,153 ****
--- 148,155 ----
  extern char *ShowBufferUsage(void);
  extern void ResetBufferUsage(void);
  extern void AtEOXact_Buffers(bool isCommit);
+ extern void AtSubStart_Buffers(void);
+ extern void AtEOSubXact_Buffers(bool commit);
  extern void FlushBufferPool(void);
  extern BlockNumber BufferGetBlockNumber(Buffer buffer);
  extern BlockNumber RelationGetNumberOfBlocks(Relation relation);
Index: src/include/storage/buf_internals.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/include/storage/buf_internals.h,v
retrieving revision 1.71
diff -c -r1.71 buf_internals.h
*** src/include/storage/buf_internals.h 18 Jun 2004 06:14:13 -0000      1.71
--- src/include/storage/buf_internals.h 29 Jun 2004 13:21:23 -0000
***************
*** 175,180 ****
--- 175,189 ----
  extern long int BufferFlushCount;
  extern long int LocalBufferFlushCount;
  
+ /*
+  * We use a list of this struct to keep track of buffer reference
+  * count checking at subtransaction boundaries.
+  */
+ typedef struct BufferRefCount 
+ {
+       Buffer  buffer;
+       int             refcount;
+ } BufferRefCount;
  
  /*
   * Bufmgr Interface:
***************
*** 211,215 ****
--- 220,226 ----
                                 bool *foundPtr);
  extern void WriteLocalBuffer(Buffer buffer, bool release);
  extern void AtEOXact_LocalBuffers(bool isCommit);
+ extern void AtSubStart_LocalBuffers(void);
+ extern void AtSubEnd_LocalBuffers(bool isCommit);
  
  #endif   /* BUFMGR_INTERNALS_H */
Index: src/backend/storage/buffer/bufmgr.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/backend/storage/buffer/bufmgr.c,v
retrieving revision 1.171
diff -c -r1.171 bufmgr.c
*** src/backend/storage/buffer/bufmgr.c 18 Jun 2004 06:13:33 -0000      1.171
--- src/backend/storage/buffer/bufmgr.c 29 Jun 2004 20:26:09 -0000
***************
*** 38,43 ****
--- 38,44 ----
  #include <sys/file.h>
  #include <unistd.h>
  
+ #include "access/xact.h"
  #include "lib/stringinfo.h"
  #include "miscadmin.h"
  #include "storage/buf_internals.h"
***************
*** 46,51 ****
--- 47,53 ----
  #include "storage/proc.h"
  #include "storage/smgr.h"
  #include "utils/relcache.h"
+ #include "utils/memutils.h"
  #include "pgstat.h"
  
  
***************
*** 67,72 ****
--- 69,75 ----
  
  static void PinBuffer(BufferDesc *buf);
  static void UnpinBuffer(BufferDesc *buf);
+ static inline void BufferFixLeak(Buffer buffer, int should, bool warn);
  static void WaitIO(BufferDesc *buf);
  static void StartBufferIO(BufferDesc *buf, bool forInput);
  static void TerminateBufferIO(BufferDesc *buf, int err_flag);
***************
*** 826,853 ****
        for (i = 0; i < NBuffers; i++)
        {
                if (PrivateRefCount[i] != 0)
!               {
!                       BufferDesc *buf = &(BufferDescriptors[i]);
  
!                       if (isCommit)
!                               elog(WARNING,
!                                        "buffer refcount leak: [%03d] "
!                                        "(rel=%u/%u/%u, blockNum=%u, flags=0x%x, 
refcount=%u %d)",
!                                        i,
!                                        buf->tag.rnode.spcNode, buf->tag.rnode.dbNode,
!                                        buf->tag.rnode.relNode,
!                                        buf->tag.blockNum, buf->flags,
!                                        buf->refcount, PrivateRefCount[i]);
  
!                       PrivateRefCount[i] = 1;         /* make sure we release shared 
pin */
!                       LWLockAcquire(BufMgrLock, LW_EXCLUSIVE);
!                       UnpinBuffer(buf);
!                       LWLockRelease(BufMgrLock);
!                       Assert(PrivateRefCount[i] == 0);
                }
        }
  
!       AtEOXact_LocalBuffers(isCommit);
  }
  
  /*
--- 829,958 ----
        for (i = 0; i < NBuffers; i++)
        {
                if (PrivateRefCount[i] != 0)
!                       BufferFixLeak(i, 0, isCommit);
!       }
  
!       AtEOXact_LocalBuffers(isCommit);
! }
  
! /*
!  * During subtransaction start, save buffer reference counts
!  * that are nonzero.
!  */
! void
! AtSubStart_Buffers(void)
! {
!       List               *refcounts = NIL;
!       MemoryContext   old_cxt;
!       int                             i;
! 
!       /*
!        * XXX It would be better to have a per-subxact memory context
!        * where we could keep this things.  We don't have one ATM
!        * so we free them by hand afterwards.
!        */
!       old_cxt = MemoryContextSwitchTo(TopTransactionContext);
! 
!       for (i = 0; i < NBuffers; i++)
!       {
!               BufferRefCount *brc;
! 
!               if (PrivateRefCount[i] == 0)
!                       continue;
! 
!               brc = (BufferRefCount *) palloc(sizeof(BufferRefCount));
!               brc->buffer = i;
!               brc->refcount = PrivateRefCount[i];
! 
!               refcounts = lappend(refcounts, brc);
!       }
! 
!       TransactionSetBufferRefcounts(refcounts);
! 
!       MemoryContextSwitchTo(old_cxt);
! 
!       AtSubStart_LocalBuffers();
! }
! 
! /*
!  * AtEOSubXact_Buffers
!  *
!  * At subtransaction end, we restore the saved counts.  If committing, we croak
!  * if the refcounts don't match; if aborting, just restore silently.
!  *
!  * This only works because we know the list is in ascending order.
!  */
! void
! AtEOSubXact_Buffers(bool commit)
! {
!       List       *counts = TransactionGetBufferRefcounts();
!       ListCell   *nextElt;
!       int                     i, j;
!       Buffer          nextSaved;
! 
!       nextElt   = list_head(counts);
! 
!       for (i = 0; i < NBuffers; i++)
!       {
!               BufferRefCount     *brc = NULL;
! 
!               if (nextElt)
!               {
!                       brc = (BufferRefCount *) lfirst(nextElt);
!                       nextSaved = brc->buffer;
                }
+               else
+                       nextSaved = NBuffers;
+ 
+               /*
+                * Buffers that are not on the saved list are supposed
+                * to have refcount 0.  If they do not, unpin them.
+                * In the commit case, emit a message.
+                */
+               for (j = i; j < nextSaved; j++, i++)
+                       if (PrivateRefCount[j] != 0)
+                               BufferFixLeak(j, 0, commit);
+ 
+               /* No more buffers to check. */
+               if (nextSaved == NBuffers)
+                       break;
+ 
+               if (PrivateRefCount[i] != brc->refcount)
+                       BufferFixLeak(i, brc->refcount, commit);
+ 
+               if (nextElt)
+                       nextElt = nextElt->next;
        }
  
!       /* Free the whole list, including the elements. */
!       list_free_deep(counts);
! 
!       AtSubEnd_LocalBuffers(commit);
! }
! 
! /*
!  * Fix a buffer refcount leak.
!  */
! inline void
! BufferFixLeak(Buffer buffer, int should, bool emitWarning)
! {
!       BufferDesc      *buf = &(BufferDescriptors[buffer]);
! 
!       if (emitWarning)
!               elog(WARNING,
!                               "buffer refcount leak: [%03d] "
!                               "(rel=%u/%u/%u, blockNum=%u, flags=0x%x, refcount=%u 
%d, should be=%d)",
!                               buffer,
!                               buf->tag.rnode.spcNode, buf->tag.rnode.dbNode,
!                               buf->tag.rnode.relNode,
!                               buf->tag.blockNum, buf->flags,
!                               buf->refcount, PrivateRefCount[buffer], should);
! 
!       PrivateRefCount[buffer] = 1;            /* make sure we release shared pin */
!       LWLockAcquire(BufMgrLock, LW_EXCLUSIVE);
!       UnpinBuffer(buf);
!       LWLockRelease(BufMgrLock);
!       Assert(PrivateRefCount[buffer] == 0);
  }
  
  /*
Index: src/backend/storage/buffer/localbuf.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/backend/storage/buffer/localbuf.c,v
retrieving revision 1.56
diff -c -r1.56 localbuf.c
*** src/backend/storage/buffer/localbuf.c       18 Jun 2004 06:13:33 -0000      1.56
--- src/backend/storage/buffer/localbuf.c       29 Jun 2004 13:21:39 -0000
***************
*** 15,23 ****
--- 15,25 ----
   */
  #include "postgres.h"
  
+ #include "access/xact.h"
  #include "storage/buf_internals.h"
  #include "storage/bufmgr.h"
  #include "storage/smgr.h"
+ #include "utils/memutils.h"
  #include "utils/relcache.h"
  
  
***************
*** 32,37 ****
--- 34,40 ----
  
  static int    nextFreeLocalBuf = 0;
  
+ static inline void LocalBufferFixLeak(Buffer buffer, int should, bool emitWarning);
  
  /*
   * LocalBufferAlloc -
***************
*** 229,248 ****
        int                     i;
  
        for (i = 0; i < NLocBuffer; i++)
-       {
                if (LocalRefCount[i] != 0)
!               {
!                       BufferDesc *buf = &(LocalBufferDescriptors[i]);
  
!                       if (isCommit)
!                               elog(WARNING,
!                                        "local buffer leak: [%03d] (rel=%u/%u/%u, 
blockNum=%u, flags=0x%x, refcount=%u %d)",
!                                        i,
!                                        buf->tag.rnode.spcNode, buf->tag.rnode.dbNode,
!                                        buf->tag.rnode.relNode, buf->tag.blockNum, 
buf->flags,
!                                        buf->refcount, LocalRefCount[i]);
  
!                       LocalRefCount[i] = 0;
                }
        }
  }
--- 232,343 ----
        int                     i;
  
        for (i = 0; i < NLocBuffer; i++)
                if (LocalRefCount[i] != 0)
!                       LocalBufferFixLeak(i, 0, isCommit);
! }
! 
! inline void
! LocalBufferFixLeak(Buffer buffer, int should, bool emitWarning)
! {
!       BufferDesc *buf = &(LocalBufferDescriptors[buffer]);
! 
!       if (emitWarning)
!               elog(WARNING,
!                        "local buffer leak: [%03d] (rel=%u/%u/%u, blockNum=%u, 
flags=0x%x, refcount=%u %d should be=%d)",
!                        buffer,
!                        buf->tag.rnode.spcNode, buf->tag.rnode.dbNode,
!                        buf->tag.rnode.relNode, buf->tag.blockNum, buf->flags,
!                        buf->refcount, LocalRefCount[buffer], should);
! 
!       LocalRefCount[buffer] = should;
! }
! 
! 
! /*
!  * AtSubStart_LocalBuffers
!  * 
!  * Save reference counts for local buffers.
!  */
! void
! AtSubStart_LocalBuffers(void)
! {
!       MemoryContext old_cxt;
!       List   *refcounts = NIL;
!       int             i;
! 
!       old_cxt = MemoryContextSwitchTo(TopTransactionContext);
! 
!       for (i = 0; i < NLocBuffer; i++)
!       {
!               BufferRefCount  *brc;
  
!               if (LocalRefCount[i] == 0)
!                       continue;
! 
!               brc = (BufferRefCount *) palloc(sizeof(BufferRefCount));
!               brc->buffer = i;
!               brc->refcount = LocalRefCount[i];
! 
!               refcounts = lappend(refcounts, brc);
!       }
! 
!       TransactionSetLocalBufferRefcounts(refcounts);
! 
!       MemoryContextSwitchTo(old_cxt);
! }
  
! /*
!  * AtSubEnd_LocalBuffers
!  *
!  * Restore (and possible croak about) local buffer reference counts
!  * at subtransaction end.
!  */
! void
! AtSubEnd_LocalBuffers(bool commit)
! {
!       List       *counts = TransactionGetLocalBufferRefcounts();
!       ListCell   *nextElt;
!       int                     i, j;
!       Buffer          nextSaved;
! 
!       nextElt = list_head(counts);
! 
!       for (i = 0; i < NLocBuffer; i++)
!       {
!               BufferRefCount  *brc = NULL;
! 
!               /*
!                * If there is a next element in this list, skip
!                * all local buffers until that element.  If not,
!                * skip until the end of the local buffer array.
!                */
!               if (nextElt)
!               {
!                       brc = (BufferRefCount *) lfirst(nextElt);
!                       nextSaved = brc->buffer;
                }
+               else
+                       nextSaved = NLocBuffer;
+ 
+               /*
+                * Buffers not in the list should have a refcount of 0.
+                */
+               for (j = i; j < nextSaved; j++, i++)
+                       if (LocalRefCount[j] != 0)
+                               LocalBufferFixLeak(j, 0, commit);
+ 
+               /*
+                * Stop here if we are at the end of the array.
+                */
+               if (nextSaved == NLocBuffer)
+                       break;
+ 
+               if (LocalRefCount[i] != brc->refcount)
+                       LocalBufferFixLeak(i, brc->refcount, commit);
+ 
+               if (nextElt)
+                       nextElt = nextElt->next;
        }
+ 
+       list_free_deep(counts);
  }
*** src/backend/access/transam/xact.c~  2004-06-29 16:38:23.468928928 -0400
--- src/backend/access/transam/xact.c   2004-06-29 16:38:29.306995811 -0400
***************
*** 230,235 ****
--- 230,236 ----
        NIL,                                            /* async notifies */
        NIL,                                            /* committed child Xids */
        NIL,                                            /* bufmgr refcounts */
+       NIL,                                            /* local bufmgr refcounts */
        NIL,                                            /* GUC variables */
        NULL,                                           /* deferred triggers state */
        0,                                                      /* XLog record offset 
*/
***************
*** 499,504 ****
--- 500,527 ----
  }
  
  /*
+  * TransactionSetLocalBufferRefcounts
+  */
+ void
+ TransactionSetLocalBufferRefcounts(List *counts)
+ {
+       TransactionState s = CurrentTransactionState;
+ 
+       s->localBufRefCnts = counts;
+ }
+ 
+ /*
+  * TransactionGetLocalBufferRefcounts
+  */
+ List *
+ TransactionGetLocalBufferRefcounts(void)
+ {
+       TransactionState s = CurrentTransactionState;
+ 
+       return s->localBufRefCnts;
+ }
+ 
+ /*
   * TransactionSaveGucVar
   *
   * This is called from GUC, so be sure to use the right MemoryContext.
*** src/include/access/xact.h~  2004-06-29 16:39:24.256213104 -0400
--- src/include/access/xact.h   2004-06-29 16:39:27.125754456 -0400
***************
*** 97,102 ****
--- 97,103 ----
        List               *notifies;
        List               *childXids;
        List               *bufRefCnts;
+       List               *localBufRefCnts;
        List               *GUCvars;
        void               *deftrigState;
        uint32                  XLogRecOff;
***************
*** 165,170 ****
--- 166,173 ----
  extern void TransactionSetParentNotifies(List *);
  extern List *TransactionGetBufferRefcounts(void);
  extern void TransactionSetBufferRefcounts(List *);
+ extern List *TransactionGetLocalBufferRefcounts(void);
+ extern void TransactionSetLocalBufferRefcounts(List *);
  extern void TransactionSaveGucVar(void *);
  extern void TransactionSetDeftrigState(void *);
  extern void *TransactionGetDeftrigState(void);
---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

               http://www.postgresql.org/docs/faqs/FAQ.html

Reply via email to