Fixed bug where FlushRelationBuffers() did call StrategyInvalidateBuffer()

for already empty buffers because their buffer tag was not cleard out
when the buffers have been invalidated before.

Also removed the misnamed BM_FREE bufhdr flag and replaced the checks,
which effectively ask if the buffer is unpinned, with checks against the
refcount field.

Jan

diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c
index 2f3c818c19167e5dd5608251759467762a43ee47..a671bf9f7ff87a755fa53b0d75826efc5956fa36 100644 (file)
--- a/src/backend/storage/buffer/buf_init.c
+++ b/src/backend/storage/buffer/buf_init.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/storage/buffer/buf_init.c,v 1.61 2004/01/15 16:14:26 wieck Exp $
+ *   $PostgreSQL: pgsql/src/backend/storage/buffer/buf_init.c,v 1.62 2004/02/12 15:06:56 wieck Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -164,7 +164,7 @@ InitBufferPool(void)
            buf->buf_id = i;
 
            buf->data = MAKE_OFFSET(block);
-           buf->flags = (BM_DELETED | BM_FREE | BM_VALID);
+           buf->flags = (BM_DELETED | BM_VALID);
            buf->refcount = 0;
            buf->io_in_progress_lock = LWLockAssign();
            buf->cntx_lock = LWLockAssign();

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 0ad26ee4f0be2aeae73400caf908236dbde2a4de..e7435b9534a4e894ef06276a817da7f3edfc4786 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.158 2004/02/10 03:42:45 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.159 2004/02/12 15:06:56 wieck Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1190,7 +1190,7 @@ recheck:
             * Release any refcount we may have.  If someone else has a
             * pin on the buffer, we got trouble.
             */
-           if (!(bufHdr->flags & BM_FREE))
+           if (bufHdr->refcount != 0)
            {
                /* the sole pin should be ours */
                if (bufHdr->refcount != 1 || PrivateRefCount[i - 1] == 0)
@@ -1268,7 +1268,7 @@ recheck:
             * The thing should be free, if caller has checked that no
             * backends are running in that database.
             */
-           Assert(bufHdr->flags & BM_FREE);
+           Assert(bufHdr->refcount == 0);
 
            /*
             * And mark the buffer as no longer occupied by this page.
@@ -1501,7 +1501,7 @@ FlushRelationBuffers(Relation rel, BlockNumber firstDelBlock)
                }
                UnpinBuffer(bufHdr);
            }
-           if (!(bufHdr->flags & BM_FREE))
+           if (bufHdr->refcount != 0)
            {
                LWLockRelease(BufMgrLock);
                error_context_stack = errcontext.previous;

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 74ec4518ab90fdec2b61310df342a862f930987a..595e4905a80c6fdb1e8d4f9946d867f4126c26e6 100644 (file)
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/storage/buffer/freelist.c,v 1.40 2004/02/06 19:36:18 wieck Exp $
+ *   $PostgreSQL: pgsql/src/backend/storage/buffer/freelist.c,v 1.41 2004/02/12 15:06:56 wieck Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -662,9 +662,6 @@ void
 StrategyInvalidateBuffer(BufferDesc *buf)
 {
    int                 cdb_id;
-#ifdef USE_ASSERT_CHECKING
-   int                 buf_id;
-#endif
    BufferStrategyCDB  *cdb;
 
    /* The buffer cannot be dirty or pinned */
@@ -672,41 +669,37 @@ StrategyInvalidateBuffer(BufferDesc *buf)
    Assert(buf->refcount == 0);
 
    /*
-    * If we have the buffer somewhere in the directory, remove it,
-    * add the CDB to the list of unused CDB's. and the buffer to
-    * the list of free buffers
+    * Lookup the cache directory block for this buffer
     */
    cdb_id = BufTableLookup(&(buf->tag));
-   if (cdb_id >= 0)
-   {
-       cdb = &StrategyCDB[cdb_id];
-       BufTableDelete(&(cdb->buf_tag));
-       STRAT_LIST_REMOVE(cdb);
-       cdb->buf_id = -1;
-       cdb->next = StrategyControl->listUnusedCDB;
-       StrategyControl->listUnusedCDB = cdb_id;
-
-       buf->bufNext = StrategyControl->listFreeBuffers;
-       StrategyControl->listFreeBuffers = buf->buf_id;
-       return;
-   }
+   if (cdb_id < 0)
+       elog(ERROR, "StrategyInvalidateBuffer() buffer %d not in directory",
+               buf->buf_id);
+   cdb = &StrategyCDB[cdb_id];
 
-#ifdef USE_ASSERT_CHECKING
    /*
-    * Check that we have this buffer in the freelist already.
+    * Remove the CDB from the hashtable and the ARC queue it is
+    * currently on.
     */
-   buf_id = StrategyControl->listFreeBuffers;
-   while (buf_id >= 0)
-   {
-       if (buf == &BufferDescriptors[buf_id])
-           return;
+   BufTableDelete(&(cdb->buf_tag));
+   STRAT_LIST_REMOVE(cdb);
 
-       buf_id = BufferDescriptors[buf_id].bufNext;
-   }
+   /*
+    * Clear out the CDB's buffer tag and association with the buffer
+    * and add it to the list of unused CDB's
+    */
+   CLEAR_BUFFERTAG(&(cdb->buf_tag));
+   cdb->buf_id = -1;
+   cdb->next = StrategyControl->listUnusedCDB;
+   StrategyControl->listUnusedCDB = cdb_id;
 
-   elog(ERROR, "StrategyInvalidateBuffer() buffer %d not in directory or freelist",
-               buf->buf_id);
-#endif
+   /*
+    * Clear out the buffers tag and add it to the list of
+    * currently unused buffers.
+    */
+   CLEAR_BUFFERTAG(&(buf->tag));
+   buf->bufNext = StrategyControl->listFreeBuffers;
+   StrategyControl->listFreeBuffers = buf->buf_id;
 }
 
 
