Repair VACUUM FULL bug introduced by HOT patch: the original way of

calculating a page's initial free space was fine, and should not have been
"improved" by letting PageGetHeapFreeSpace do it.  VACUUM FULL is going to
reclaim LP_DEAD line pointers later, so there is no need for a guard
against the page being too full of line pointers, and having one risks
rejecting pages that are perfectly good move destinations.

This also exposed a second bug, which is that the empty_end_pages logic
assumed that any page with no live tuples would get entered into the
fraged_pages list automatically (by virtue of having more free space than
the threshold in the do_frag calculation).  This assumption certainly
seems risky when a low fillfactor has been chosen, and even without
tunable fillfactor I think it could conceivably fail on a page with many
unused line pointers.  So fix the code to force do_frag true when notup
is true, and patch this part of the fix all the way back.

Per report from Tomas Szepe.

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 39d769aaa8af66a62e7b8d5873611194aa669765..e9f41eac323c95c94ef0b09e4e3dda435972782a 100644 (file)
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.317.2.6 2008/01/03 21:24:26 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.317.2.7 2008/02/11 19:14:45 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1515,12 +1515,18 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
                free_space += vacpage->free;
 
                /*
-                * Add the page to fraged_pages if it has a useful amount of free
-                * space.  "Useful" means enough for a minimal-sized tuple. But we
-                * don't know that accurately near the start of the relation, so add
-                * pages unconditionally if they have >= BLCKSZ/10 free space.
+                * Add the page to vacuum_pages if it requires reaping, and add it to
+                * fraged_pages if it has a useful amount of free space.  "Useful"
+                * means enough for a minimal-sized tuple.  But we don't know that
+                * accurately near the start of the relation, so add pages
+                * unconditionally if they have >= BLCKSZ/10 free space.  Also
+                * forcibly add pages with no live tuples, to avoid confusing the
+                * empty_end_pages logic.  (In the presence of unreasonably small
+                * fillfactor, it seems possible that such pages might not pass
+                * the free-space test, but they had better be in the list anyway.)
                 */
-               do_frag = (vacpage->free >= min_tlen || vacpage->free >= BLCKSZ / 10);
+               do_frag = (vacpage->free >= min_tlen || vacpage->free >= BLCKSZ / 10 ||
+                                  notup);
 
                if (do_reap || do_frag)
                {
@@ -1535,6 +1541,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
                /*
                 * Include the page in empty_end_pages if it will be empty after
                 * vacuuming; this is to keep us from using it as a move destination.
+                * Note that such pages are guaranteed to be in fraged_pages.
                 */
                if (notup)
                {