Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c931c07

Browse files
committed
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.
1 parent 082aca9 commit c931c07

File tree

1 file changed

+26
-7
lines changed

1 file changed

+26
-7
lines changed

src/backend/commands/vacuum.c

+26-7
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.363 2008/01/03 21:23:15 tgl Exp $
16+
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.364 2008/02/11 19:14:30 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -1659,12 +1659,18 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
16591659
free_space += vacpage->free;
16601660

16611661
/*
1662-
* Add the page to fraged_pages if it has a useful amount of free
1663-
* space. "Useful" means enough for a minimal-sized tuple. But we
1664-
* don't know that accurately near the start of the relation, so add
1665-
* pages unconditionally if they have >= BLCKSZ/10 free space.
1662+
* Add the page to vacuum_pages if it requires reaping, and add it to
1663+
* fraged_pages if it has a useful amount of free space. "Useful"
1664+
* means enough for a minimal-sized tuple. But we don't know that
1665+
* accurately near the start of the relation, so add pages
1666+
* unconditionally if they have >= BLCKSZ/10 free space. Also
1667+
* forcibly add pages with no live tuples, to avoid confusing the
1668+
* empty_end_pages logic. (In the presence of unreasonably small
1669+
* fillfactor, it seems possible that such pages might not pass
1670+
* the free-space test, but they had better be in the list anyway.)
16661671
*/
1667-
do_frag = (vacpage->free >= min_tlen || vacpage->free >= BLCKSZ / 10);
1672+
do_frag = (vacpage->free >= min_tlen || vacpage->free >= BLCKSZ / 10 ||
1673+
notup);
16681674

16691675
if (do_reap || do_frag)
16701676
{
@@ -1679,6 +1685,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
16791685
/*
16801686
* Include the page in empty_end_pages if it will be empty after
16811687
* vacuuming; this is to keep us from using it as a move destination.
1688+
* Note that such pages are guaranteed to be in fraged_pages.
16821689
*/
16831690
if (notup)
16841691
{
@@ -3725,7 +3732,19 @@ enough_space(VacPage vacpage, Size len)
37253732
static Size
37263733
PageGetFreeSpaceWithFillFactor(Relation relation, Page page)
37273734
{
3728-
Size freespace = PageGetHeapFreeSpace(page);
3735+
/*
3736+
* It is correct to use PageGetExactFreeSpace() here, *not*
3737+
* PageGetHeapFreeSpace(). This is because (a) we do our own, exact
3738+
* accounting for whether line pointers must be added, and (b) we will
3739+
* recycle any LP_DEAD line pointers before starting to add rows to a
3740+
* page, but that may not have happened yet at the time this function is
3741+
* applied to a page, which means PageGetHeapFreeSpace()'s protection
3742+
* against too many line pointers on a page could fire incorrectly. We do
3743+
* not need that protection here: since VACUUM FULL always recycles all
3744+
* dead line pointers first, it'd be physically impossible to insert more
3745+
* than MaxHeapTuplesPerPage tuples anyway.
3746+
*/
3747+
Size freespace = PageGetExactFreeSpace(page);
37293748
Size targetfree;
37303749

37313750
targetfree = RelationGetTargetPageFreeSpace(relation,

0 commit comments

Comments
 (0)