agora inbox for pgsql-hackers@postgresql.orghelp / color / mirror / Atom feed
[PATCH v5a 2/7] Add lazy_scan_skip unskippable state to LVRelState 3+ messages / 2 participants [nested] [flat]
* [PATCH v5a 2/7] Add lazy_scan_skip unskippable state to LVRelState @ 2023-12-30 21:22 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Melanie Plageman @ 2023-12-30 21:22 UTC (permalink / raw) Future commits will remove all skipping logic from lazy_scan_heap() and confine it to lazy_scan_skip(). To make those commits more clear, first introduce add a struct to LVRelState containing variables needed to skip ranges less than SKIP_PAGES_THRESHOLD. lazy_scan_prune() and lazy_scan_new_or_empty() can now access the buffer containing the relevant block of the visibility map through the LVRelState.skip, so it no longer needs to be a separate function parameter. While we are at it, add additional information to the lazy_scan_skip() comment, including descriptions of the role and expectations for its function parameters. --- src/backend/access/heap/vacuumlazy.c | 154 ++++++++++++++++----------- 1 file changed, 90 insertions(+), 64 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 1dc6cc8e4db..0ddb986bc03 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -204,6 +204,22 @@ typedef struct LVRelState int64 live_tuples; /* # live tuples remaining */ int64 recently_dead_tuples; /* # dead, but not yet removable */ 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. + */ + struct + { + /* Next unskippable block */ + BlockNumber next_unskippable_block; + /* Buffer containing next unskippable block's visibility info */ + Buffer vmbuffer; + /* Next unskippable block's visibility status */ + bool next_unskippable_allvis; + /* Whether or not skippable blocks should be skipped */ + bool skipping_current_range; + } skip; } LVRelState; /* Struct for saving and restoring vacuum error information. */ @@ -214,19 +230,15 @@ typedef struct LVSavedErrInfo VacErrPhase phase; } LVSavedErrInfo; - /* non-export function prototypes */ static void lazy_scan_heap(LVRelState *vacrel); -static BlockNumber lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, - BlockNumber next_block, - bool *next_unskippable_allvis, - bool *skipping_current_range); +static void lazy_scan_skip(LVRelState *vacrel, BlockNumber next_block); static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, - bool sharelock, Buffer vmbuffer); + bool sharelock); static void lazy_scan_prune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, - Buffer vmbuffer, bool all_visible_according_to_vm, + bool all_visible_according_to_vm, bool *has_lpdead_items); static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, @@ -803,12 +815,8 @@ lazy_scan_heap(LVRelState *vacrel) { BlockNumber rel_pages = vacrel->rel_pages, blkno, - next_unskippable_block, next_fsm_block_to_vacuum = 0; VacDeadItems *dead_items = vacrel->dead_items; - Buffer vmbuffer = InvalidBuffer; - bool next_unskippable_allvis, - skipping_current_range; const int initprog_index[] = { PROGRESS_VACUUM_PHASE, PROGRESS_VACUUM_TOTAL_HEAP_BLKS, @@ -822,10 +830,9 @@ lazy_scan_heap(LVRelState *vacrel) initprog_val[2] = dead_items->max_items; pgstat_progress_update_multi_param(3, initprog_index, initprog_val); + vacrel->skip.vmbuffer = InvalidBuffer; /* Set up an initial range of skippable blocks using the visibility map */ - next_unskippable_block = lazy_scan_skip(vacrel, &vmbuffer, 0, - &next_unskippable_allvis, - &skipping_current_range); + lazy_scan_skip(vacrel, 0); for (blkno = 0; blkno < rel_pages; blkno++) { Buffer buf; @@ -834,26 +841,23 @@ lazy_scan_heap(LVRelState *vacrel) bool has_lpdead_items; bool got_cleanup_lock = false; - if (blkno == next_unskippable_block) + if (blkno == vacrel->skip.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 = next_unskippable_allvis; - next_unskippable_block = lazy_scan_skip(vacrel, &vmbuffer, - blkno + 1, - &next_unskippable_allvis, - &skipping_current_range); + all_visible_according_to_vm = vacrel->skip.next_unskippable_allvis; + lazy_scan_skip(vacrel, blkno + 1); - Assert(next_unskippable_block >= blkno + 1); + Assert(vacrel->skip.next_unskippable_block >= blkno + 1); } else { /* Last page always scanned (may need to set nonempty_pages) */ Assert(blkno < rel_pages - 1); - if (skipping_current_range) + if (vacrel->skip.skipping_current_range) continue; /* Current range is too small to skip -- just scan the page */ @@ -896,10 +900,10 @@ lazy_scan_heap(LVRelState *vacrel) * correctness, but we do it anyway to avoid holding the pin * across a lengthy, unrelated operation. */ - if (BufferIsValid(vmbuffer)) + if (BufferIsValid(vacrel->skip.vmbuffer)) { - ReleaseBuffer(vmbuffer); - vmbuffer = InvalidBuffer; + ReleaseBuffer(vacrel->skip.vmbuffer); + vacrel->skip.vmbuffer = InvalidBuffer; } /* Perform a round of index and heap vacuuming */ @@ -924,7 +928,7 @@ lazy_scan_heap(LVRelState *vacrel) * all-visible. In most cases this will be very cheap, because we'll * already have the correct page pinned anyway. */ - visibilitymap_pin(vacrel->rel, blkno, &vmbuffer); + visibilitymap_pin(vacrel->rel, blkno, &vacrel->skip.vmbuffer); buf = ReadBufferExtended(vacrel->rel, MAIN_FORKNUM, blkno, RBM_NORMAL, vacrel->bstrategy); @@ -942,8 +946,7 @@ lazy_scan_heap(LVRelState *vacrel) LockBuffer(buf, BUFFER_LOCK_SHARE); /* Check for new or empty pages before lazy_scan_[no]prune call */ - if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, !got_cleanup_lock, - vmbuffer)) + if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, !got_cleanup_lock)) { /* Processed as new/empty page (lock and pin released) */ continue; @@ -985,7 +988,7 @@ lazy_scan_heap(LVRelState *vacrel) */ if (got_cleanup_lock) lazy_scan_prune(vacrel, buf, blkno, page, - vmbuffer, all_visible_according_to_vm, + all_visible_according_to_vm, &has_lpdead_items); /* @@ -1035,8 +1038,11 @@ lazy_scan_heap(LVRelState *vacrel) } vacrel->blkno = InvalidBlockNumber; - if (BufferIsValid(vmbuffer)) - ReleaseBuffer(vmbuffer); + if (BufferIsValid(vacrel->skip.vmbuffer)) + { + ReleaseBuffer(vacrel->skip.vmbuffer); + vacrel->skip.vmbuffer = InvalidBuffer; + } /* report that everything is now scanned */ pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno); @@ -1080,15 +1086,34 @@ lazy_scan_heap(LVRelState *vacrel) * lazy_scan_skip() -- set up range of skippable blocks using visibility map. * * lazy_scan_heap() calls here every time it needs to set up a new range of - * blocks to skip via the visibility map. Caller passes the next block in - * line. We return a next_unskippable_block for this range. When there are - * no skippable blocks we just return caller's next_block. The all-visible - * status of the returned block is set in *next_unskippable_allvis for caller, - * too. Block usually won't be all-visible (since it's unskippable), but it - * can be during aggressive VACUUMs (as well as in certain edge cases). + * blocks to skip via the visibility map. Caller passes next_block, the next + * block in line. The parameters of the skipped range are recorded in skip. + * 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. + * + * skip->vmbuffer will contain the block from the VM containing visibility + * information for the next unskippable heap block. We may end up needed 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. + * + * 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. * - * Sets *skipping_current_range to indicate if caller should skip this range. - * Costs and benefits drive our decision. Very small ranges won't be skipped. + * 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 along with whether or not we are skipping the current + * range of skippable blocks. This can be used to derive the next block + * lazy_scan_heap() must process and its visibility status. + * + * The block number and visibility status of the next unskippable block are set + * in skip->next_unskippable_block and next_unskippable_allvis. + * skip->skipping_current_range indicates to the caller whether or not it is + * processing a skippable (and thus all-visible) block. * * Note: our opinion of which blocks can be skipped can go stale immediately. * It's okay if caller "misses" a page whose all-visible or all-frozen marking @@ -1098,25 +1123,26 @@ 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 BlockNumber -lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, - bool *next_unskippable_allvis, bool *skipping_current_range) +static void +lazy_scan_skip(LVRelState *vacrel, BlockNumber next_block) { + /* Use local variables for better optimized loop code */ BlockNumber rel_pages = vacrel->rel_pages, next_unskippable_block = next_block; + bool skipsallvis = false; - *next_unskippable_allvis = true; + vacrel->skip.next_unskippable_allvis = true; while (next_unskippable_block < rel_pages) { uint8 mapbits = visibilitymap_get_status(vacrel->rel, next_unskippable_block, - vmbuffer); + &vacrel->skip.vmbuffer); if ((mapbits & VISIBILITYMAP_ALL_VISIBLE) == 0) { Assert((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0); - *next_unskippable_allvis = false; + vacrel->skip.next_unskippable_allvis = false; break; } @@ -1137,7 +1163,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, if (!vacrel->skipwithvm) { /* Caller shouldn't rely on all_visible_according_to_vm */ - *next_unskippable_allvis = false; + vacrel->skip.next_unskippable_allvis = false; break; } @@ -1162,6 +1188,8 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, next_unskippable_block++; } + vacrel->skip.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 @@ -1172,16 +1200,14 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, * non-aggressive VACUUMs. If the range has any all-visible pages then * skipping makes updating relfrozenxid unsafe, which is a real downside. */ - if (next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD) - *skipping_current_range = false; + if (vacrel->skip.next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD) + vacrel->skip.skipping_current_range = false; else { - *skipping_current_range = true; + vacrel->skip.skipping_current_range = true; if (skipsallvis) vacrel->skippedallvis = true; } - - return next_unskippable_block; } /* @@ -1214,7 +1240,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, */ static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, - Page page, bool sharelock, Buffer vmbuffer) + Page page, bool sharelock) { Size freespace; @@ -1300,7 +1326,7 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, PageSetAllVisible(page); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, - vmbuffer, InvalidTransactionId, + vacrel->skip.vmbuffer, InvalidTransactionId, VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN); END_CRIT_SECTION(); } @@ -1336,10 +1362,11 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, * any tuple that becomes dead after the call to heap_page_prune() can't need to * be frozen, because it was visible to another session when vacuum started. * - * vmbuffer is the buffer containing the VM block with visibility information - * for the heap block, blkno. all_visible_according_to_vm is the saved - * visibility status of the heap block looked up earlier by the caller. We - * won't rely entirely on this status, as it may be out of date. + * vacrel->skipstate.vmbuffer is the buffer containing the VM block with + * visibility information for the heap block, blkno. + * all_visible_according_to_vm is the saved visibility status of the heap block + * looked up earlier by the caller. We won't rely entirely on this status, as + * it may be out of date. * * *has_lpdead_items is set to true or false depending on whether, upon return * from this function, any LP_DEAD items are still present on the page. @@ -1349,7 +1376,6 @@ lazy_scan_prune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, - Buffer vmbuffer, bool all_visible_according_to_vm, bool *has_lpdead_items) { @@ -1783,7 +1809,7 @@ lazy_scan_prune(LVRelState *vacrel, PageSetAllVisible(page); MarkBufferDirty(buf); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, - vmbuffer, visibility_cutoff_xid, + vacrel->skip.vmbuffer, visibility_cutoff_xid, flags); } @@ -1794,11 +1820,11 @@ lazy_scan_prune(LVRelState *vacrel, * 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) + visibilitymap_get_status(vacrel->rel, blkno, &vacrel->skip.vmbuffer) != 0) { elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u", vacrel->relname, blkno); - visibilitymap_clear(vacrel->rel, blkno, vmbuffer, + visibilitymap_clear(vacrel->rel, blkno, vacrel->skip.vmbuffer, VISIBILITYMAP_VALID_BITS); } @@ -1822,7 +1848,7 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->relname, blkno); PageClearAllVisible(page); MarkBufferDirty(buf); - visibilitymap_clear(vacrel->rel, blkno, vmbuffer, + visibilitymap_clear(vacrel->rel, blkno, vacrel->skip.vmbuffer, VISIBILITYMAP_VALID_BITS); } @@ -1832,7 +1858,7 @@ lazy_scan_prune(LVRelState *vacrel, * true, so we must check both all_visible and all_frozen. */ else if (all_visible_according_to_vm && all_visible && - all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer)) + all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vacrel->skip.vmbuffer)) { /* * Avoid relying on all_visible_according_to_vm as a proxy for the @@ -1854,7 +1880,7 @@ lazy_scan_prune(LVRelState *vacrel, */ Assert(!TransactionIdIsValid(visibility_cutoff_xid)); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, - vmbuffer, InvalidTransactionId, + vacrel->skip.vmbuffer, InvalidTransactionId, VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN); } -- 2.40.1 --lb5e4aqx4b6ik5lq Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5a-0003-Confine-vacuum-skip-logic-to-lazy_scan_skip.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* [PATCH v5a 2/7] Add lazy_scan_skip unskippable state to LVRelState @ 2023-12-30 21:22 Melanie Plageman <melanieplageman@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Melanie Plageman @ 2023-12-30 21:22 UTC (permalink / raw) Future commits will remove all skipping logic from lazy_scan_heap() and confine it to lazy_scan_skip(). To make those commits more clear, first introduce add a struct to LVRelState containing variables needed to skip ranges less than SKIP_PAGES_THRESHOLD. lazy_scan_prune() and lazy_scan_new_or_empty() can now access the buffer containing the relevant block of the visibility map through the LVRelState.skip, so it no longer needs to be a separate function parameter. While we are at it, add additional information to the lazy_scan_skip() comment, including descriptions of the role and expectations for its function parameters. --- src/backend/access/heap/vacuumlazy.c | 154 ++++++++++++++++----------- 1 file changed, 90 insertions(+), 64 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 1dc6cc8e4db..0ddb986bc03 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -204,6 +204,22 @@ typedef struct LVRelState int64 live_tuples; /* # live tuples remaining */ int64 recently_dead_tuples; /* # dead, but not yet removable */ 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. + */ + struct + { + /* Next unskippable block */ + BlockNumber next_unskippable_block; + /* Buffer containing next unskippable block's visibility info */ + Buffer vmbuffer; + /* Next unskippable block's visibility status */ + bool next_unskippable_allvis; + /* Whether or not skippable blocks should be skipped */ + bool skipping_current_range; + } skip; } LVRelState; /* Struct for saving and restoring vacuum error information. */ @@ -214,19 +230,15 @@ typedef struct LVSavedErrInfo VacErrPhase phase; } LVSavedErrInfo; - /* non-export function prototypes */ static void lazy_scan_heap(LVRelState *vacrel); -static BlockNumber lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, - BlockNumber next_block, - bool *next_unskippable_allvis, - bool *skipping_current_range); +static void lazy_scan_skip(LVRelState *vacrel, BlockNumber next_block); static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, - bool sharelock, Buffer vmbuffer); + bool sharelock); static void lazy_scan_prune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, - Buffer vmbuffer, bool all_visible_according_to_vm, + bool all_visible_according_to_vm, bool *has_lpdead_items); static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, @@ -803,12 +815,8 @@ lazy_scan_heap(LVRelState *vacrel) { BlockNumber rel_pages = vacrel->rel_pages, blkno, - next_unskippable_block, next_fsm_block_to_vacuum = 0; VacDeadItems *dead_items = vacrel->dead_items; - Buffer vmbuffer = InvalidBuffer; - bool next_unskippable_allvis, - skipping_current_range; const int initprog_index[] = { PROGRESS_VACUUM_PHASE, PROGRESS_VACUUM_TOTAL_HEAP_BLKS, @@ -822,10 +830,9 @@ lazy_scan_heap(LVRelState *vacrel) initprog_val[2] = dead_items->max_items; pgstat_progress_update_multi_param(3, initprog_index, initprog_val); + vacrel->skip.vmbuffer = InvalidBuffer; /* Set up an initial range of skippable blocks using the visibility map */ - next_unskippable_block = lazy_scan_skip(vacrel, &vmbuffer, 0, - &next_unskippable_allvis, - &skipping_current_range); + lazy_scan_skip(vacrel, 0); for (blkno = 0; blkno < rel_pages; blkno++) { Buffer buf; @@ -834,26 +841,23 @@ lazy_scan_heap(LVRelState *vacrel) bool has_lpdead_items; bool got_cleanup_lock = false; - if (blkno == next_unskippable_block) + if (blkno == vacrel->skip.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 = next_unskippable_allvis; - next_unskippable_block = lazy_scan_skip(vacrel, &vmbuffer, - blkno + 1, - &next_unskippable_allvis, - &skipping_current_range); + all_visible_according_to_vm = vacrel->skip.next_unskippable_allvis; + lazy_scan_skip(vacrel, blkno + 1); - Assert(next_unskippable_block >= blkno + 1); + Assert(vacrel->skip.next_unskippable_block >= blkno + 1); } else { /* Last page always scanned (may need to set nonempty_pages) */ Assert(blkno < rel_pages - 1); - if (skipping_current_range) + if (vacrel->skip.skipping_current_range) continue; /* Current range is too small to skip -- just scan the page */ @@ -896,10 +900,10 @@ lazy_scan_heap(LVRelState *vacrel) * correctness, but we do it anyway to avoid holding the pin * across a lengthy, unrelated operation. */ - if (BufferIsValid(vmbuffer)) + if (BufferIsValid(vacrel->skip.vmbuffer)) { - ReleaseBuffer(vmbuffer); - vmbuffer = InvalidBuffer; + ReleaseBuffer(vacrel->skip.vmbuffer); + vacrel->skip.vmbuffer = InvalidBuffer; } /* Perform a round of index and heap vacuuming */ @@ -924,7 +928,7 @@ lazy_scan_heap(LVRelState *vacrel) * all-visible. In most cases this will be very cheap, because we'll * already have the correct page pinned anyway. */ - visibilitymap_pin(vacrel->rel, blkno, &vmbuffer); + visibilitymap_pin(vacrel->rel, blkno, &vacrel->skip.vmbuffer); buf = ReadBufferExtended(vacrel->rel, MAIN_FORKNUM, blkno, RBM_NORMAL, vacrel->bstrategy); @@ -942,8 +946,7 @@ lazy_scan_heap(LVRelState *vacrel) LockBuffer(buf, BUFFER_LOCK_SHARE); /* Check for new or empty pages before lazy_scan_[no]prune call */ - if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, !got_cleanup_lock, - vmbuffer)) + if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, !got_cleanup_lock)) { /* Processed as new/empty page (lock and pin released) */ continue; @@ -985,7 +988,7 @@ lazy_scan_heap(LVRelState *vacrel) */ if (got_cleanup_lock) lazy_scan_prune(vacrel, buf, blkno, page, - vmbuffer, all_visible_according_to_vm, + all_visible_according_to_vm, &has_lpdead_items); /* @@ -1035,8 +1038,11 @@ lazy_scan_heap(LVRelState *vacrel) } vacrel->blkno = InvalidBlockNumber; - if (BufferIsValid(vmbuffer)) - ReleaseBuffer(vmbuffer); + if (BufferIsValid(vacrel->skip.vmbuffer)) + { + ReleaseBuffer(vacrel->skip.vmbuffer); + vacrel->skip.vmbuffer = InvalidBuffer; + } /* report that everything is now scanned */ pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno); @@ -1080,15 +1086,34 @@ lazy_scan_heap(LVRelState *vacrel) * lazy_scan_skip() -- set up range of skippable blocks using visibility map. * * lazy_scan_heap() calls here every time it needs to set up a new range of - * blocks to skip via the visibility map. Caller passes the next block in - * line. We return a next_unskippable_block for this range. When there are - * no skippable blocks we just return caller's next_block. The all-visible - * status of the returned block is set in *next_unskippable_allvis for caller, - * too. Block usually won't be all-visible (since it's unskippable), but it - * can be during aggressive VACUUMs (as well as in certain edge cases). + * blocks to skip via the visibility map. Caller passes next_block, the next + * block in line. The parameters of the skipped range are recorded in skip. + * 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. + * + * skip->vmbuffer will contain the block from the VM containing visibility + * information for the next unskippable heap block. We may end up needed 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. + * + * 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. * - * Sets *skipping_current_range to indicate if caller should skip this range. - * Costs and benefits drive our decision. Very small ranges won't be skipped. + * 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 along with whether or not we are skipping the current + * range of skippable blocks. This can be used to derive the next block + * lazy_scan_heap() must process and its visibility status. + * + * The block number and visibility status of the next unskippable block are set + * in skip->next_unskippable_block and next_unskippable_allvis. + * skip->skipping_current_range indicates to the caller whether or not it is + * processing a skippable (and thus all-visible) block. * * Note: our opinion of which blocks can be skipped can go stale immediately. * It's okay if caller "misses" a page whose all-visible or all-frozen marking @@ -1098,25 +1123,26 @@ 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 BlockNumber -lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, - bool *next_unskippable_allvis, bool *skipping_current_range) +static void +lazy_scan_skip(LVRelState *vacrel, BlockNumber next_block) { + /* Use local variables for better optimized loop code */ BlockNumber rel_pages = vacrel->rel_pages, next_unskippable_block = next_block; + bool skipsallvis = false; - *next_unskippable_allvis = true; + vacrel->skip.next_unskippable_allvis = true; while (next_unskippable_block < rel_pages) { uint8 mapbits = visibilitymap_get_status(vacrel->rel, next_unskippable_block, - vmbuffer); + &vacrel->skip.vmbuffer); if ((mapbits & VISIBILITYMAP_ALL_VISIBLE) == 0) { Assert((mapbits & VISIBILITYMAP_ALL_FROZEN) == 0); - *next_unskippable_allvis = false; + vacrel->skip.next_unskippable_allvis = false; break; } @@ -1137,7 +1163,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, if (!vacrel->skipwithvm) { /* Caller shouldn't rely on all_visible_according_to_vm */ - *next_unskippable_allvis = false; + vacrel->skip.next_unskippable_allvis = false; break; } @@ -1162,6 +1188,8 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, next_unskippable_block++; } + vacrel->skip.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 @@ -1172,16 +1200,14 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, * non-aggressive VACUUMs. If the range has any all-visible pages then * skipping makes updating relfrozenxid unsafe, which is a real downside. */ - if (next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD) - *skipping_current_range = false; + if (vacrel->skip.next_unskippable_block - next_block < SKIP_PAGES_THRESHOLD) + vacrel->skip.skipping_current_range = false; else { - *skipping_current_range = true; + vacrel->skip.skipping_current_range = true; if (skipsallvis) vacrel->skippedallvis = true; } - - return next_unskippable_block; } /* @@ -1214,7 +1240,7 @@ lazy_scan_skip(LVRelState *vacrel, Buffer *vmbuffer, BlockNumber next_block, */ static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, - Page page, bool sharelock, Buffer vmbuffer) + Page page, bool sharelock) { Size freespace; @@ -1300,7 +1326,7 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, PageSetAllVisible(page); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, - vmbuffer, InvalidTransactionId, + vacrel->skip.vmbuffer, InvalidTransactionId, VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN); END_CRIT_SECTION(); } @@ -1336,10 +1362,11 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, * any tuple that becomes dead after the call to heap_page_prune() can't need to * be frozen, because it was visible to another session when vacuum started. * - * vmbuffer is the buffer containing the VM block with visibility information - * for the heap block, blkno. all_visible_according_to_vm is the saved - * visibility status of the heap block looked up earlier by the caller. We - * won't rely entirely on this status, as it may be out of date. + * vacrel->skipstate.vmbuffer is the buffer containing the VM block with + * visibility information for the heap block, blkno. + * all_visible_according_to_vm is the saved visibility status of the heap block + * looked up earlier by the caller. We won't rely entirely on this status, as + * it may be out of date. * * *has_lpdead_items is set to true or false depending on whether, upon return * from this function, any LP_DEAD items are still present on the page. @@ -1349,7 +1376,6 @@ lazy_scan_prune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, - Buffer vmbuffer, bool all_visible_according_to_vm, bool *has_lpdead_items) { @@ -1783,7 +1809,7 @@ lazy_scan_prune(LVRelState *vacrel, PageSetAllVisible(page); MarkBufferDirty(buf); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, - vmbuffer, visibility_cutoff_xid, + vacrel->skip.vmbuffer, visibility_cutoff_xid, flags); } @@ -1794,11 +1820,11 @@ lazy_scan_prune(LVRelState *vacrel, * 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) + visibilitymap_get_status(vacrel->rel, blkno, &vacrel->skip.vmbuffer) != 0) { elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u", vacrel->relname, blkno); - visibilitymap_clear(vacrel->rel, blkno, vmbuffer, + visibilitymap_clear(vacrel->rel, blkno, vacrel->skip.vmbuffer, VISIBILITYMAP_VALID_BITS); } @@ -1822,7 +1848,7 @@ lazy_scan_prune(LVRelState *vacrel, vacrel->relname, blkno); PageClearAllVisible(page); MarkBufferDirty(buf); - visibilitymap_clear(vacrel->rel, blkno, vmbuffer, + visibilitymap_clear(vacrel->rel, blkno, vacrel->skip.vmbuffer, VISIBILITYMAP_VALID_BITS); } @@ -1832,7 +1858,7 @@ lazy_scan_prune(LVRelState *vacrel, * true, so we must check both all_visible and all_frozen. */ else if (all_visible_according_to_vm && all_visible && - all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer)) + all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vacrel->skip.vmbuffer)) { /* * Avoid relying on all_visible_according_to_vm as a proxy for the @@ -1854,7 +1880,7 @@ lazy_scan_prune(LVRelState *vacrel, */ Assert(!TransactionIdIsValid(visibility_cutoff_xid)); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, - vmbuffer, InvalidTransactionId, + vacrel->skip.vmbuffer, InvalidTransactionId, VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN); } -- 2.40.1 --lb5e4aqx4b6ik5lq Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5a-0003-Confine-vacuum-skip-logic-to-lazy_scan_skip.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* [PATCH v47 2/9] give options bitmask to table_delete/table_update @ 2026-03-30 11:27 Álvaro Herrera <alvherre@kurilemu.de> 0 siblings, 0 replies; 3+ messages in thread From: Álvaro Herrera @ 2026-03-30 11:27 UTC (permalink / raw) --- src/backend/access/heap/heapam.c | 16 ++++++++++------ src/backend/access/heap/heapam_handler.c | 13 ++++++++----- src/backend/access/table/tableam.c | 6 +++--- src/backend/executor/nodeModifyTable.c | 9 +++++++-- src/include/access/heapam.h | 7 ++++--- src/include/access/tableam.h | 23 +++++++++++++++-------- 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index d34136d2e94..0645b2d5f58 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2862,8 +2862,8 @@ xmax_infomask_changed(uint16 new_infomask, uint16 old_infomask) */ TM_Result heap_delete(Relation relation, const ItemPointerData *tid, - CommandId cid, Snapshot crosscheck, bool wait, - TM_FailureData *tmfd, bool changingPart) + CommandId cid, uint32 options, Snapshot crosscheck, + bool wait, TM_FailureData *tmfd) { TM_Result result; TransactionId xid = GetCurrentTransactionId(); @@ -2876,6 +2876,7 @@ heap_delete(Relation relation, const ItemPointerData *tid, TransactionId new_xmax; uint16 new_infomask, new_infomask2; + bool changingPart = (options & TABLE_DELETE_CHANGING_PARTITION) != 0; bool have_tuple_lock = false; bool iscombo; bool all_visible_cleared = false; @@ -3290,9 +3291,11 @@ simple_heap_delete(Relation relation, const ItemPointerData *tid) TM_FailureData tmfd; result = heap_delete(relation, tid, - GetCurrentCommandId(true), InvalidSnapshot, + GetCurrentCommandId(true), + 0, + InvalidSnapshot, true /* wait for commit */ , - &tmfd, false /* changingPart */ ); + &tmfd); switch (result) { case TM_SelfModified: @@ -3331,7 +3334,7 @@ simple_heap_delete(Relation relation, const ItemPointerData *tid) */ TM_Result heap_update(Relation relation, const ItemPointerData *otid, HeapTuple newtup, - CommandId cid, Snapshot crosscheck, bool wait, + CommandId cid, uint32 options, Snapshot crosscheck, bool wait, TM_FailureData *tmfd, LockTupleMode *lockmode, TU_UpdateIndexes *update_indexes) { @@ -4585,7 +4588,8 @@ simple_heap_update(Relation relation, const ItemPointerData *otid, HeapTuple tup LockTupleMode lockmode; result = heap_update(relation, otid, tup, - GetCurrentCommandId(true), InvalidSnapshot, + GetCurrentCommandId(true), 0, + InvalidSnapshot, true /* wait for commit */ , &tmfd, &lockmode, update_indexes); switch (result) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index cdd153c6b6d..69debeff516 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -313,21 +313,23 @@ heapam_tuple_complete_speculative(Relation relation, TupleTableSlot *slot, static TM_Result heapam_tuple_delete(Relation relation, ItemPointer tid, CommandId cid, - Snapshot snapshot, Snapshot crosscheck, bool wait, - TM_FailureData *tmfd, bool changingPart) + uint32 options, Snapshot snapshot, Snapshot crosscheck, + bool wait, TM_FailureData *tmfd) { /* * Currently Deleting of index tuples are handled at vacuum, in case if * the storage itself is cleaning the dead tuples by itself, it is the * time to call the index tuple deletion also. */ - return heap_delete(relation, tid, cid, crosscheck, wait, tmfd, changingPart); + return heap_delete(relation, tid, cid, options, crosscheck, wait, + tmfd); } static TM_Result heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot, - CommandId cid, Snapshot snapshot, Snapshot crosscheck, + CommandId cid, uint32 options pg_attribute_unused(), + Snapshot snapshot, Snapshot crosscheck, bool wait, TM_FailureData *tmfd, LockTupleMode *lockmode, TU_UpdateIndexes *update_indexes) { @@ -339,7 +341,8 @@ heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot, slot->tts_tableOid = RelationGetRelid(relation); tuple->t_tableOid = slot->tts_tableOid; - result = heap_update(relation, otid, tuple, cid, crosscheck, wait, + result = heap_update(relation, otid, tuple, cid, options, + crosscheck, wait, tmfd, lockmode, update_indexes); ItemPointerCopy(&tuple->t_self, &slot->tts_tid); diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 86481d7c029..68ff0966f1c 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -320,9 +320,9 @@ simple_table_tuple_delete(Relation rel, ItemPointer tid, Snapshot snapshot) result = table_tuple_delete(rel, tid, GetCurrentCommandId(true), - snapshot, InvalidSnapshot, + 0, snapshot, InvalidSnapshot, true /* wait for commit */ , - &tmfd, false /* changingPart */ ); + &tmfd); switch (result) { @@ -369,7 +369,7 @@ simple_table_tuple_update(Relation rel, ItemPointer otid, result = table_tuple_update(rel, otid, slot, GetCurrentCommandId(true), - snapshot, InvalidSnapshot, + 0, snapshot, InvalidSnapshot, true /* wait for commit */ , &tmfd, &lockmode, update_indexes); diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 582bcc367c0..76728f08734 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1522,14 +1522,18 @@ ExecDeleteAct(ModifyTableContext *context, ResultRelInfo *resultRelInfo, ItemPointer tupleid, bool changingPart) { EState *estate = context->estate; + uint32 options = 0; + + if (changingPart) + options |= TABLE_DELETE_CHANGING_PARTITION; return table_tuple_delete(resultRelInfo->ri_RelationDesc, tupleid, estate->es_output_cid, + options, estate->es_snapshot, estate->es_crosscheck_snapshot, true /* wait for commit */ , - &context->tmfd, - changingPart); + &context->tmfd); } /* @@ -2331,6 +2335,7 @@ lreplace: */ result = table_tuple_update(resultRelationDesc, tupleid, slot, estate->es_output_cid, + 0, estate->es_snapshot, estate->es_crosscheck_snapshot, true /* wait for commit */ , diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 6018dacf0f7..77560b22877 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -382,13 +382,14 @@ extern void heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, CommandId cid, uint32 options, BulkInsertState bistate); extern TM_Result heap_delete(Relation relation, const ItemPointerData *tid, - CommandId cid, Snapshot crosscheck, bool wait, - TM_FailureData *tmfd, bool changingPart); + CommandId cid, uint32 options, Snapshot crosscheck, + bool wait, TM_FailureData *tmfd); extern void heap_finish_speculative(Relation relation, const ItemPointerData *tid); extern void heap_abort_speculative(Relation relation, const ItemPointerData *tid); extern TM_Result heap_update(Relation relation, const ItemPointerData *otid, HeapTuple newtup, - CommandId cid, Snapshot crosscheck, bool wait, + CommandId cid, uint32 options, + Snapshot crosscheck, bool wait, TM_FailureData *tmfd, LockTupleMode *lockmode, TU_UpdateIndexes *update_indexes); extern TM_Result heap_lock_tuple(Relation relation, HeapTuple tuple, diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index ab2e7fc1dfe..6cdd949f58d 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -287,6 +287,11 @@ typedef struct TM_IndexDeleteOp /* Follow update chain and lock latest version of tuple */ #define TUPLE_LOCK_FLAG_FIND_LAST_VERSION (1 << 1) +/* "options" flag bits for table_tuple_delete */ +#define TABLE_DELETE_CHANGING_PARTITION (1 << 0) + +/* "options" flag bits for table_tuple_update */ +/* XXX none at present */ /* Typedef for callback function for table_index_build_scan */ typedef void (*IndexBuildCallback) (Relation index, @@ -558,17 +563,18 @@ typedef struct TableAmRoutine TM_Result (*tuple_delete) (Relation rel, ItemPointer tid, CommandId cid, + uint32 options, Snapshot snapshot, Snapshot crosscheck, bool wait, - TM_FailureData *tmfd, - bool changingPart); + TM_FailureData *tmfd); /* see table_tuple_update() for reference about parameters */ TM_Result (*tuple_update) (Relation rel, ItemPointer otid, TupleTableSlot *slot, CommandId cid, + uint32 options, Snapshot snapshot, Snapshot crosscheck, bool wait, @@ -1533,12 +1539,12 @@ table_multi_insert(Relation rel, TupleTableSlot **slots, int nslots, */ static inline TM_Result table_tuple_delete(Relation rel, ItemPointer tid, CommandId cid, - Snapshot snapshot, Snapshot crosscheck, bool wait, - TM_FailureData *tmfd, bool changingPart) + uint32 options, Snapshot snapshot, Snapshot crosscheck, + bool wait, TM_FailureData *tmfd) { - return rel->rd_tableam->tuple_delete(rel, tid, cid, + return rel->rd_tableam->tuple_delete(rel, tid, cid, options, snapshot, crosscheck, - wait, tmfd, changingPart); + wait, tmfd); } /* @@ -1578,12 +1584,13 @@ table_tuple_delete(Relation rel, ItemPointer tid, CommandId cid, */ static inline TM_Result table_tuple_update(Relation rel, ItemPointer otid, TupleTableSlot *slot, - CommandId cid, Snapshot snapshot, Snapshot crosscheck, + CommandId cid, uint32 options, + Snapshot snapshot, Snapshot crosscheck, bool wait, TM_FailureData *tmfd, LockTupleMode *lockmode, TU_UpdateIndexes *update_indexes) { return rel->rd_tableam->tuple_update(rel, otid, slot, - cid, snapshot, crosscheck, + cid, options, snapshot, crosscheck, wait, tmfd, lockmode, update_indexes); } -- 2.47.3 --2fxmamo6mu2qbxgv Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v47-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-03-30 11:27 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2023-12-30 21:22 [PATCH v5a 2/7] Add lazy_scan_skip unskippable state to LVRelState Melanie Plageman <melanieplageman@gmail.com> 2023-12-30 21:22 [PATCH v5a 2/7] Add lazy_scan_skip unskippable state to LVRelState Melanie Plageman <melanieplageman@gmail.com> 2026-03-30 11:27 [PATCH v47 2/9] give options bitmask to table_delete/table_update Álvaro Herrera <alvherre@kurilemu.de>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox