From 991c5a7ed46cc5dee36352194058ffb06a4e8670 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Sat, 30 Dec 2023 16:59:27 -0500 Subject: [PATCH v7 3/7] Confine vacuum skip logic to lazy_scan_skip In preparation for vacuum to use the streaming read interface [1] (and eventually AIO), refactor vacuum's logic for skipping blocks such that it is entirely confined to lazy_scan_skip(). This turns lazy_scan_skip() and its next block state in LVRelState into an iterator which yields blocks to lazy_scan_heap(). Such a structure is conducive to an async interface. While we are at it, rename lazy_scan_skip() to heap_vac_scan_next_block(), which now more accurately describes it. By always calling heap_vac_scan_next_block(), instead of only when we have reached the next unskippable block, we no longer need the skipping_current_range variable. Furthermore, lazy_scan_heap() no longer needs to manage the skipped range by checking if we reached the end in order to then call heap_vac_scan_next_block(). And heap_vac_scan_next_block() can derive the visibility status of a block from whether or not we are in a skippable range; that is, if the next block is equal to the next unskippable block, then the block isn't all visible, otherwise it is. [1] https://postgr.es/m/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com Discussion: https://postgr.es/m/flat/CAAKRu_Yf3gvXGcCnqqfoq0Q8LX8UM-e-qbm_B1LeZh60f8WhWA%40mail.gmail.com --- src/backend/access/heap/vacuumlazy.c | 228 ++++++++++++++------------- 1 file changed, 115 insertions(+), 113 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index accc6303fa2..8d715caccc1 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -206,19 +206,20 @@ typedef struct LVRelState int64 missed_dead_tuples; /* # removable, but not removed */ /* - * Parameters maintained by lazy_scan_skip() to manage skipping ranges of - * pages greater than SKIP_PAGES_THRESHOLD. + * Parameters maintained by heap_vac_scan_next_block() to manage getting + * the next block for vacuum to process. */ struct { - /* The last block lazy_scan_skip() returned and vacuum processed */ + /* + * The last block heap_vac_scan_next_block() returned and vacuum + * processed + */ BlockNumber current_block; /* Next unskippable block */ BlockNumber next_unskippable_block; /* Next unskippable block's visibility status */ bool next_unskippable_allvis; - /* Whether or not skippable blocks should be skipped */ - bool skipping_current_range; } next_block_state; } LVRelState; @@ -232,7 +233,9 @@ typedef struct LVSavedErrInfo /* non-export function prototypes */ static void lazy_scan_heap(LVRelState *vacrel); -static void lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer); +static bool heap_vac_scan_next_block(LVRelState *vacrel, Buffer *vmbuffer, + BlockNumber *blkno, + bool *all_visible_according_to_vm); static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, bool sharelock, Buffer vmbuffer); @@ -816,6 +819,8 @@ lazy_scan_heap(LVRelState *vacrel) BlockNumber rel_pages = vacrel->rel_pages, blkno, next_fsm_block_to_vacuum = 0; + bool all_visible_according_to_vm; + VacDeadItems *dead_items = vacrel->dead_items; Buffer vmbuffer = InvalidBuffer; const int initprog_index[] = { @@ -831,41 +836,18 @@ lazy_scan_heap(LVRelState *vacrel) initprog_val[2] = dead_items->max_items; pgstat_progress_update_multi_param(3, initprog_index, initprog_val); - /* Initialize for first lazy_scan_skip() call */ + /* Initialize for first heap_vac_scan_next_block() call */ vacrel->next_block_state.current_block = InvalidBlockNumber; vacrel->next_block_state.next_unskippable_block = InvalidBlockNumber; - /* Set up an initial range of skippable blocks using the visibility map */ - lazy_scan_skip(vacrel, &vmbuffer); - for (blkno = 0; blkno < rel_pages; blkno++) + while (heap_vac_scan_next_block(vacrel, &vmbuffer, + &blkno, &all_visible_according_to_vm)) { Buffer buf; Page page; - bool all_visible_according_to_vm; bool has_lpdead_items; bool got_cleanup_lock = false; - if (blkno == vacrel->next_block_state.next_unskippable_block) - { - /* - * Can't skip this page safely. Must scan the page. But - * determine the next skippable range after the page first. - */ - all_visible_according_to_vm = vacrel->next_block_state.next_unskippable_allvis; - lazy_scan_skip(vacrel, &vmbuffer); - } - else - { - /* Last page always scanned (may need to set nonempty_pages) */ - Assert(blkno < rel_pages - 1); - - if (vacrel->next_block_state.skipping_current_range) - continue; - - /* Current range is too small to skip -- just scan the page */ - all_visible_according_to_vm = true; - } - vacrel->scanned_pages++; /* Report as block scanned, update error traceback information */ @@ -1086,10 +1068,16 @@ lazy_scan_heap(LVRelState *vacrel) } /* - * lazy_scan_skip() -- set up range of skippable blocks using visibility map. + * heap_vac_scan_next_block() -- get next block for vacuum to process + * + * lazy_scan_heap() calls here every time it needs to get the next block to + * prune and vacuum, using the visibility map, vacuum options, and various + * thresholds to skip blocks which do not need to be processed and set blkno to + * the next block that actually needs to be processed. * - * lazy_scan_heap() calls here every time it needs to set up a new range of - * blocks to skip via the visibility map. + * 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_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, members of vacrel->next_block_state are read and set as @@ -1112,19 +1100,14 @@ lazy_scan_heap(LVRelState *vacrel) * older XIDs/MXIDs. The vacrel->skippedallvis flag will be set here when the * choice to skip such a range is actually made, making everything safe.) */ -static void -lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer) +static bool +heap_vac_scan_next_block(LVRelState *vacrel, Buffer *vmbuffer, + BlockNumber *blkno, bool *all_visible_according_to_vm) { - /* Use local variables for better optimized loop code */ - BlockNumber rel_pages = vacrel->rel_pages; /* Relies on InvalidBlockNumber + 1 == 0 */ BlockNumber next_block = vacrel->next_block_state.current_block + 1; - BlockNumber next_unskippable_block = next_block; - bool skipsallvis = false; - vacrel->next_block_state.next_unskippable_allvis = true; - /* * 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 @@ -1144,88 +1127,107 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer) * indicates to the caller whether or not it is processing a skippable * (and thus all-visible) block. */ - while (next_unskippable_block < rel_pages) + if (next_block >= vacrel->rel_pages) { - uint8 mapbits = visibilitymap_get_status(vacrel->rel, - next_unskippable_block, - vmbuffer); + vacrel->next_block_state.current_block = *blkno = InvalidBlockNumber; + return false; + } + + if (vacrel->next_block_state.next_unskippable_block == InvalidBlockNumber || + next_block > vacrel->next_block_state.next_unskippable_block) + { + /* Use local variables for better optimized loop code */ + BlockNumber rel_pages = vacrel->rel_pages; + BlockNumber next_unskippable_block = vacrel->next_block_state.next_unskippable_block; - if ((mapbits & VISIBILITYMAP_ALL_VISIBLE) == 0) + while (++next_unskippable_block < rel_pages) { - Assert((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0); - vacrel->next_block_state.next_unskippable_allvis = false; - break; - } + uint8 mapbits = visibilitymap_get_status(vacrel->rel, + next_unskippable_block, + vmbuffer); - /* - * Caller must scan the last page to determine whether it has tuples - * (caller must have the opportunity to set vacrel->nonempty_pages). - * This rule avoids having lazy_truncate_heap() take access-exclusive - * lock on rel to attempt a truncation that fails anyway, just because - * there are tuples on the last page (it is likely that there will be - * tuples on other nearby pages as well, but those can be skipped). - * - * Implement this by always treating the last block as unsafe to skip. - */ - if (next_unskippable_block == rel_pages - 1) - break; + vacrel->next_block_state.next_unskippable_allvis = mapbits & VISIBILITYMAP_ALL_VISIBLE; - /* DISABLE_PAGE_SKIPPING makes all skipping unsafe */ - if (!vacrel->skipwithvm) - { - /* Caller shouldn't rely on all_visible_according_to_vm */ - vacrel->next_block_state.next_unskippable_allvis = false; - break; - } + if (!vacrel->next_block_state.next_unskippable_allvis) + { + Assert((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0); + break; + } - /* - * Aggressive VACUUM caller can't skip pages just because they are - * all-visible. They may still skip all-frozen pages, which can't - * contain XIDs < OldestXmin (XIDs that aren't already frozen by now). - */ - if ((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0) - { - if (vacrel->aggressive) + /* + * Caller must scan the last page to determine whether it has + * tuples (caller must have the opportunity to set + * vacrel->nonempty_pages). This rule avoids having + * lazy_truncate_heap() take access-exclusive lock on rel to + * attempt a truncation that fails anyway, just because there are + * tuples on the last page (it is likely that there will be tuples + * on other nearby pages as well, but those can be skipped). + * + * Implement this by always treating the last block as unsafe to + * skip. + */ + if (next_unskippable_block == rel_pages - 1) break; + /* DISABLE_PAGE_SKIPPING makes all skipping unsafe */ + if (!vacrel->skipwithvm) + { + /* Caller shouldn't rely on all_visible_according_to_vm */ + vacrel->next_block_state.next_unskippable_allvis = false; + break; + } + /* - * All-visible block is safe to skip in non-aggressive case. But - * remember that the final range contains such a block for later. + * Aggressive VACUUM caller can't skip pages just because they are + * all-visible. They may still skip all-frozen pages, which can't + * contain XIDs < OldestXmin (XIDs that aren't already frozen by + * now). */ - skipsallvis = true; - } + if ((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0) + { + if (vacrel->aggressive) + break; - vacuum_delay_point(); - next_unskippable_block++; - } + /* + * All-visible block is safe to skip in non-aggressive case. + * But remember that the final range contains such a block for + * later. + */ + skipsallvis = true; + } - Assert(vacrel->next_block_state.next_unskippable_block >= - vacrel->next_block_state.current_block); - vacrel->next_block_state.next_unskippable_block = next_unskippable_block; + vacuum_delay_point(); + } - /* - * We only skip a range with at least SKIP_PAGES_THRESHOLD consecutive - * pages. Since we're reading sequentially, the OS should be doing - * readahead for us, so there's no gain in skipping a page now and then. - * Skipping such a range might even discourage sequential detection. - * - * This test also enables more frequent relfrozenxid advancement during - * non-aggressive VACUUMs. If the range has any all-visible pages then - * skipping makes updating relfrozenxid unsafe, which is a real downside. - */ - if (vacrel->next_block_state.next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD) - vacrel->next_block_state.skipping_current_range = false; - else - { - vacrel->next_block_state.skipping_current_range = true; - if (skipsallvis) - vacrel->skippedallvis = true; + vacrel->next_block_state.next_unskippable_block = next_unskippable_block; + + /* + * We only skip a range with at least SKIP_PAGES_THRESHOLD consecutive + * pages. Since we're reading sequentially, the OS should be doing + * readahead for us, so there's no gain in skipping a page now and + * then. Skipping such a range might even discourage sequential + * detection. + * + * This test also enables more frequent relfrozenxid advancement + * during non-aggressive VACUUMs. If the range has any all-visible + * pages then skipping makes updating relfrozenxid unsafe, which is a + * real downside. + */ + if (vacrel->next_block_state.next_unskippable_block - next_block >= SKIP_PAGES_THRESHOLD) + { + next_block = vacrel->next_block_state.next_unskippable_block; + if (skipsallvis) + vacrel->skippedallvis = true; + } } - if (next_unskippable_block >= rel_pages) - next_block = InvalidBlockNumber; + if (next_block == vacrel->next_block_state.next_unskippable_block) + *all_visible_according_to_vm = vacrel->next_block_state.next_unskippable_allvis; + else + *all_visible_according_to_vm = true; - vacrel->next_block_state.current_block = next_block; + vacrel->next_block_state.current_block = *blkno = next_block; + return true; } /* @@ -1798,8 +1800,8 @@ lazy_scan_prune(LVRelState *vacrel, /* * Handle setting visibility map bit based on information from the VM (as - * of last lazy_scan_skip() call), and from all_visible and all_frozen - * variables + * of last heap_vac_scan_next_block() call), and from all_visible and + * all_frozen variables */ if (!all_visible_according_to_vm && all_visible) { @@ -1834,8 +1836,8 @@ lazy_scan_prune(LVRelState *vacrel, /* * As of PostgreSQL 9.2, the visibility map bit should never be set if the * page-level bit is clear. However, it's possible that the bit got - * cleared after lazy_scan_skip() was called, so we must recheck with - * buffer lock before concluding that the VM is corrupt. + * cleared after heap_vac_scan_next_block() was called, so we must recheck + * with buffer lock before concluding that the VM is corrupt. */ else if (all_visible_according_to_vm && !PageIsAllVisible(page) && visibilitymap_get_status(vacrel->rel, blkno, &vmbuffer) != 0) -- 2.40.1