From e5fba63482e7d3bd44a991773ac3da50d2402781 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 16 Sep 2025 10:39:31 -0400 Subject: [PATCH v15 09/23] Vacuum phase III set PD_ALL_VISIBLE in vacuum WAL record Instead of setting PD_ALL_VISIBLE on the heap page when setting bits in the VM, set it when flipping the line pointers on the page to LP_UNUSED. This will allow us to omit the heap page from the VM WAL chain. To do this, we must check if the page will be all-visible once we flip the line pointers before we actually do so. One functional change is that a single critical section surrounds both the VM update and the heap update. Previously they were each in a critical section, so we could crash and have set PD_ALL_VISIBLE but not set bits in the VM. --- src/backend/access/heap/vacuumlazy.c | 140 ++++++++++++++++++++------- 1 file changed, 105 insertions(+), 35 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 5a6bbbd97f2..9bfcd67a61b 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -465,6 +465,11 @@ static void dead_items_reset(LVRelState *vacrel); static void dead_items_cleanup(LVRelState *vacrel); static bool heap_page_is_all_visible(LVRelState *vacrel, Buffer buf, TransactionId *visibility_cutoff_xid, bool *all_frozen); +static bool heap_page_would_be_all_visible(LVRelState *vacrel, Buffer buf, + OffsetNumber *deadoffsets, + int ndeadoffsets, + bool *all_frozen, + TransactionId *visibility_cutoff_xid); static void update_relstats_all_indexes(LVRelState *vacrel); static void vacuum_error_callback(void *arg); static void update_vacuum_error_info(LVRelState *vacrel, @@ -2793,6 +2798,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, TransactionId visibility_cutoff_xid; bool all_frozen; LVSavedErrInfo saved_err_info; + uint8 vmflags = 0; Assert(vacrel->do_index_vacuuming); @@ -2803,6 +2809,18 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, VACUUM_ERRCB_PHASE_VACUUM_HEAP, blkno, InvalidOffsetNumber); + if (heap_page_would_be_all_visible(vacrel, buffer, + deadoffsets, num_offsets, + &all_frozen, &visibility_cutoff_xid)) + { + vmflags |= VISIBILITYMAP_ALL_VISIBLE; + if (all_frozen) + { + vmflags |= VISIBILITYMAP_ALL_FROZEN; + Assert(!TransactionIdIsValid(visibility_cutoff_xid)); + } + } + START_CRIT_SECTION(); for (int i = 0; i < num_offsets; i++) @@ -2822,6 +2840,13 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, /* Attempt to truncate line pointer array now */ PageTruncateLinePointerArray(page); + /* + * The page will never have PD_ALL_VISIBLE already set, so if we are + * setting the VM, we must set PD_ALL_VISIBLE as well. + */ + if ((vmflags & VISIBILITYMAP_VALID_BITS) != 0) + PageSetAllVisible(page); + /* * Mark buffer dirty before we write WAL. */ @@ -2833,7 +2858,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, log_heap_prune_and_freeze(vacrel->rel, buffer, InvalidTransactionId, false, /* no cleanup lock required */ - false, + (vmflags & VISIBILITYMAP_VALID_BITS) != 0, PRUNE_VACUUM_CLEANUP, NULL, 0, /* frozen */ NULL, 0, /* redirected */ @@ -2842,36 +2867,26 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, } /* - * End critical section, so we safely can do visibility tests (which - * possibly need to perform IO and allocate memory!). If we crash now the - * page (including the corresponding vm bit) might not be marked all - * visible, but that's fine. A later vacuum will fix that. + * Note that we don't end the critical section until after emitting the VM + * record. This ensures both PD_ALL_VISIBLE and the VM bits are set or + * unset in the event of a crash. While it is correct for PD_ALL_VISIBLE + * to be set and the VM to be clear, we should do our best to keep these + * in sync. This does mean that we will take a lock on the VM buffer + * inside of a critical section, which is generally discouraged. There is + * precedent for this in other callers of visibilitymap_set(), though. */ - END_CRIT_SECTION(); /* - * Now that we have removed the LP_DEAD items from the page, once again - * check if the page has become all-visible. The page is already marked - * dirty, exclusively locked, and, if needed, a full page image has been - * emitted. + * Now that we have removed the LP_DEAD items from the page, set the + * visibility map if the page became all-visible/all-frozen. Changes to + * the heap page have already been logged. */ - Assert(!PageIsAllVisible(page)); - if (heap_page_is_all_visible(vacrel, buffer, &visibility_cutoff_xid, - &all_frozen)) + if ((vmflags & VISIBILITYMAP_ALL_VISIBLE) != 0) { - uint8 flags = VISIBILITYMAP_ALL_VISIBLE; - - if (all_frozen) - { - Assert(!TransactionIdIsValid(visibility_cutoff_xid)); - flags |= VISIBILITYMAP_ALL_FROZEN; - } - - PageSetAllVisible(page); visibilitymap_set(vacrel->rel, blkno, buffer, InvalidXLogRecPtr, vmbuffer, visibility_cutoff_xid, - flags); + vmflags); /* Count the newly set VM page for logging */ vacrel->vm_new_visible_pages++; @@ -2879,6 +2894,8 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, vacrel->vm_new_visible_frozen_pages++; } + END_CRIT_SECTION(); + /* Revert to the previous phase information for error traceback */ restore_vacuum_error_info(vacrel, &saved_err_info); } @@ -3540,30 +3557,77 @@ dead_items_cleanup(LVRelState *vacrel) } /* - * Check if every tuple in the given page is visible to all current and future - * transactions. Also return the visibility_cutoff_xid which is the highest - * xmin amongst the visible tuples. Set *all_frozen to true if every tuple - * on this page is frozen. - * - * This is a stripped down version of lazy_scan_prune(). If you change - * anything here, make sure that everything stays in sync. Note that an - * assertion calls us to verify that everybody still agrees. Be sure to avoid - * introducing new side-effects here. + * Wrapper for heap_page_would_be_all_visible() which can be used for + * callers that expect no LP_DEAD on the page. */ static bool heap_page_is_all_visible(LVRelState *vacrel, Buffer buf, TransactionId *visibility_cutoff_xid, bool *all_frozen) { + + return heap_page_would_be_all_visible(vacrel, buf, + NULL, 0, + all_frozen, + visibility_cutoff_xid); +} + +/* + * Determines whether or not the heap page in buf is all-visible other than + * the dead line pointers referred to by the provided deadoffsets array. + * + * deadoffsets are the offsets the caller knows about and already removed + * associated index entries. Vacuum will call this before setting those line + * pointers LP_UNUSED. So, if there are no new LP_DEAD items, then the page + * can be set all-visible in the VM by the caller. + * + * Returns true if the page is all-visible other than the provided + * deadoffsets and false otherwise. + * + * vacrel->cutoffs.OldestXmin is used to determine visibility. + * + * *all_frozen is an output parameter indicating to the caller if every tuple + * on the page is frozen. + * + * *visibility_cutoff_xid is an output parameter with the highest xmin amongst the + * visible tuples. It is only valid if the page is all-visible. + * + * Callers looking to verify that the page is already all-visible can call + * heap_page_is_all_visible(). + * + * This is similar logic to that in heap_prune_record_unchanged_lp_normal() If + * you change anything here, make sure that everything stays in sync. Note + * that an assertion calls us to verify that everybody still agrees. Be sure + * to avoid introducing new side-effects here. + */ +static bool +heap_page_would_be_all_visible(LVRelState *vacrel, Buffer buf, + OffsetNumber *deadoffsets, + int ndeadoffsets, + bool *all_frozen, + TransactionId *visibility_cutoff_xid) +{ Page page = BufferGetPage(buf); BlockNumber blockno = BufferGetBlockNumber(buf); OffsetNumber offnum, maxoff; bool all_visible = true; + int matched_dead_count = 0; *visibility_cutoff_xid = InvalidTransactionId; *all_frozen = true; + Assert(ndeadoffsets == 0 || deadoffsets); + +#ifdef USE_ASSERT_CHECKING + /* Confirm input deadoffsets[] is strictly sorted */ + if (ndeadoffsets > 1) + { + for (int i = 1; i < ndeadoffsets; i++) + Assert(deadoffsets[i - 1] < deadoffsets[i]); + } +#endif + maxoff = PageGetMaxOffsetNumber(page); for (offnum = FirstOffsetNumber; offnum <= maxoff && all_visible; @@ -3591,9 +3655,15 @@ heap_page_is_all_visible(LVRelState *vacrel, Buffer buf, */ if (ItemIdIsDead(itemid)) { - all_visible = false; - *all_frozen = false; - break; + if (!deadoffsets || + matched_dead_count >= ndeadoffsets || + deadoffsets[matched_dead_count] != offnum) + { + *all_frozen = all_visible = false; + break; + } + matched_dead_count++; + continue; } Assert(ItemIdIsNormal(itemid)); -- 2.43.0