]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Give the ResourceOwner mechanism full responsibility for releasing buffer
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 16 Oct 2004 18:57:26 +0000 (18:57 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 16 Oct 2004 18:57:26 +0000 (18:57 +0000)
pins at end of transaction, and reduce AtEOXact_Buffers to an Assert
cross-check that this was done correctly.  When not USE_ASSERT_CHECKING,
AtEOXact_Buffers is a complete no-op.  This gets rid of an O(NBuffers)
bottleneck during transaction commit/abort, which recent testing has shown
becomes significant above a few tens of thousands of shared buffers.

src/backend/access/transam/xact.c
src/backend/storage/buffer/bufmgr.c
src/backend/storage/buffer/localbuf.c
src/backend/storage/lmgr/proc.c
src/backend/utils/resowner/resowner.c
src/include/storage/bufmgr.h

index 321a86f30c22f6c33ac05098d603ba19330771b3..7a6fd3a132320552f6edf9c6dc25e2990f0651c2 100644 (file)
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.191 2004/10/04 21:52:14 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.192 2004/10/16 18:57:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1528,6 +1528,9 @@ CommitTransaction(void)
                                                 RESOURCE_RELEASE_BEFORE_LOCKS,
                                                 true, true);
 
+       /* Check we've released all buffer pins */
+       AtEOXact_Buffers(true);
+
        /*
         * Make catalog changes visible to all backends.  This has to happen
         * after relcache references are dropped (see comments for
@@ -1684,6 +1687,7 @@ AbortTransaction(void)
        ResourceOwnerRelease(TopTransactionResourceOwner,
                                                 RESOURCE_RELEASE_BEFORE_LOCKS,
                                                 false, true);
+       AtEOXact_Buffers(false);
        AtEOXact_Inval(false);
        smgrDoPendingDeletes(false);
        ResourceOwnerRelease(TopTransactionResourceOwner,
index a8cf506688fcd22f3e12390e489110008580d540..b9d8fc3ad53d06746d5f7071732f949422660771 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.179 2004/10/16 18:05:06 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.180 2004/10/16 18:57:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -851,35 +851,45 @@ ResetBufferUsage(void)
 /*
  *             AtEOXact_Buffers - clean up at end of transaction.
  *
- *             During abort, we need to release any buffer pins we're holding
- *             (this cleans up in case ereport interrupted a routine that pins a
- *             buffer).  During commit, we shouldn't need to do that, but check
- *             anyway to see if anyone leaked a buffer reference count.
+ *             As of PostgreSQL 8.0, buffer pins should get released by the
+ *             ResourceOwner mechanism.  This routine is just a debugging
+ *             cross-check that no pins remain.
  */
 void
 AtEOXact_Buffers(bool isCommit)
 {
+#ifdef USE_ASSERT_CHECKING
        int                     i;
 
+       for (i = 0; i < NBuffers; i++)
+       {
+               Assert(PrivateRefCount[i] == 0);
+       }
+
+       AtEOXact_LocalBuffers(isCommit);
+#endif
+}
+
+/*
+ * Ensure we have released all shared-buffer locks and pins during backend exit
+ */
+void
+AtProcExit_Buffers(void)
+{
+       int                     i;
+
+       AbortBufferIO();
+       UnlockBuffers();
+
        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]);
-
                        /*
-                        * We don't worry about updating the ResourceOwner structures;
-                        * resowner.c will clear them for itself.
+                        * We don't worry about updating ResourceOwner; if we even got
+                        * here, it suggests that ResourceOwners are messed up.
                         */
                        PrivateRefCount[i] = 1;         /* make sure we release shared pin */
                        LWLockAcquire(BufMgrLock, LW_EXCLUSIVE);
@@ -888,8 +898,37 @@ AtEOXact_Buffers(bool isCommit)
                        Assert(PrivateRefCount[i] == 0);
                }
        }
+}
 
-       AtEOXact_LocalBuffers(isCommit);
+/*
+ * Helper routine to issue warnings when a buffer is unexpectedly pinned
+ */
+void
+PrintBufferLeakWarning(Buffer buffer)
+{
+       BufferDesc *buf;
+       int32           loccount;
+
+       Assert(BufferIsValid(buffer));
+       if (BufferIsLocal(buffer))
+       {
+               buf = &LocalBufferDescriptors[-buffer - 1];
+               loccount = LocalRefCount[-buffer - 1];
+       }
+       else
+       {
+               buf = &BufferDescriptors[buffer - 1];
+               loccount = PrivateRefCount[buffer - 1];
+       }
+
+       elog(WARNING,
+                "buffer refcount leak: [%03d] "
+                "(rel=%u/%u/%u, blockNum=%u, flags=0x%x, refcount=%u %d)",
+                buffer,
+                buf->tag.rnode.spcNode, buf->tag.rnode.dbNode,
+                buf->tag.rnode.relNode,
+                buf->tag.blockNum, buf->flags,
+                buf->refcount, loccount);
 }
 
 /*
index 6ccc18ddad352c8935c475684f7302854d80e3a9..4df6befcacf727e24b7e0c801534263889895184 100644 (file)
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/storage/buffer/localbuf.c,v 1.59 2004/08/29 05:06:47 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/buffer/localbuf.c,v 1.60 2004/10/16 18:57:24 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -232,23 +232,12 @@ InitLocalBuffer(void)
 void
 AtEOXact_LocalBuffers(bool isCommit)
 {
+#ifdef USE_ASSERT_CHECKING
        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;
-               }
+               Assert(LocalRefCount[i] == 0);
        }
+#endif
 }
index ac04b5e80423493b073e6a5cca6022d7391dd9fc..e50ed2f08fe68b260f01658394aae95454000392 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/storage/lmgr/proc.c,v 1.154 2004/09/29 15:15:55 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/lmgr/proc.c,v 1.155 2004/10/16 18:57:24 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -461,9 +461,7 @@ ProcKill(int code, Datum arg)
         * shutdown callback registered by the bufmgr ... but we must do this
         * *after* LWLockReleaseAll and *before* zapping MyProc.
         */