@@ -869,12 +862,6 @@ PinBuffer(BufferDesc *buf)
 {
    int         b = BufferDescriptorGetBuffer(buf) - 1;
 
-   if (buf->refcount == 0)
-   {
-       /* mark buffer as no longer free */
-       buf->flags &= ~BM_FREE;
-   }
-
    if (PrivateRefCount[b] == 0)
        buf->refcount++;
    PrivateRefCount[b]++;
@@ -917,12 +904,7 @@ UnpinBuffer(BufferDesc *buf)
    if (PrivateRefCount[b] == 0)
        buf->refcount--;
 
-   if (buf->refcount == 0)
-   {
-       /* buffer is now unpinned */
-       buf->flags |= BM_FREE;
-   }
-   else if ((buf->flags & BM_PIN_COUNT_WAITER) != 0 &&
+   if ((buf->flags & BM_PIN_COUNT_WAITER) != 0 &&
             buf->refcount == 1)
    {
        /* we just released the last pin other than the waiter's */
@@ -953,70 +935,3 @@ refcount = %ld, file: %s, line: %d\n",
 #endif
 
 
-/*
- * print out the free list and check for breaks.
- */
-#ifdef NOT_USED
-void
-DBG_FreeListCheck(int nfree)
-{
-   int         i;
-   BufferDesc *buf;
-
-   buf = &(BufferDescriptors[SharedFreeList->freeNext]);
-   for (i = 0; i < nfree; i++, buf = &(BufferDescriptors[buf->freeNext]))
-   {
-       if (!(buf->flags & (BM_FREE)))
-       {
-           if (buf != SharedFreeList)
-               printf("\tfree list corrupted: %d flags %x\n",
-                      buf->buf_id, buf->flags);
-           else
-               printf("\tfree list corrupted: too short -- %d not %d\n",
-                      i, nfree);
-       }
-       if ((BufferDescriptors[buf->freeNext].freePrev != buf->buf_id) ||
-           (BufferDescriptors[buf->freePrev].freeNext != buf->buf_id))
-           printf("\tfree list links corrupted: %d %ld %ld\n",
-                  buf->buf_id, buf->freePrev, buf->freeNext);
-   }
-   if (buf != SharedFreeList)
-       printf("\tfree list corrupted: %d-th buffer is %d\n",
-              nfree, buf->buf_id);
-}
-#endif
-
-#ifdef NOT_USED
-/*
- * PrintBufferFreeList -
- *   prints the buffer free list, for debugging
- */
-static void
-PrintBufferFreeList()
-{
-   BufferDesc *buf;
-
-   if (SharedFreeList->freeNext == Free_List_Descriptor)
-   {
-       printf("free list is empty.\n");
-       return;
-   }
-
-   buf = &(BufferDescriptors[SharedFreeList->freeNext]);
-   for (;;)
-   {
-       int         i = (buf - BufferDescriptors);
-
-       printf("[%-2d] (%s, %d) flags=0x%x, refcnt=%d %ld, nxt=%ld prv=%ld)\n",
-              i, buf->blind.relname, buf->tag.blockNum,
-              buf->flags, buf->refcount, PrivateRefCount[i],
-              buf->freeNext, buf->freePrev);
-
-       if (buf->freeNext == Free_List_Descriptor)
-           break;
-
-       buf = &(BufferDescriptors[buf->freeNext]);
-   }
-}
-
-#endif

diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index ce4be5439b4d2a5712643f6ab9f305e82e68e16a..f401791d93a7c7c3168cee9305a9d3562bdc82eb 100644 (file)
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/buf_internals.h,v 1.67 2004/01/15 16:14:26 wieck Exp $
+ * $PostgreSQL: pgsql/src/include/storage/buf_internals.h,v 1.68 2004/02/12 15:06:56 wieck Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,11 +36,10 @@ extern int  ShowPinTrace;
 #define BM_DIRTY               (1 << 0)
 #define BM_VALID               (1 << 1)
 #define BM_DELETED             (1 << 2)
-#define BM_FREE                    (1 << 3)
-#define BM_IO_IN_PROGRESS      (1 << 4)
-#define BM_IO_ERROR                (1 << 5)
-#define BM_JUST_DIRTIED            (1 << 6)
-#define BM_PIN_COUNT_WAITER        (1 << 7)
+#define BM_IO_IN_PROGRESS      (1 << 3)
+#define BM_IO_ERROR                (1 << 4)
+#define BM_JUST_DIRTIED            (1 << 5)
+#define BM_PIN_COUNT_WAITER        (1 << 6)
 
 typedef bits16 BufFlags;