From e0d1a4724fcef6826bdd86c7fc2d068641624b5f Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 17 Dec 2025 16:51:05 -0500 Subject: [PATCH v42 03/12] Use GlobalVisState in vacuum to determine page level visibility During vacuum's first and third phases, we examine tuples' visibility to determine if we can set the page all-visible in the visibility map. Previously, this check compared tuple xmins against a single XID chosen at the start of vacuum (OldestXmin). We now use GlobalVisState, which also enables future work to set the VM during on-access pruning, since ordinary queries have access to GlobalVisState but not OldestXmin. This also benefits vacuum: in some cases, GlobalVisState may advance during a vacuum, allowing more pages to become considered all-visible. And, in the future, we could easily add a heuristic to update GlobalVisState more frequently during vacuums of large tables. OldestXmin is still used for freezing and as a backstop to ensure we don't freeze a dead tuple that wasn't yet prunable according to GlobalVisState in the rare occurrences where GlobalVisState moves backwards. Because comparing a transaction ID against GlobalVisState is more expensive than comparing against a single XID, we defer this check until after scanning all tuples on the page. Therefore, we perform the GlobalVisState check only once per page. This is safe because visibility_cutoff_xid records the newest live xmin on the page; if it is globally visible, then the entire page is all-visible. Using GlobalVisState means on-access pruning can also maintain visibility_cutoff_xid, which is required to set the visibility map on-access in the future. Author: Melanie Plageman Reviewed-by: Andres Freund Reviewed-by: Chao Li Reviewed-by: Kirill Reshke Discussion: https://postgr.es/m/flat/bqc4kh5midfn44gnjiqez3bjqv4zogydguvdn446riw45jcf3y%404ez66il7ebvk#c755ef151507aba58471ffaca607e493 --- src/backend/access/heap/heapam_visibility.c | 30 ++++++++++- src/backend/access/heap/pruneheap.c | 53 +++++++++--------- src/backend/access/heap/vacuumlazy.c | 60 ++++++++++++++------- src/backend/access/spgist/spgvacuum.c | 2 +- src/backend/storage/ipc/procarray.c | 19 ++++--- src/include/access/heapam.h | 2 + src/include/utils/snapmgr.h | 4 +- 7 files changed, 115 insertions(+), 55 deletions(-) diff --git a/src/backend/access/heap/heapam_visibility.c b/src/backend/access/heap/heapam_visibility.c index fc64f4343ce..c678f5a3c8f 100644 --- a/src/backend/access/heap/heapam_visibility.c +++ b/src/backend/access/heap/heapam_visibility.c @@ -1131,6 +1131,31 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, return res; } +/* + * Wrapper around GlobalVisTestIsRemovableXid() for use when examining live + * tuples. Returns true if the given XID may be considered running by at least + * one snapshot. + * + * This function alone is insufficient to determine tuple visibility; callers + * must also consider the XID's commit status. Its purpose is purely semantic: + * when applied to live tuples, GlobalVisTestIsRemovableXid() is checking + * whether the inserting transaction is still considered running, not whether + * the tuple is removable. Live tuples are, by definition, not removable, but + * the snapshot criteria for “transaction still running” are identical to + * those used for removal XIDs. + * + * If allow_update is false, the GlobalVisState boundaries will not be updated. + * + * See the comment above GlobalVisTestIsRemovable[Full]Xid() for details on the + * required preconditions for calling this function. + */ +bool +GlobalVisTestXidConsideredRunning(GlobalVisState *state, TransactionId xid, + bool allow_update) +{ + return !GlobalVisTestIsRemovableXid(state, xid, allow_update); +} + /* * Work horse for HeapTupleSatisfiesVacuum and similar routines. * @@ -1354,7 +1379,7 @@ HeapTupleSatisfiesNonVacuumable(HeapTuple htup, Snapshot snapshot, { Assert(TransactionIdIsValid(dead_after)); - if (GlobalVisTestIsRemovableXid(snapshot->vistest, dead_after)) + if (GlobalVisTestIsRemovableXid(snapshot->vistest, dead_after, true)) res = HEAPTUPLE_DEAD; } else @@ -1420,7 +1445,8 @@ HeapTupleIsSurelyDead(HeapTuple htup, GlobalVisState *vistest) /* Deleter committed, so tuple is dead if the XID is old enough. */ return GlobalVisTestIsRemovableXid(vistest, - HeapTupleHeaderGetRawXmax(tuple)); + HeapTupleHeaderGetRawXmax(tuple), + true); } /* diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 22f2d9d9798..f9db97a6edf 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -166,10 +166,13 @@ typedef struct * all-frozen bits in the visibility map can be set for this page after * pruning. * - * visibility_cutoff_xid is the newest xmin of live tuples on the page. - * The caller can use it as the conflict horizon, when setting the VM - * bits. It is only valid if we froze some tuples, and set_all_frozen is - * true. + * visibility_cutoff_xid is the newest xmin of live tuples on the page. It + * is used after processing all tuples to determine if the page can be + * considered all-visible (if the newest xmin is still considered running + * by some snapshot, it cannot be). It is also used by the caller as the + * conflict horizon when setting the VM bits, unless we froze all tuples + * on the page (in which case the conflict xid was already included in the + * WAL record). * * NOTE: set_all_visible and set_all_frozen initially don't include * LP_DEAD items. That's convenient for heap_page_prune_and_freeze() to @@ -285,7 +288,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer, Buffer *vmbuffer) */ vistest = GlobalVisTestFor(relation); - if (!GlobalVisTestIsRemovableXid(vistest, prune_xid)) + if (!GlobalVisTestIsRemovableXid(vistest, prune_xid, true)) return; /* @@ -1087,6 +1090,18 @@ heap_page_prune_and_freeze(PruneFreezeParams *params, */ prune_freeze_plan(&prstate, off_loc); + /* + * After processing all the live tuples on the page, if the newest xmin + * amongst them may be considered running by any snapshot, the page cannot + * be all-visible. + */ + if (prstate.set_all_visible && + TransactionIdIsNormal(prstate.visibility_cutoff_xid) && + GlobalVisTestXidConsideredRunning(prstate.vistest, + prstate.visibility_cutoff_xid, + true)) + prstate.set_all_visible = prstate.set_all_frozen = false; + /* * If checksums are enabled, calling heap_prune_satisfies_vacuum() while * checking tuple visibility information in prune_freeze_plan() may have @@ -1289,7 +1304,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup) * if the GlobalVisState has been updated since the beginning of vacuuming * the relation. */ - if (GlobalVisTestIsRemovableXid(prstate->vistest, dead_after)) + if (GlobalVisTestIsRemovableXid(prstate->vistest, dead_after, true)) return HEAPTUPLE_DEAD; return res; @@ -1755,29 +1770,15 @@ heap_prune_record_unchanged_lp_normal(PruneState *prstate, OffsetNumber offnum) } /* - * The inserter definitely committed. But is it old enough - * that everyone sees it as committed? A FrozenTransactionId - * is seen as committed to everyone. Otherwise, we check if - * there is a snapshot that considers this xid to still be - * running, and if so, we don't consider the page all-visible. + * The inserter definitely committed. But we don't know if it + * is old enough that everyone sees it as committed. Later, + * after processing all the tuples on the page, we'll check if + * there is any snapshot that still considers the newest xid + * on the page to be running. If so, we don't consider the + * page all-visible. */ xmin = HeapTupleHeaderGetXmin(htup); - /* - * For now always use prstate->cutoffs for this test, because - * we only update 'set_all_visible' and 'set_all_frozen' when - * freezing is requested. We could use - * GlobalVisTestIsRemovableXid instead, if a non-freezing - * caller wanted to set the VM bit. - */ - Assert(prstate->cutoffs); - if (!TransactionIdPrecedes(xmin, prstate->cutoffs->OldestXmin)) - { - prstate->set_all_visible = false; - prstate->set_all_frozen = false; - break; - } - /* Track newest xmin on page. */ if (TransactionIdFollows(xmin, prstate->visibility_cutoff_xid) && TransactionIdIsNormal(xmin)) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 1a446050d85..797973d7bd0 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -468,13 +468,14 @@ static void dead_items_cleanup(LVRelState *vacrel); #ifdef USE_ASSERT_CHECKING static bool heap_page_is_all_visible(Relation rel, Buffer buf, - TransactionId OldestXmin, + GlobalVisState *vistest, bool *all_frozen, TransactionId *visibility_cutoff_xid, OffsetNumber *logging_offnum); #endif static bool heap_page_would_be_all_visible(Relation rel, Buffer buf, - TransactionId OldestXmin, + GlobalVisState *vistest, + bool allow_update_vistest, OffsetNumber *deadoffsets, int ndeadoffsets, bool *all_frozen, @@ -2089,7 +2090,7 @@ lazy_scan_prune(LVRelState *vacrel, Assert(presult.lpdead_items == 0); Assert(heap_page_is_all_visible(vacrel->rel, buf, - vacrel->cutoffs.OldestXmin, &debug_all_frozen, + vacrel->vistest, &debug_all_frozen, &debug_cutoff, &vacrel->offnum)); Assert(presult.set_all_frozen == debug_all_frozen); @@ -2852,7 +2853,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, * done outside the critical section. */ if (heap_page_would_be_all_visible(vacrel->rel, buffer, - vacrel->cutoffs.OldestXmin, + vacrel->vistest, true, deadoffsets, num_offsets, &all_frozen, &visibility_cutoff_xid, &vacrel->offnum)) @@ -3614,14 +3615,19 @@ dead_items_cleanup(LVRelState *vacrel) */ static bool heap_page_is_all_visible(Relation rel, Buffer buf, - TransactionId OldestXmin, + GlobalVisState *vistest, bool *all_frozen, TransactionId *visibility_cutoff_xid, OffsetNumber *logging_offnum) { - + /* + * Pass allow_update_vistest as false so that the GlobalVisState + * boundaries used here match those used by the pruning code we are + * cross-checking. Allowing an update could move the boundaries between + * the two calls, causing a spurious assertion failure. + */ return heap_page_would_be_all_visible(rel, buf, - OldestXmin, + vistest, false, NULL, 0, all_frozen, visibility_cutoff_xid, @@ -3642,7 +3648,9 @@ heap_page_is_all_visible(Relation rel, Buffer buf, * Returns true if the page is all-visible other than the provided * deadoffsets and false otherwise. * - * OldestXmin is used to determine visibility. + * vistest is used to determine visibility. If allow_update_vistest is true, + * the boundaries of the GlobalVisState may be updated when checking the + * visibility of the newest live XID on the page. * * Output parameters: * @@ -3661,7 +3669,8 @@ heap_page_is_all_visible(Relation rel, Buffer buf, */ static bool heap_page_would_be_all_visible(Relation rel, Buffer buf, - TransactionId OldestXmin, + GlobalVisState *vistest, + bool allow_update_vistest, OffsetNumber *deadoffsets, int ndeadoffsets, bool *all_frozen, @@ -3742,7 +3751,7 @@ heap_page_would_be_all_visible(Relation rel, Buffer buf, { TransactionId xmin; - /* Check comments in lazy_scan_prune. */ + /* Check heap_prune_record_unchanged_lp_normal comments */ if (!HeapTupleHeaderXminCommitted(tuple.t_data)) { all_visible = false; @@ -3751,16 +3760,17 @@ heap_page_would_be_all_visible(Relation rel, Buffer buf, } /* - * The inserter definitely committed. But is it old enough - * that everyone sees it as committed? + * The inserter definitely committed. But we don't know if + * it is old enough that everyone sees it as committed. + * Don't check that now. + * + * If we scan all tuples without finding one that prevents + * the page from being all-visible, we then check whether + * any snapshot still considers the newest XID on the page + * to be running. In that case, the page is not considered + * all-visible. */ xmin = HeapTupleHeaderGetXmin(tuple.t_data); - if (!TransactionIdPrecedes(xmin, OldestXmin)) - { - all_visible = false; - *all_frozen = false; - break; - } /* Track newest xmin on page. */ if (TransactionIdFollows(xmin, *visibility_cutoff_xid) && @@ -3789,6 +3799,20 @@ heap_page_would_be_all_visible(Relation rel, Buffer buf, } } /* scan along page */ + /* + * After processing all the live tuples on the page, if the newest xmin + * among them may still be considered running by any snapshot, the page + * cannot be all-visible. + */ + if (all_visible && + TransactionIdIsNormal(*visibility_cutoff_xid) && + GlobalVisTestXidConsideredRunning(vistest, *visibility_cutoff_xid, + allow_update_vistest)) + { + all_visible = false; + *all_frozen = false; + } + /* Clear the offset information once we have processed the given page. */ *logging_offnum = InvalidOffsetNumber; diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c index 6b7117b56b2..c461f8dc02d 100644 --- a/src/backend/access/spgist/spgvacuum.c +++ b/src/backend/access/spgist/spgvacuum.c @@ -536,7 +536,7 @@ vacuumRedirectAndPlaceholder(Relation index, Relation heaprel, Buffer buffer) */ if (dt->tupstate == SPGIST_REDIRECT && (!TransactionIdIsValid(dt->xid) || - GlobalVisTestIsRemovableXid(vistest, dt->xid))) + GlobalVisTestIsRemovableXid(vistest, dt->xid, true))) { dt->tupstate = SPGIST_PLACEHOLDER; Assert(opaque->nRedirection > 0); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 0f913897acc..27e5adeebfb 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -4223,11 +4223,17 @@ GlobalVisUpdate(void) * The state passed needs to have been initialized for the relation fxid is * from (NULL is also OK), otherwise the result may not be correct. * + * If allow_update is false, the GlobalVisState boundaries will not be updated + * even if it would otherwise be beneficial. This is useful for callers that + * do not want GlobalVisState to advance at all, for example because they need + * a conservative answer based on the current boundaries. + * * See comment for GlobalVisState for details. */ bool GlobalVisTestIsRemovableFullXid(GlobalVisState *state, - FullTransactionId fxid) + FullTransactionId fxid, + bool allow_update) { /* * If fxid is older than maybe_needed bound, it definitely is visible to @@ -4248,7 +4254,7 @@ GlobalVisTestIsRemovableFullXid(GlobalVisState *state, * might not exist a snapshot considering fxid running. If it makes sense, * update boundaries and recheck. */ - if (GlobalVisTestShouldUpdate(state)) + if (allow_update && GlobalVisTestShouldUpdate(state)) { GlobalVisUpdate(); @@ -4268,7 +4274,8 @@ GlobalVisTestIsRemovableFullXid(GlobalVisState *state, * relfrozenxid). */ bool -GlobalVisTestIsRemovableXid(GlobalVisState *state, TransactionId xid) +GlobalVisTestIsRemovableXid(GlobalVisState *state, TransactionId xid, + bool allow_update) { FullTransactionId fxid; @@ -4282,7 +4289,7 @@ GlobalVisTestIsRemovableXid(GlobalVisState *state, TransactionId xid) */ fxid = FullXidRelativeTo(state->definitely_needed, xid); - return GlobalVisTestIsRemovableFullXid(state, fxid); + return GlobalVisTestIsRemovableFullXid(state, fxid, allow_update); } /* @@ -4296,7 +4303,7 @@ GlobalVisCheckRemovableFullXid(Relation rel, FullTransactionId fxid) state = GlobalVisTestFor(rel); - return GlobalVisTestIsRemovableFullXid(state, fxid); + return GlobalVisTestIsRemovableFullXid(state, fxid, true); } /* @@ -4310,7 +4317,7 @@ GlobalVisCheckRemovableXid(Relation rel, TransactionId xid) state = GlobalVisTestFor(rel); - return GlobalVisTestIsRemovableXid(state, xid); + return GlobalVisTestIsRemovableXid(state, xid, true); } /* diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 305ecc31a9e..8815acccafb 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -480,6 +480,8 @@ extern TM_Result HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid, Buffer buffer); extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin, Buffer buffer); + +extern bool GlobalVisTestXidConsideredRunning(GlobalVisState *state, TransactionId xid, bool allow_update); extern HTSV_Result HeapTupleSatisfiesVacuumHorizon(HeapTuple htup, Buffer buffer, TransactionId *dead_after); extern void HeapTupleSetHintBits(HeapTupleHeader tuple, Buffer buffer, diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 8c919d2640e..db903709c49 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -115,8 +115,8 @@ extern char *ExportSnapshot(Snapshot snapshot); */ typedef struct GlobalVisState GlobalVisState; extern GlobalVisState *GlobalVisTestFor(Relation rel); -extern bool GlobalVisTestIsRemovableXid(GlobalVisState *state, TransactionId xid); -extern bool GlobalVisTestIsRemovableFullXid(GlobalVisState *state, FullTransactionId fxid); +extern bool GlobalVisTestIsRemovableXid(GlobalVisState *state, TransactionId xid, bool allow_update); +extern bool GlobalVisTestIsRemovableFullXid(GlobalVisState *state, FullTransactionId fxid, bool allow_update); extern bool GlobalVisCheckRemovableXid(Relation rel, TransactionId xid); extern bool GlobalVisCheckRemovableFullXid(Relation rel, FullTransactionId fxid); -- 2.43.0