diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2008-02-11 19:14:53 +0000 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2008-02-11 19:14:53 +0000 |
| commit | 45639b220d3888601b3ee886f0c94766706144cf (patch) | |
| tree | 78958fa4d2c482c41b34d2fa9020b9e06512b047 | |
| parent | a209121c06df71abe3222b6798748c13c35f7017 (diff) | |
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.
| -rw-r--r-- | src/backend/commands/vacuum.c | 18 |
1 files changed, 12 insertions, 6 deletions
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 273c79a2fab..478cf8253ff 100644 --- 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.299.4.3 2008/01/03 21:25:00 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.299.4.4 2008/02/11 19:14:53 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1442,12 +1442,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) { |