From 5f8d7544fbe53645b9d46b0eeb74c39cbfdc21f7 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Fri, 5 Jan 2024 11:47:31 -0500 Subject: [PATCH v3 4/6] Inline LVPagePruneState members in lazy_scan_prune() After moving the code to update the visibility map from lazy_scan_heap into lazy_scan_prune, most of the members of LVPagePruneState are no longer required. Make them local variables in lazy_scan_prune(). --- src/backend/access/heap/vacuumlazy.c | 104 ++++++++++++++------------- 1 file changed, 55 insertions(+), 49 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 1bc7c56396f..c0266eabb44 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -217,18 +217,8 @@ typedef struct LVRelState */ typedef struct LVPagePruneState { - bool has_lpdead_items; /* includes existing LP_DEAD items */ /* whether or not caller should do FSM processing for block */ bool recordfreespace; - - /* - * State describes the proper VM bit states to set for the page following - * pruning and freezing. all_visible implies !has_lpdead_items, but don't - * trust all_frozen result unless all_visible is also set to true. - */ - bool all_visible; /* Every item visible to all? */ - bool all_frozen; /* provided all_visible is also true */ - TransactionId visibility_cutoff_xid; /* For recovery conflicts */ } LVPagePruneState; /* Struct for saving and restoring vacuum error information. */ @@ -1397,6 +1387,10 @@ lazy_scan_prune(LVRelState *vacrel, HeapPageFreeze pagefrz; bool no_indexes; bool hastup = false; + bool all_visible, + all_frozen; + TransactionId visibility_cutoff_xid; + bool has_lpdead_items; /* includes existing LP_DEAD items */ int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; HeapTupleFreeze frozen[MaxHeapTuplesPerPage]; @@ -1436,14 +1430,24 @@ lazy_scan_prune(LVRelState *vacrel, /* * Now scan the page to collect LP_DEAD items and check for tuples - * requiring freezing among remaining tuples with storage + * requiring freezing among remaining tuples with storage. + * has_lpdead_items includes existing LP_DEAD items. */ - prunestate->has_lpdead_items = false; - prunestate->all_visible = true; - prunestate->all_frozen = true; - prunestate->visibility_cutoff_xid = InvalidTransactionId; + has_lpdead_items = false; + /* for recovery conflicts */ + visibility_cutoff_xid = InvalidTransactionId; prunestate->recordfreespace = false; + /* + * We will update the VM after collecting LP_DEAD items and freezing + * tuples. Keep track of whether or not the page is all_visible and + * all_frozen and use this information to update the VM. all_visible + * implies !has_lpdead_items, but don't trust all_frozen result unless + * all_visible is also set to true. + */ + all_visible = true; + all_frozen = true; + for (offnum = FirstOffsetNumber; offnum <= maxoff; offnum = OffsetNumberNext(offnum)) @@ -1530,13 +1534,13 @@ lazy_scan_prune(LVRelState *vacrel, * asynchronously. See SetHintBits for more info. Check that * the tuple is hinted xmin-committed because of that. */ - if (prunestate->all_visible) + if (all_visible) { TransactionId xmin; if (!HeapTupleHeaderXminCommitted(htup)) { - prunestate->all_visible = false; + all_visible = false; break; } @@ -1548,14 +1552,14 @@ lazy_scan_prune(LVRelState *vacrel, if (!TransactionIdPrecedes(xmin, vacrel->cutoffs.OldestXmin)) { - prunestate->all_visible = false; + all_visible = false; break; } /* Track newest xmin on page. */ - if (TransactionIdFollows(xmin, prunestate->visibility_cutoff_xid) && + if (TransactionIdFollows(xmin, visibility_cutoff_xid) && TransactionIdIsNormal(xmin)) - prunestate->visibility_cutoff_xid = xmin; + visibility_cutoff_xid = xmin; } break; case HEAPTUPLE_RECENTLY_DEAD: @@ -1566,7 +1570,7 @@ lazy_scan_prune(LVRelState *vacrel, * pruning.) */ recently_dead_tuples++; - prunestate->all_visible = false; + all_visible = false; break; case HEAPTUPLE_INSERT_IN_PROGRESS: @@ -1577,11 +1581,11 @@ lazy_scan_prune(LVRelState *vacrel, * results. This assumption is a bit shaky, but it is what * acquire_sample_rows() does, so be consistent. */ - prunestate->all_visible = false; + all_visible = false; break; case HEAPTUPLE_DELETE_IN_PROGRESS: /* This is an expected case during concurrent vacuum */ - prunestate->all_visible = false; + all_visible = false; /* * Count such rows as live. As above, we assume the deleting @@ -1611,7 +1615,7 @@ lazy_scan_prune(LVRelState *vacrel, * definitely cannot be set all-frozen in the visibility map later on */ if (!totally_frozen) - prunestate->all_frozen = false; + all_frozen = false; } /* @@ -1629,7 +1633,7 @@ lazy_scan_prune(LVRelState *vacrel, * page all-frozen afterwards (might not happen until final heap pass). */ if (pagefrz.freeze_required || tuples_frozen == 0 || - (prunestate->all_visible && prunestate->all_frozen && + (all_visible && all_frozen && fpi_before != pgWalUsage.wal_fpi)) { /* @@ -1667,11 +1671,11 @@ lazy_scan_prune(LVRelState *vacrel, * once we're done with it. Otherwise we generate a conservative * cutoff by stepping back from OldestXmin. */ - if (prunestate->all_visible && prunestate->all_frozen) + if (all_visible && all_frozen) { /* Using same cutoff when setting VM is now unnecessary */ - snapshotConflictHorizon = prunestate->visibility_cutoff_xid; - prunestate->visibility_cutoff_xid = InvalidTransactionId; + snapshotConflictHorizon = visibility_cutoff_xid; + visibility_cutoff_xid = InvalidTransactionId; } else { @@ -1694,7 +1698,7 @@ lazy_scan_prune(LVRelState *vacrel, */ vacrel->NewRelfrozenXid = pagefrz.NoFreezePageRelfrozenXid; vacrel->NewRelminMxid = pagefrz.NoFreezePageRelminMxid; - prunestate->all_frozen = false; + all_frozen = false; tuples_frozen = 0; /* avoid miscounts in instrumentation */ } @@ -1707,16 +1711,17 @@ lazy_scan_prune(LVRelState *vacrel, */ #ifdef USE_ASSERT_CHECKING /* Note that all_frozen value does not matter when !all_visible */ - if (prunestate->all_visible && lpdead_items == 0) + if (all_visible && lpdead_items == 0) { - TransactionId cutoff; - bool all_frozen; + TransactionId debug_cutoff; + bool debug_all_frozen; - if (!heap_page_is_all_visible(vacrel, buf, &cutoff, &all_frozen)) + if (!heap_page_is_all_visible(vacrel, buf, + &debug_cutoff, &debug_all_frozen)) Assert(false); - Assert(!TransactionIdIsValid(cutoff) || - cutoff == prunestate->visibility_cutoff_xid); + Assert(!TransactionIdIsValid(debug_cutoff) || + debug_cutoff == visibility_cutoff_xid); } #endif @@ -1729,7 +1734,7 @@ lazy_scan_prune(LVRelState *vacrel, ItemPointerData tmp; vacrel->lpdead_item_pages++; - prunestate->has_lpdead_items = true; + has_lpdead_items = true; ItemPointerSetBlockNumber(&tmp, blkno); @@ -1754,7 +1759,7 @@ lazy_scan_prune(LVRelState *vacrel, * Now that freezing has been finalized, unset all_visible. It needs * to reflect the present state of things, as expected by our caller. */ - prunestate->all_visible = false; + all_visible = false; } /* Finally, add page-local counts to whole-VACUUM counts */ @@ -1777,19 +1782,20 @@ lazy_scan_prune(LVRelState *vacrel, if (hastup) vacrel->nonempty_pages = blkno + 1; - Assert(!prunestate->all_visible || !prunestate->has_lpdead_items); + Assert(!all_visible || !has_lpdead_items); /* * Handle setting visibility map bit based on information from the VM (as - * of last lazy_scan_skip() call), and from prunestate + * of last lazy_scan_skip() call), and from all_visible and all_frozen + * variables */ - if (!all_visible_according_to_vm && prunestate->all_visible) + if (!all_visible_according_to_vm && all_visible) { uint8 flags = VISIBILITYMAP_ALL_VISIBLE; - if (prunestate->all_frozen) + if (all_frozen) { - Assert(!TransactionIdIsValid(prunestate->visibility_cutoff_xid)); + Assert(!TransactionIdIsValid(visibility_cutoff_xid)); flags |= VISIBILITYMAP_ALL_FROZEN; } @@ -1809,7 +1815,7 @@ lazy_scan_prune(LVRelState *vacrel, PageSetAllVisible(page); MarkBufferDirty(buf); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, - vmbuffer, prunestate->visibility_cutoff_xid, + vmbuffer, visibility_cutoff_xid, flags); } @@ -1842,7 +1848,7 @@ lazy_scan_prune(LVRelState *vacrel, * There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set, * however. */ - else if (prunestate->has_lpdead_items && PageIsAllVisible(page)) + else if (has_lpdead_items && PageIsAllVisible(page)) { elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u", vacrel->relname, blkno); @@ -1855,16 +1861,16 @@ lazy_scan_prune(LVRelState *vacrel, /* * If the all-visible page is all-frozen but not marked as such yet, mark * it as all-frozen. Note that all_frozen is only valid if all_visible is - * true, so we must check both prunestate fields. + * true, so we must check both all_visible and all_frozen. */ - else if (all_visible_according_to_vm && prunestate->all_visible && - prunestate->all_frozen && + else if (all_visible_according_to_vm && all_visible && + all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer)) { /* * Avoid relying on all_visible_according_to_vm as a proxy for the * page-level PD_ALL_VISIBLE bit being set, since it might have become - * stale -- even when all_visible is set in prunestate + * stale -- even when all_visible is set */ if (!PageIsAllVisible(page)) { @@ -1879,7 +1885,7 @@ lazy_scan_prune(LVRelState *vacrel, * since a snapshotConflictHorizon sufficient to make everything safe * for REDO was logged when the page's tuples were frozen. */ - Assert(!TransactionIdIsValid(prunestate->visibility_cutoff_xid)); + Assert(!TransactionIdIsValid(visibility_cutoff_xid)); visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr, vmbuffer, InvalidTransactionId, VISIBILITYMAP_ALL_VISIBLE | -- 2.37.2