Fix ReadBuffer() to correctly handle the case where it's trying to extend

the relation but it finds a pre-existing valid buffer.  The buffer does not
correspond to any page known to the kernel, so we *must* do smgrextend to
ensure that the space becomes allocated.  The 7.x branches all do this
correctly, but the corner case got lost somewhere during 8.0 bufmgr rewrites.
(My fault no doubt :-( ... I think I assumed that such a buffer must be
not-BM_VALID, which is not so.)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index a8a37e5ca3cfa5f2ccba3ec2bacf6aae1441da5e..11e83812d0c0712053995fd7347a04e29f7a9912 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.201 2005/12/29 18:08:05 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.202 2006/01/06 00:04:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -164,13 +164,47 @@ ReadBuffer(Relation reln, BlockNumber blockNum)
        /* if it was already in the buffer pool, we're done */
        if (found)
        {
-               /* Just need to update stats before we exit */
-               pgstat_count_buffer_hit(&reln->pgstat_info, reln);
+               if (!isExtend)
+               {
+                       /* Just need to update stats before we exit */
+                       pgstat_count_buffer_hit(&reln->pgstat_info, reln);
+
+                       if (VacuumCostActive)
+                               VacuumCostBalance += VacuumCostPageHit;
 
-               if (VacuumCostActive)
-                       VacuumCostBalance += VacuumCostPageHit;
+                       return BufferDescriptorGetBuffer(bufHdr);
+               }
 
-               return BufferDescriptorGetBuffer(bufHdr);
+               /*
+                * We get here only in the corner case where we are trying to extend
+                * the relation but we found a pre-existing buffer marked BM_VALID.
+                * (This can happen because mdread doesn't complain about reads
+                * beyond EOF --- which is arguably bogus, but changing it seems
+                * tricky.)  We *must* do smgrextend before succeeding, else the
+                * page will not be reserved by the kernel, and the next P_NEW call
+                * will decide to return the same page.  Clear the BM_VALID bit,
+                * do the StartBufferIO call that BufferAlloc didn't, and proceed.
+                */
+               if (isLocalBuf)
+               {
+                       /* Only need to adjust flags */
+                       Assert(bufHdr->flags & BM_VALID);
+                       bufHdr->flags &= ~BM_VALID;
+               }
+               else
+               {
+                       /*
+                        * Loop to handle the very small possibility that someone
+                        * re-sets BM_VALID between our clearing it and StartBufferIO
+                        * inspecting it.
+                        */
+                       do {
+                               LockBufHdr(bufHdr);
+                               Assert(bufHdr->flags & BM_VALID);
+                               bufHdr->flags &= ~BM_VALID;
+                               UnlockBufHdr(bufHdr);
+                       } while (!StartBufferIO(bufHdr, true));
+               }
        }
 
        /*