Make sure that FlushRelationBuffers() is invoked by all paths through

vacuum.c.  This is needed to make the world safe for pg_upgrade.

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7d4005d21d5b5d953cc4a4cacafa547f10fd1591..3bd65fed7658ca8adf0dbeb6d15eab8798eb4abb 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.165 2000/09/12 04:49:07 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.166 2000/09/19 19:30:03 tgl Exp $
  *
 
  *-------------------------------------------------------------------------
@@ -397,12 +397,6 @@ vacuum_rel(Oid relid, bool analyze, bool is_toastrel)
         */
        onerel = heap_open(relid, AccessExclusiveLock);
 
-       /*
-        * Remember the relations TOAST relation for later
-        *
-        */
-       toast_relid = onerel->rd_rel->reltoastrelid;
-
 #ifndef NO_SECURITY
        if (!pg_ownercheck(GetUserId(), RelationGetRelationName(onerel),
                                           RELNAME))
@@ -416,6 +410,11 @@ vacuum_rel(Oid relid, bool analyze, bool is_toastrel)
        }
 #endif
 
+       /*
+        * Remember the relation'ss TOAST relation for later
+        */
+       toast_relid = onerel->rd_rel->reltoastrelid;
+
        /*
         * Set up statistics-gathering machinery.
         */
@@ -430,7 +429,8 @@ vacuum_rel(Oid relid, bool analyze, bool is_toastrel)
        reindex = false;
        vacuum_pages.num_pages = fraged_pages.num_pages = 0;
        scan_heap(vacrelstats, onerel, &vacuum_pages, &fraged_pages);
-       if (IsIgnoringSystemIndexes() && IsSystemRelationName(RelationGetRelationName(onerel)))
+       if (IsIgnoringSystemIndexes() &&
+               IsSystemRelationName(RelationGetRelationName(onerel)))
                reindex = true;
 
        /* Now open indices */
@@ -459,30 +459,54 @@ vacuum_rel(Oid relid, bool analyze, bool is_toastrel)
                if (vacuum_pages.num_pages > 0)
                {
                        for (i = 0; i < nindices; i++)
-                               vacuum_index(&vacuum_pages, Irel[i], vacrelstats->num_tuples, 0);
+                               vacuum_index(&vacuum_pages, Irel[i],
+                                                        vacrelstats->num_tuples, 0);
                }
                else
-               /* just scan indices to update statistic */
                {
+                       /* just scan indices to update statistic */
                        for (i = 0; i < nindices; i++)
                                scan_index(Irel[i], vacrelstats->num_tuples);
                }
        }
 
-       if (fraged_pages.num_pages > 0) /* Try to shrink heap */
-               repair_frag(vacrelstats, onerel, &vacuum_pages, &fraged_pages, nindices, Irel);
+       if (fraged_pages.num_pages > 0)
+       {
+               /* Try to shrink heap */
+               repair_frag(vacrelstats, onerel, &vacuum_pages, &fraged_pages,
+                                       nindices, Irel);
+       }
        else
        {
                if (Irel != (Relation *) NULL)
                        close_indices(nindices, Irel);
-               if (vacuum_pages.num_pages > 0)         /* Clean pages from
-                                                                                                * vacuum_pages list */
+               if (vacuum_pages.num_pages > 0)
+               {
+                       /* Clean pages from vacuum_pages list */
                        vacuum_heap(vacrelstats, onerel, &vacuum_pages);
+               }
+               else
+               {
+                       /*
+                        * Flush dirty pages out to disk.  We must do this even if we
+                        * didn't do anything else, because we want to ensure that all
+                        * tuples have correct on-row commit status on disk (see
+                        * bufmgr.c's comments for FlushRelationBuffers()).
+                        */
+                       i = FlushRelationBuffers(onerel, vacrelstats->num_pages);
+                       if (i < 0)
+                               elog(ERROR, "VACUUM (vacuum_rel): FlushRelationBuffers returned %d",
+                                        i);
+               }
        }
        if (reindex)
                activate_indexes_of_a_table(relid, true);
 
-       /* ok - free vacuum_pages list of reaped pages */
+       /*
+        * ok - free vacuum_pages list of reaped pages
+        *
+        * Isn't this a waste of code?  Upcoming commit should free memory, no?
+        */
        if (vacuum_pages.num_pages > 0)
        {
                vacpage = vacuum_pages.pagedesc;
@@ -498,12 +522,14 @@ vacuum_rel(Oid relid, bool analyze, bool is_toastrel)
 
        /* update statistics in pg_class */
        update_relstats(vacrelstats->relid, vacrelstats->num_pages,
-                       vacrelstats->num_tuples, vacrelstats->hasindex, vacrelstats);
+                                       vacrelstats->num_tuples, vacrelstats->hasindex,
+                                       vacrelstats);
 
