From 1f636ac362b7a31065eef466965a5aad5862de28 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 9 Jan 2024 17:17:01 -0500 Subject: [PATCH v6 4/5] Inline LVPagePruneState members in lazy_scan_prune() After moving the code to update the visibility map from lazy_scan_heap() into lazy_scan_prune(), the LVPagePruneState is obsolete. Replace all instances of use of its members in lazy_scan_prune() with local variables and remove the struct. --- src/backend/access/heap/vacuumlazy.c | 113 +++++++++++++-------------- src/tools/pgindent/typedefs.list | 1 - 2 files changed, 53 insertions(+), 61 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index bbfd3437fc7..a554a650d62 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -212,23 +212,6 @@ typedef struct LVRelState int64 missed_dead_tuples; /* # removable, but not removed */ } LVRelState; -/* - * State returned by lazy_scan_prune() - */ -typedef struct LVPagePruneState -{ - bool has_lpdead_items; /* includes existing LP_DEAD items */ - - /* - * 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. */ typedef struct LVSavedErrInfo { @@ -250,7 +233,7 @@ static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, static void lazy_scan_prune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, Buffer vmbuffer, bool all_visible_according_to_vm, - LVPagePruneState *prunestate, bool *recordfreespace); + bool *recordfreespace); static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf, BlockNumber blkno, Page page, bool *recordfreespace); @@ -856,7 +839,6 @@ lazy_scan_heap(LVRelState *vacrel) Buffer buf; Page page; bool all_visible_according_to_vm; - LVPagePruneState prunestate; if (blkno == next_unskippable_block) { @@ -1024,7 +1006,7 @@ lazy_scan_heap(LVRelState *vacrel) */ lazy_scan_prune(vacrel, buf, blkno, page, vmbuffer, all_visible_according_to_vm, - &prunestate, &recordfreespace); + &recordfreespace); /* * Final steps for block: drop cleanup lock, record free space in the @@ -1380,7 +1362,6 @@ lazy_scan_prune(LVRelState *vacrel, Page page, Buffer vmbuffer, bool all_visible_according_to_vm, - LVPagePruneState *prunestate, bool *recordfreespace) { Relation rel = vacrel->rel; @@ -1394,6 +1375,9 @@ lazy_scan_prune(LVRelState *vacrel, recently_dead_tuples; HeapPageFreeze pagefrz; bool hastup = false; + bool all_visible, + all_frozen; + TransactionId visibility_cutoff_xid; int64 fpi_before = pgWalUsage.wal_fpi; OffsetNumber deadoffsets[MaxHeapTuplesPerPage]; HeapTupleFreeze frozen[MaxHeapTuplesPerPage]; @@ -1431,14 +1415,22 @@ 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. Keep track of + * the visibility cutoff xid for recovery conflicts. */ - prunestate->has_lpdead_items = false; - prunestate->all_visible = true; - prunestate->all_frozen = true; - prunestate->visibility_cutoff_xid = InvalidTransactionId; + visibility_cutoff_xid = InvalidTransactionId; *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 0 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)) @@ -1525,13 +1517,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; } @@ -1543,14 +1535,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: @@ -1561,7 +1553,7 @@ lazy_scan_prune(LVRelState *vacrel, * pruning.) */ recently_dead_tuples++; - prunestate->all_visible = false; + all_visible = false; break; case HEAPTUPLE_INSERT_IN_PROGRESS: @@ -1572,11 +1564,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 @@ -1606,7 +1598,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; } /* @@ -1624,7 +1616,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)) { /* @@ -1662,11 +1654,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 { @@ -1689,7 +1681,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 */ } @@ -1702,16 +1694,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 @@ -1724,7 +1717,6 @@ lazy_scan_prune(LVRelState *vacrel, ItemPointerData tmp; vacrel->lpdead_item_pages++; - prunestate->has_lpdead_items = true; ItemPointerSetBlockNumber(&tmp, blkno); @@ -1749,7 +1741,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 */ @@ -1772,19 +1764,20 @@ lazy_scan_prune(LVRelState *vacrel, if (hastup) vacrel->nonempty_pages = blkno + 1; - Assert(!prunestate->all_visible || !prunestate->has_lpdead_items); + Assert(!all_visible || lpdead_items == 0); /* * 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; } @@ -1804,7 +1797,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); } @@ -1837,7 +1830,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 (lpdead_items > 0 && PageIsAllVisible(page)) { elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u", vacrel->relname, blkno); @@ -1850,16 +1843,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)) { @@ -1874,7 +1867,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 | diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 5fd46b7bd1f..1edafd0f516 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1405,7 +1405,6 @@ LPVOID LPWSTR LSEG LUID -LVPagePruneState LVRelState LVSavedErrInfo LWLock -- 2.37.2