-       AbortBufferIO();
-       UnlockBuffers();
-       AtEOXact_Buffers(false);
+       AtProcExit_Buffers();
 
        /* Get off any wait queue I might be on */
        LockWaitCancel();
@@ -509,9 +507,7 @@ DummyProcKill(int code, Datum arg)
        LWLockReleaseAll();
 
        /* Release buffer locks and pins, too */
-       AbortBufferIO();
-       UnlockBuffers();
-       AtEOXact_Buffers(false);
+       AtProcExit_Buffers();
 
        /* I can't be on regular lock queues, so needn't check */
 
index 4075e2c45ea5466ea01f0a21c89b5d7c5796ff7a..293cb31a26cb6e966cd4b21b9885881b08f93cfa 100644 (file)
@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.7 2004/08/30 02:54:40 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.8 2004/10/16 18:57:25 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -191,37 +191,30 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 
        if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
        {
-               /* Release buffer pins */
-               if (isTopLevel)
-               {
-                       /*
-                        * For a top-level xact we are going to release all buffers,
-                        * so just do a single bufmgr call at the top of the
-                        * recursion.
-                        */
-                       if (owner == TopTransactionResourceOwner)
-                               AtEOXact_Buffers(isCommit);
-                       /* Mark object as owning no buffers, just for sanity */
-                       owner->nbuffers = 0;
-               }
-               else
+               /*
+                * Release buffer pins.  Note that ReleaseBuffer will
+                * remove the buffer entry from my list, so I just have to
+                * iterate till there are none.
+                *
+                * During a commit, there shouldn't be any remaining pins ---
+                * that would indicate failure to clean up the executor correctly ---
+                * so issue warnings.  In the abort case, just clean up quietly.
+                *
+                * XXX this is fairly inefficient due to multiple BufMgrLock
+                * grabs if there are lots of buffers to be released, but we
+                * don't expect many (indeed none in the success case) so it's
+                * probably not worth optimizing.
+                *
+                * We are however careful to release back-to-front, so as to
+                * avoid O(N^2) behavior in ResourceOwnerForgetBuffer().
+                */
+               while (owner->nbuffers > 0)
                {
-                       /*
-                        * Release buffers retail.      Note that ReleaseBuffer will
-                        * remove the buffer entry from my list, so I just have to
-                        * iterate till there are none.
-                        *
-                        * XXX this is fairly inefficient due to multiple BufMgrLock
-                        * grabs if there are lots of buffers to be released, but we
-                        * don't expect many (indeed none in the success case) so it's
-                        * probably not worth optimizing.
-                        *
-                        * We are however careful to release back-to-front, so as to
-                        * avoid O(N^2) behavior in ResourceOwnerForgetBuffer().
-                        */
-                       while (owner->nbuffers > 0)
-                               ReleaseBuffer(owner->buffers[owner->nbuffers - 1]);
+                       if (isCommit)
+                               PrintBufferLeakWarning(owner->buffers[owner->nbuffers - 1]);
+                       ReleaseBuffer(owner->buffers[owner->nbuffers - 1]);
                }
+
                /* Release relcache references */
                if (isTopLevel)
                {
index 04bc09e22c69707301f4688205a5981ae46728b1..5064a1857f674a05646f6dfe374a2facf803c818 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/bufmgr.h,v 1.87 2004/10/15 22:40:25 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/storage/bufmgr.h,v 1.88 2004/10/16 18:57:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -122,6 +122,8 @@ extern void InitBufferPoolAccess(void);
 extern char *ShowBufferUsage(void);
 extern void ResetBufferUsage(void);
 extern void AtEOXact_Buffers(bool isCommit);
+extern void AtProcExit_Buffers(void);
+extern void PrintBufferLeakWarning(Buffer buffer);
 extern void FlushBufferPool(void);
 extern BlockNumber BufferGetBlockNumber(Buffer buffer);
 extern BlockNumber RelationGetNumberOfBlocks(Relation relation);