Second try at fixing no-room-to-move-down PANIC in compact_fsm_storage.

Ward's report that it can still happen in RC2 forces me to realize that
this is not a can't-happen condition after all, and that the compaction
code had better cope rather than panicking.

diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index b5e1a53b35eea118cd00ae819dfd78b2cb95a656..62a0044f09eb9425bc4b2e42640945dda569067d 100644 (file)
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/storage/freespace/freespace.c,v 1.24 2003/10/29 17:36:57 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/storage/freespace/freespace.c,v 1.25 2003/11/26 20:50:11 tgl Exp $
  *
  *
  * NOTES:
@@ -1394,6 +1394,7 @@ static void
 compact_fsm_storage(void)
 {
    int         nextChunkIndex = 0;
+   bool        did_push = false;
    FSMRelation *fsmrel;
 
    for (fsmrel = FreeSpaceMap->firstRel;
@@ -1419,16 +1420,15 @@ compact_fsm_storage(void)
            newAllocPages = newAlloc * INDEXCHUNKPAGES;
        else
            newAllocPages = newAlloc * CHUNKPAGES;
-       newChunkIndex = nextChunkIndex;
-       nextChunkIndex += newAlloc;
 
        /*
         * Determine current size, current and new locations
         */
        curChunks = fsm_current_chunks(fsmrel);
        oldChunkIndex = fsmrel->firstChunk;
-       newLocation = FreeSpaceMap->arena + newChunkIndex * CHUNKBYTES;
        oldLocation = FreeSpaceMap->arena + oldChunkIndex * CHUNKBYTES;
+       newChunkIndex = nextChunkIndex;
+       newLocation = FreeSpaceMap->arena + newChunkIndex * CHUNKBYTES;
 
        /*
         * It's possible that we have to move data down, not up, if the
@@ -1440,10 +1440,16 @@ compact_fsm_storage(void)
         * more than once, so pack everything against the end of the arena
         * if so.
         *
-        * In corner cases where roundoff has affected our allocation, it's
-        * possible that we have to move down and compress our data too.
-        * Since this case is extremely infrequent, we do not try to be smart
-        * about it --- we just drop pages from the end of the rel's data.
+        * In corner cases where we are on the short end of a roundoff choice
+        * that we were formerly on the long end of, it's possible that we
+        * have to move down and compress our data too.  In fact, even after
+        * pushing down the following rels, there might not be as much space
+        * as we computed for this rel above --- that would imply that some
+        * following rel(s) are also on the losing end of roundoff choices.
+        * We could handle this fairly by doing the per-rel compactions
+        * out-of-order, but that seems like way too much complexity to deal
+        * with a very infrequent corner case.  Instead, we simply drop pages
+        * from the end of the current rel's data until it fits.
         */
        if (newChunkIndex > oldChunkIndex)
        {
@@ -1455,21 +1461,44 @@ compact_fsm_storage(void)
                fsmrel->storedPages = newAllocPages;
                curChunks = fsm_current_chunks(fsmrel);
            }
+           /* is there enough space? */
            if (fsmrel->nextPhysical != NULL)
                limitChunkIndex = fsmrel->nextPhysical->firstChunk;
            else
                limitChunkIndex = FreeSpaceMap->totalChunks;
            if (newChunkIndex + curChunks > limitChunkIndex)
            {
-               /* need to push down additional rels */
-               push_fsm_rels_after(fsmrel);
-               /* recheck for safety */
+               /* not enough space, push down following rels */
+               if (!did_push)
+               {
+                   push_fsm_rels_after(fsmrel);
+                   did_push = true;
+               }
+               /* now is there enough space? */
                if (fsmrel->nextPhysical != NULL)
                    limitChunkIndex = fsmrel->nextPhysical->firstChunk;
                else
                    limitChunkIndex = FreeSpaceMap->totalChunks;
                if (newChunkIndex + curChunks > limitChunkIndex)
-                   elog(PANIC, "insufficient room in FSM");
+               {
+                   /* uh-oh, forcibly cut the allocation to fit */
+                   newAlloc = limitChunkIndex - newChunkIndex;
+                   /*
+                    * If newAlloc < 0 at this point, we are moving the rel's
+                    * firstChunk into territory currently assigned to a later
+                    * rel.  This is okay so long as we do not copy any data.
+                    * The rels will be back in nondecreasing firstChunk order
+                    * at completion of the compaction pass.
+                    */
+                   if (newAlloc < 0)
+                       newAlloc = 0;
+                   if (fsmrel->isIndex)
+                       newAllocPages = newAlloc * INDEXCHUNKPAGES;
+                   else
+                       newAllocPages = newAlloc * CHUNKPAGES;
+                   fsmrel->storedPages = newAllocPages;
+                   curChunks = fsm_current_chunks(fsmrel);
+               }
            }
            memmove(newLocation, oldLocation, curChunks * CHUNKBYTES);
        }
@@ -1504,6 +1533,7 @@ compact_fsm_storage(void)
            memmove(newLocation, oldLocation, curChunks * CHUNKBYTES);
        }
        fsmrel->firstChunk = newChunkIndex;
+       nextChunkIndex += newAlloc;
    }
    Assert(nextChunkIndex <= FreeSpaceMap->totalChunks);
    FreeSpaceMap->usedChunks = nextChunkIndex;
@@ -1544,8 +1574,8 @@ push_fsm_rels_after(FSMRelation *afterRel)
        oldChunkIndex = fsmrel->firstChunk;
        if (newChunkIndex < oldChunkIndex)
        {
-           /* trouble... */
-           elog(PANIC, "insufficient room in FSM");
+           /* we're pushing down, how can it move up? */
+           elog(PANIC, "inconsistent entry sizes in FSM");
        }
        else if (newChunkIndex > oldChunkIndex)
        {
@@ -1758,14 +1788,14 @@ fsm_current_chunks(FSMRelation *fsmrel)
 {
    int         chunkCount;
 
+   /* Make sure storedPages==0 produces right answer */
+   if (fsmrel->storedPages <= 0)
+       return 0;
    /* Convert page count to chunk count */
    if (fsmrel->isIndex)
        chunkCount = (fsmrel->storedPages - 1) / INDEXCHUNKPAGES + 1;
    else
        chunkCount = (fsmrel->storedPages - 1) / CHUNKPAGES + 1;
-   /* Make sure storedPages==0 produces right answer */
-   if (chunkCount < 0)
-       chunkCount = 0;
    return chunkCount;
 }