-       /* If the relation has a secondary toast one, vacuum that too
+       /*
+        * If the relation has a secondary toast one, vacuum that too
         * while we still hold the lock on the master table. We don't
         * need to propagate "analyze" to it, because the toaster
-        * allways uses hardcoded index access and statistics are
+        * always uses hardcoded index access and statistics are
         * totally unimportant for toast relations
         */
        if (toast_relid != InvalidOid)
@@ -1820,12 +1846,20 @@ failed to add item with len = %u to page %u (free space %u, nusd %u, noff %u)",
                pfree(Nvacpagelist.pagedesc);
        }
 
-       /* truncate relation, after flushing any dirty pages out to disk */
+       /*
+        * Flush dirty pages out to disk.  We do this unconditionally, even if
+        * we don't need to truncate, because we want to ensure that all tuples
+        * have correct on-row commit status on disk (see bufmgr.c's comments
+        * for FlushRelationBuffers()).
+        */
+       i = FlushRelationBuffers(onerel, blkno);
+       if (i < 0)
+               elog(ERROR, "VACUUM (repair_frag): FlushRelationBuffers returned %d",
+                        i);
+
+       /* truncate relation, if needed */
        if (blkno < nblocks)
        {
-               i = FlushRelationBuffers(onerel, blkno);
-               if (i < 0)
-                       elog(FATAL, "VACUUM (repair_frag): FlushRelationBuffers returned %d", i);
                blkno = smgrtruncate(DEFAULT_SMGR, onerel, blkno);
                Assert(blkno >= 0);
                vacrelstats->num_pages = blkno; /* set new number of blocks */
@@ -1840,7 +1874,6 @@ failed to add item with len = %u to page %u (free space %u, nusd %u, noff %u)",
        pfree(vacpage);
        if (vacrelstats->vtlinks != NULL)
                pfree(vacrelstats->vtlinks);
-
 }
 
 /*
@@ -1860,7 +1893,7 @@ vacuum_heap(VRelStats *vacrelstats, Relation onerel, VacPageList vacuum_pages)
 
        nblocks = vacuum_pages->num_pages;
        nblocks -= vacuum_pages->empty_end_pages;               /* nothing to do with
-                                                                                                                * them */
+                                                                                                        * them */
 
        for (i = 0, vacpage = vacuum_pages->pagedesc; i < nblocks; i++, vacpage++)
        {
@@ -1873,33 +1906,30 @@ vacuum_heap(VRelStats *vacrelstats, Relation onerel, VacPageList vacuum_pages)
                }
        }
 
+       /*
+        * Flush dirty pages out to disk.  We do this unconditionally, even if
+        * we don't need to truncate, because we want to ensure that all tuples
+        * have correct on-row commit status on disk (see bufmgr.c's comments
+        * for FlushRelationBuffers()).
+        */
+       Assert(vacrelstats->num_pages >= vacuum_pages->empty_end_pages);
+       nblocks = vacrelstats->num_pages - vacuum_pages->empty_end_pages;
+
+       i = FlushRelationBuffers(onerel, nblocks);
+       if (i < 0)
+               elog(ERROR, "VACUUM (vacuum_heap): FlushRelationBuffers returned %d",
+                        i);
+
        /* truncate relation if there are some empty end-pages */
        if (vacuum_pages->empty_end_pages > 0)
        {
-               Assert(vacrelstats->num_pages >= vacuum_pages->empty_end_pages);
-               nblocks = vacrelstats->num_pages - vacuum_pages->empty_end_pages;
                elog(MESSAGE_LEVEL, "Rel %s: Pages: %u --> %u.",
                         RelationGetRelationName(onerel),
                         vacrelstats->num_pages, nblocks);
-
-               /*
-                * We have to flush "empty" end-pages (if changed, but who knows
-                * it) before truncation
-                *
-                * XXX is FlushBufferPool() still needed here?
-                */
-               FlushBufferPool();
-
-               i = FlushRelationBuffers(onerel, nblocks);
-               if (i < 0)
-                       elog(FATAL, "VACUUM (vacuum_heap): FlushRelationBuffers returned %d", i);
-
                nblocks = smgrtruncate(DEFAULT_SMGR, onerel, nblocks);
                Assert(nblocks >= 0);
-               vacrelstats->num_pages = nblocks;               /* set new number of
-                                                                                                * blocks */
+               vacrelstats->num_pages = nblocks; /* set new number of blocks */
        }
-
 }
 
 /*