From 0219842487931f899abcf183c863c43270c098f0 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sun, 17 Mar 2024 23:05:31 +0200 Subject: [PATCH v3heikki 2/4] Misc cleanup FIXME and some comments I added with HEIKKI: prefix with questions --- src/backend/access/heap/pruneheap.c | 16 +++++++--------- src/backend/access/heap/vacuumlazy.c | 7 ++++++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index f4f5468e144..b3573bb628b 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -32,10 +32,9 @@ /* Working data for heap_page_prune_and_freeze() and subroutines */ typedef struct { - Relation rel; - /* tuple visibility test, initialized for the relation */ GlobalVisState *vistest; + /* whether or not dead items can be set LP_UNUSED during pruning */ bool mark_unused_now; @@ -248,12 +247,12 @@ prune_freeze_xmin_is_removable(GlobalVisState *visstate, TransactionId xmin) * the current relfrozenxid and relminmxids used if the caller is interested in * freezing tuples on the page. * - * off_loc is the offset location required by the caller to use in error - * callback. - * * presult contains output parameters needed by callers such as the number of * tuples removed and the number of line pointers newly marked LP_DEAD. * heap_page_prune_and_freeze() is responsible for initializing it. + * + * off_loc is the offset location required by the caller to use in error + * callback. */ void heap_page_prune_and_freeze(Relation relation, Buffer buffer, @@ -294,7 +293,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, * initialize the rest of our working state. */ prstate.new_prune_xid = InvalidTransactionId; - prstate.rel = relation; prstate.vistest = vistest; prstate.mark_unused_now = mark_unused_now; prstate.snapshotConflictHorizon = InvalidTransactionId; @@ -302,9 +300,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, memset(prstate.marked, 0, sizeof(prstate.marked)); /* - * presult->htsv is not initialized here because all ntuple spots in the + * prstate.htsv is not initialized here because all ntuple spots in the * array will be set either to a valid HTSV_Result value or -1. */ + presult->ndeleted = 0; presult->nnewlpdead = 0; presult->nfrozen = 0; @@ -328,7 +327,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, presult->new_relminmxid = InvalidMultiXactId; maxoff = PageGetMaxOffsetNumber(page); - tup.t_tableOid = RelationGetRelid(prstate.rel); + tup.t_tableOid = RelationGetRelid(relation); /* * Determine HTSV for all tuples. @@ -378,7 +377,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, prstate.htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup, buffer); - Assert(ItemIdIsNormal(itemid)); /* * The criteria for counting a tuple as live in this block need to diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 8d3723faf3a..d3df7029667 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -433,7 +433,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, * heap_page_prune_and_freeze(). We expect vistest will always make * heap_page_prune_and_freeze() remove any deleted tuple whose xmax is < * OldestXmin. lazy_scan_prune must never become confused about whether a - * tuple should be frozen or removed. (In the future we might want to + * tuple should be frozen or removed. + * HEIKKI: Is such confusion possible anymore? + * (In the future we might want to * teach lazy_scan_prune to recompute vistest from time to time, to * increase the number of dead tuples it can prune away.) */ @@ -1387,6 +1389,9 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, * so we could deal with tuples that were DEAD to VACUUM, but nevertheless were * left with storage after pruning. * + * HEIKKI: does the above paragraph still make sense? We don't call + * HeapTupleSatisfiesVacuum() here anymore + * * As of Postgres 17, we circumvent this problem altogether by reusing the * result of heap_page_prune_and_freeze()'s visibility check. Without the * second call to HeapTupleSatisfiesVacuum(), there is no new HTSV_Result and -- 2.39.2