postgres.git / summary / log / commit / refs

commit    bb4b179b5e80b66c4049226f8dc2966bd1ce50aa
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Mon Feb 11 19:15:00 2008 +0000
First released in: 8.3.1, 7.4.20, 8.0.16, 8.1.12, 8.2.7

    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.


 src/backend/commands/vacuum.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7ce960b3c7e..84a654799c5 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.263.2.4 2008/01/03 21:25:33 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.263.2.5 2008/02/11 19:15:00 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1294,12 +1294,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)
 		{


[parent: d85096ce1296]