From 09745c1131674b829038892317f13fc3670cb32d Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 12 Jan 2024 14:25:22 -0500 Subject: [PATCH v1] Be more consistent about whether to update the FSM while vacuuming. Previously, when lazy_scan_noprune() was called and returned true, we would update the FSM immediately if the relation had no indexes or if the page contained no dead items. On the other hand, when lazy_scan_prune() was called, we would update the FSM if either of those things was true or if index vacuuming was disabled. Eliminate that behavioral difference by considering vacrel->do_index_vacuuming in both cases. Also, instead of having lazy_scan_noprune() make the decision internally and pass it back to the caller via *recordfreespace, just make the decision directly in the caller. That seems less confusing, since the caller also decides in the lazy_scan_prune() case; moreover, this way, the whole test is in one place, instead of spread out. --- src/backend/access/heap/vacuumlazy.c | 51 ++++++++++++---------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index b63cad1335..ea452f4415 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -251,8 +251,7 @@ static void lazy_scan_prune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, LVPagePruneState *prunestate); static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf, - BlockNumber blkno, Page page, - bool *recordfreespace); + BlockNumber blkno, Page page); static void lazy_vacuum(LVRelState *vacrel); static bool lazy_vacuum_all_indexes(LVRelState *vacrel); static void lazy_vacuum_heap_rel(LVRelState *vacrel); @@ -958,8 +957,6 @@ lazy_scan_heap(LVRelState *vacrel) page = BufferGetPage(buf); if (!ConditionalLockBufferForCleanup(buf)) { - bool recordfreespace; - LockBuffer(buf, BUFFER_LOCK_SHARE); /* Check for new or empty pages before lazy_scan_noprune call */ @@ -974,17 +971,29 @@ lazy_scan_heap(LVRelState *vacrel) * Collect LP_DEAD items in dead_items array, count tuples, * determine if rel truncation is safe */ - if (lazy_scan_noprune(vacrel, buf, blkno, page, - &recordfreespace)) + if (lazy_scan_noprune(vacrel, buf, blkno, page)) { Size freespace = 0; + bool recordfreespace; /* - * Processed page successfully (without cleanup lock) -- just - * need to update the FSM, much like the lazy_scan_prune case. - * Don't bother trying to match its visibility map setting - * steps, though. + * We processed the page successfully (without a cleanup lock). + * + * Update the FSM, just as we would in the case where + * lazy_scan_prune() is called. Our goal is to update the + * freespace map the last time we touch the page. If the + * relation has no indexes, or if index vacuuming is disabled, + * there will be no second heap pass; if this particular page + * has no dead items, the second heap pass will not touch this + * page. So, in those cases, update the FSM now. + * + * After a call to lazy_scan_prune(), we would also try to + * adjust the page-level all-visible bit and the visibility + * map, but we skip that step in this path. */ + recordfreespace = vacrel->nindexes == 0 + || !vacrel->do_index_vacuuming + || !prunestate.has_lpdead_items; if (recordfreespace) freespace = PageGetHeapFreeSpace(page); UnlockReleaseBuffer(buf); @@ -1942,8 +1951,7 @@ static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, - Page page, - bool *recordfreespace) + Page page) { OffsetNumber offnum, maxoff; @@ -1960,7 +1968,6 @@ lazy_scan_noprune(LVRelState *vacrel, Assert(BufferGetBlockNumber(buf) == blkno); hastup = false; /* for now */ - *recordfreespace = false; /* for now */ lpdead_items = 0; live_tuples = 0; @@ -2102,18 +2109,8 @@ lazy_scan_noprune(LVRelState *vacrel, hastup = true; missed_dead_tuples += lpdead_items; } - - *recordfreespace = true; - } - else if (lpdead_items == 0) - { - /* - * Won't be vacuuming this page later, so record page's freespace in - * the FSM now - */ - *recordfreespace = true; } - else + else if (lpdead_items != 0) { VacDeadItems *dead_items = vacrel->dead_items; ItemPointerData tmp; @@ -2138,12 +2135,6 @@ lazy_scan_noprune(LVRelState *vacrel, dead_items->num_items); vacrel->lpdead_items += lpdead_items; - - /* - * Assume that we'll go on to vacuum this heap page during final pass - * over the heap. Don't record free space until then. - */ - *recordfreespace = false; } /* -- 2.39.3 (Apple Git-145)