From 619556cad4aad68d1711c12b962e9002e56d8db2 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 6 Mar 2024 21:35:11 +0200 Subject: [PATCH v6 9/9] Comment & whitespace cleanup I moved some of the paragraphs to inside the heap_vac_scan_get_next_block() function. I found the explanation in the function comment at the old place like too much detail. Someone looking at the function signature and how to call it would not care about all the details of what can or cannot be skipped. The new place isn't great either, but will do for now --- src/backend/access/heap/vacuumlazy.c | 41 ++++++++++++++++------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 535d70b71c3..b8a2dcfbbac 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -228,6 +228,7 @@ typedef struct LVSavedErrInfo VacErrPhase phase; } LVSavedErrInfo; + /* non-export function prototypes */ static void lazy_scan_heap(LVRelState *vacrel); static bool heap_vac_scan_get_next_block(LVRelState *vacrel, @@ -1068,30 +1069,17 @@ lazy_scan_heap(LVRelState *vacrel) * thresholds to skip blocks which do not need to be processed, and set blkno * to next block that actually needs to be processed. * - * A block is unskippable if it is not all visible according to the visibility - * map. It is also unskippable if it is the last block in the relation, if the - * vacuum is an aggressive vacuum, or if DISABLE_PAGE_SKIPPING was passed to - * vacuum. - * - * Even if a block is skippable, we may choose not to skip it if the range of - * skippable blocks is too small (below SKIP_PAGES_THRESHOLD). As a - * consequence, we must keep track of the next truly unskippable block and its - * visibility status separate from the next block lazy_scan_heap() should - * process (and its visibility status). - * - * The block number and visibility status of the next unskippable block are set - * in vacrel->scan_sate->next_unskippable_block and next_unskippable_allvis. - * - * The block number and visibility status of the next block to process are set - * in blkno and all_visible_according_to_vm. heap_vac_scan_get_next_block() - * returns false if there are no further blocks to process. + * The block number and visibility status of the next block to process are + * returned in blkno and all_visible_according_to_vm. + * heap_vac_scan_get_next_block() returns false if there are no further blocks + * to process. * * vacrel is an in/out parameter here; vacuum options and information about the * relation are read and vacrel->skippedallvis is set to ensure we don't * advance relfrozenxid when we have skipped vacuuming all visible blocks. * * vmbuffer will contain the block from the VM containing visibility - * information for the next unskippable heap block. We may end up needed a + * information for the next unskippable heap block. We may end up needing a * different block from the VM (if we decide not to skip a skippable block). * This is okay; visibilitymap_pin() will take care of this while processing * the block. @@ -1125,6 +1113,23 @@ heap_vac_scan_get_next_block(LVRelState *vacrel, BlockNumber *blkno, BlockNumber rel_pages = vacrel->rel_pages; BlockNumber next_unskippable_block = vacrel->get_next_block_state.next_unskippable_block; + /* + * A block is unskippable if it is not all visible according to the + * visibility map. It is also unskippable if it is the last block in + * the relation, if the vacuum is an aggressive vacuum, or if + * DISABLE_PAGE_SKIPPING was passed to vacuum. + * + * Even if a block is skippable, we may choose not to skip it if the + * range of skippable blocks is too small (below + * SKIP_PAGES_THRESHOLD). As a consequence, we must keep track of the + * next truly unskippable block and its visibility status separate + * from the next block lazy_scan_heap() should process (and its + * visibility status). + * + * The block number and visibility status of the next unskippable + * block are set in vacrel->scan_sate->next_unskippable_block and + * next_unskippable_allvis. + */ while (++next_unskippable_block < rel_pages) { uint8 mapbits = visibilitymap_get_status(vacrel->rel, -- 2.39.2