On Tue, Jan 9, 2024 at 3:40 PM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Tue, Jan 9, 2024 at 3:13 PM Melanie Plageman
> <melanieplage...@gmail.com> wrote:
> > I had already written the patch for immediate reaping addressing the
> > below feedback before I saw the emails that said everyone is happy
> > with using hastup in lazy_scan_[no]prune() in a preliminary patch. Let
> > me know if you have a strong preference for reordering. Otherwise, I
> > will write the three subsequent patches on top of this one.
>
> I don't know if it rises to the level of a strong preference. It's
> just a preference.
Attached v6 has the immediate reaping patch first followed by the code
to use hastup in lazy_scan_[no]prune(). 0003 and 0004 move the VM
update code into lazy_scan_prune() and eliminate LVPagePruneState
entirely. 0005 moves the FSM update into lazy_scan_[no]prune(),
substantially simplifying lazy_scan_heap().
> I agree that we can leave that out. It wouldn't be bad to include it
> if someone had a nice way of doing that, but it doesn't seem critical,
> and if forcing it in there makes the comment less clear overall, it's
> a net loss IMHO.
>
> > Hmm. Yes. I suppose I was trying to find something to validate. Is it
> > worth checking that the line pointer is not already LP_UNUSED? Or is
> > that a bit ridiculous?
>
> I think that's worthwhile (hence my proposed wording).
Done in attached v6.
- Melanie
From 7c499e72cccd3ce9bf45d3824bc13894ea8d4d3a Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Fri, 5 Jan 2024 11:12:58 -0500
Subject: [PATCH v6 3/5] Move VM update code to lazy_scan_prune()
The visibility map is updated after lazy_scan_prune() returns in
lazy_scan_heap(). Most of the output parameters of lazy_scan_prune() are
used to update the VM in lazy_scan_heap(). Moving that code into
lazy_scan_prune() simplifies lazy_scan_heap() and requires less
communication between the two. This is a pure lift and shift. No VM
update logic is changed. All of the members of the prunestate are now
obsolete. It will be removed in a future commit.
---
src/backend/access/heap/vacuumlazy.c | 229 ++++++++++++++-------------
1 file changed, 116 insertions(+), 113 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a5b9319fbcc..bbfd3437fc7 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -249,8 +249,8 @@ static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
bool sharelock, Buffer vmbuffer);
static void lazy_scan_prune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
- LVPagePruneState *prunestate,
- bool *recordfreespace);
+ Buffer vmbuffer, bool all_visible_according_to_vm,
+ LVPagePruneState *prunestate, bool *recordfreespace);
static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
bool *recordfreespace);
@@ -1022,117 +1022,9 @@ lazy_scan_heap(LVRelState *vacrel)
* tuple headers of remaining items with storage. It also determines
* if truncating this block is safe.
*/
- lazy_scan_prune(vacrel, buf, blkno, page, &prunestate, &recordfreespace);
-
- Assert(!prunestate.all_visible || !prunestate.has_lpdead_items);
-
- /*
- * Handle setting visibility map bit based on information from the VM
- * (as of last lazy_scan_skip() call), and from prunestate
- */
- if (!all_visible_according_to_vm && prunestate.all_visible)
- {
- uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
-
- if (prunestate.all_frozen)
- {
- Assert(!TransactionIdIsValid(prunestate.visibility_cutoff_xid));
- flags |= VISIBILITYMAP_ALL_FROZEN;
- }
-
- /*
- * It should never be the case that the visibility map page is set
- * while the page-level bit is clear, but the reverse is allowed
- * (if checksums are not enabled). Regardless, set both bits so
- * that we get back in sync.
- *
- * NB: If the heap page is all-visible but the VM bit is not set,
- * we don't need to dirty the heap page. However, if checksums
- * are enabled, we do need to make sure that the heap page is
- * dirtied before passing it to visibilitymap_set(), because it
- * may be logged. Given that this situation should only happen in
- * rare cases after a crash, it is not worth optimizing.
- */
- PageSetAllVisible(page);
- MarkBufferDirty(buf);
- visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, prunestate.visibility_cutoff_xid,
- flags);
- }
-
- /*
- * As of PostgreSQL 9.2, the visibility map bit should never be set if
- * the page-level bit is clear. However, it's possible that the bit
- * got cleared after lazy_scan_skip() was called, so we must recheck
- * with buffer lock before concluding that the VM is corrupt.
- */
- else if (all_visible_according_to_vm && !PageIsAllVisible(page) &&
- visibilitymap_get_status(vacrel->rel, blkno, &vmbuffer) != 0)
- {
- elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u",
- vacrel->relname, blkno);
- visibilitymap_clear(vacrel->rel, blkno, vmbuffer,
- VISIBILITYMAP_VALID_BITS);
- }
-
- /*
- * It's possible for the value returned by
- * GetOldestNonRemovableTransactionId() to move backwards, so it's not
- * wrong for us to see tuples that appear to not be visible to
- * everyone yet, while PD_ALL_VISIBLE is already set. The real safe
- * xmin value never moves backwards, but
- * GetOldestNonRemovableTransactionId() is conservative and sometimes
- * returns a value that's unnecessarily small, so if we see that
- * contradiction it just means that the tuples that we think are not
- * visible to everyone yet actually are, and the PD_ALL_VISIBLE flag
- * is correct.
- *
- * There should never be LP_DEAD items on a page with PD_ALL_VISIBLE
- * set, however.
- */
- else if (prunestate.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);
- PageClearAllVisible(page);
- MarkBufferDirty(buf);
- visibilitymap_clear(vacrel->rel, blkno, vmbuffer,
- VISIBILITYMAP_VALID_BITS);
- }
-
- /*
- * 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.
- */
- else if (all_visible_according_to_vm && prunestate.all_visible &&
- prunestate.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
- */
- if (!PageIsAllVisible(page))
- {
- PageSetAllVisible(page);
- MarkBufferDirty(buf);
- }
-
- /*
- * Set the page all-frozen (and all-visible) in the VM.
- *
- * We can pass InvalidTransactionId as our visibility_cutoff_xid,
- * 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));
- visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
- vmbuffer, InvalidTransactionId,
- VISIBILITYMAP_ALL_VISIBLE |
- VISIBILITYMAP_ALL_FROZEN);
- }
+ lazy_scan_prune(vacrel, buf, blkno, page,
+ vmbuffer, all_visible_according_to_vm,
+ &prunestate, &recordfreespace);
/*
* Final steps for block: drop cleanup lock, record free space in the
@@ -1486,6 +1378,8 @@ lazy_scan_prune(LVRelState *vacrel,
Buffer buf,
BlockNumber blkno,
Page page,
+ Buffer vmbuffer,
+ bool all_visible_according_to_vm,
LVPagePruneState *prunestate,
bool *recordfreespace)
{
@@ -1877,6 +1771,115 @@ lazy_scan_prune(LVRelState *vacrel,
/* Can't truncate this page */
if (hastup)
vacrel->nonempty_pages = blkno + 1;
+
+ Assert(!prunestate->all_visible || !prunestate->has_lpdead_items);
+
+ /*
+ * Handle setting visibility map bit based on information from the VM (as
+ * of last lazy_scan_skip() call), and from prunestate
+ */
+ if (!all_visible_according_to_vm && prunestate->all_visible)
+ {
+ uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
+
+ if (prunestate->all_frozen)
+ {
+ Assert(!TransactionIdIsValid(prunestate->visibility_cutoff_xid));
+ flags |= VISIBILITYMAP_ALL_FROZEN;
+ }
+
+ /*
+ * It should never be the case that the visibility map page is set
+ * while the page-level bit is clear, but the reverse is allowed (if
+ * checksums are not enabled). Regardless, set both bits so that we
+ * get back in sync.
+ *
+ * NB: If the heap page is all-visible but the VM bit is not set, we
+ * don't need to dirty the heap page. However, if checksums are
+ * enabled, we do need to make sure that the heap page is dirtied
+ * before passing it to visibilitymap_set(), because it may be logged.
+ * Given that this situation should only happen in rare cases after a
+ * crash, it is not worth optimizing.
+ */
+ PageSetAllVisible(page);
+ MarkBufferDirty(buf);
+ visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
+ vmbuffer, prunestate->visibility_cutoff_xid,
+ flags);
+ }
+
+ /*
+ * As of PostgreSQL 9.2, the visibility map bit should never be set if the
+ * page-level bit is clear. However, it's possible that the bit got
+ * cleared after lazy_scan_skip() was called, so we must recheck with
+ * buffer lock before concluding that the VM is corrupt.
+ */
+ else if (all_visible_according_to_vm && !PageIsAllVisible(page) &&
+ visibilitymap_get_status(vacrel->rel, blkno, &vmbuffer) != 0)
+ {
+ elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u",
+ vacrel->relname, blkno);
+ visibilitymap_clear(vacrel->rel, blkno, vmbuffer,
+ VISIBILITYMAP_VALID_BITS);
+ }
+
+ /*
+ * It's possible for the value returned by
+ * GetOldestNonRemovableTransactionId() to move backwards, so it's not
+ * wrong for us to see tuples that appear to not be visible to everyone
+ * yet, while PD_ALL_VISIBLE is already set. The real safe xmin value
+ * never moves backwards, but GetOldestNonRemovableTransactionId() is
+ * conservative and sometimes returns a value that's unnecessarily small,
+ * so if we see that contradiction it just means that the tuples that we
+ * think are not visible to everyone yet actually are, and the
+ * PD_ALL_VISIBLE flag is correct.
+ *
+ * There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
+ * however.
+ */
+ else if (prunestate->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);
+ PageClearAllVisible(page);
+ MarkBufferDirty(buf);
+ visibilitymap_clear(vacrel->rel, blkno, vmbuffer,
+ VISIBILITYMAP_VALID_BITS);
+ }
+
+ /*
+ * 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.
+ */
+ else if (all_visible_according_to_vm && prunestate->all_visible &&
+ prunestate->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
+ */
+ if (!PageIsAllVisible(page))
+ {
+ PageSetAllVisible(page);
+ MarkBufferDirty(buf);
+ }
+
+ /*
+ * Set the page all-frozen (and all-visible) in the VM.
+ *
+ * We can pass InvalidTransactionId as our visibility_cutoff_xid,
+ * 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));
+ visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
+ vmbuffer, InvalidTransactionId,
+ VISIBILITYMAP_ALL_VISIBLE |
+ VISIBILITYMAP_ALL_FROZEN);
+ }
}
/*
--
2.37.2
From b6d9ce973bf60733381260ffaa5d368792cbb419 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Tue, 9 Jan 2024 15:00:24 -0500
Subject: [PATCH v6 1/5] Set would-be dead items LP_UNUSED while pruning
If there are no indexes on a relation, items can be marked LP_UNUSED
instead of LP_DEAD during while pruning. This avoids a separate
invocation of lazy_vacuum_heap_page() and saves a vacuum WAL record.
To accomplish this, pass heap_page_prune() a new parameter, no_indexes,
which indicates that dead line pointers should be set to LP_UNUSED
during pruning, allowing earlier reaping of tuples.
Because we don't update the freespace map until after dropping the lock
on the buffer and we need the lock while we update the visibility map,
save our intent to update the freespace map in output parameter
recordfreespace. This parameter will likely be removed, along with the
LVPagePruneState, by future commits pushing per-page processing from
lazy_scan_heap() into lazy_scan_[no]prune().
Discussion: https://postgr.es/m/CAAKRu_bgvb_k0gKOXWzNKWHt560R0smrGe3E8zewKPs8fiMKkw%40mail.gmail.com
---
src/backend/access/heap/pruneheap.c | 65 ++++++++++--
src/backend/access/heap/vacuumlazy.c | 150 ++++++++++-----------------
src/include/access/heapam.h | 1 +
3 files changed, 111 insertions(+), 105 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 3e0a1a260e6..4ca9d54fe4a 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -35,6 +35,8 @@ typedef struct
/* tuple visibility test, initialized for the relation */
GlobalVisState *vistest;
+ /* whether or not dead items can be set LP_UNUSED during pruning */
+ bool no_indexes;
TransactionId new_prune_xid; /* new prune hint value for page */
TransactionId snapshotConflictHorizon; /* latest xid removed */
@@ -148,7 +150,13 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
{
PruneResult presult;
- heap_page_prune(relation, buffer, vistest, &presult, NULL);
+ /*
+ * For now, pass no_indexes == false regardless of whether or not
+ * the relation has indexes. In the future we may enable immediate
+ * reaping for on access pruning.
+ */
+ heap_page_prune(relation, buffer, vistest, false,
+ &presult, NULL);
/*
* Report the number of tuples reclaimed to pgstats. This is
@@ -193,6 +201,9 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
* (see heap_prune_satisfies_vacuum and
* HeapTupleSatisfiesVacuum).
*
+ * no_indexes indicates whether or not dead items can be set LP_UNUSED during
+ * pruning.
+ *
* off_loc is the offset location required by the caller to use in error
* callback.
*
@@ -203,6 +214,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
void
heap_page_prune(Relation relation, Buffer buffer,
GlobalVisState *vistest,
+ bool no_indexes,
PruneResult *presult,
OffsetNumber *off_loc)
{
@@ -227,6 +239,7 @@ heap_page_prune(Relation relation, Buffer buffer,
prstate.new_prune_xid = InvalidTransactionId;
prstate.rel = relation;
prstate.vistest = vistest;
+ prstate.no_indexes = no_indexes;
prstate.snapshotConflictHorizon = InvalidTransactionId;
prstate.nredirected = prstate.ndead = prstate.nunused = 0;
memset(prstate.marked, 0, sizeof(prstate.marked));
@@ -306,9 +319,9 @@ heap_page_prune(Relation relation, Buffer buffer,
if (off_loc)
*off_loc = offnum;
- /* Nothing to do if slot is empty or already dead */
+ /* Nothing to do if slot is empty */
itemid = PageGetItemId(page, offnum);
- if (!ItemIdIsUsed(itemid) || ItemIdIsDead(itemid))
+ if (!ItemIdIsUsed(itemid))
continue;
/* Process this item or chain of items */
@@ -581,7 +594,17 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* function.)
*/
if (ItemIdIsDead(lp))
+ {
+ /*
+ * If the relation has no indexes, we can set dead line pointers
+ * LP_UNUSED now. We don't increment ndeleted here since the LP
+ * was already marked dead.
+ */
+ if (unlikely(prstate->no_indexes))
+ heap_prune_record_unused(prstate, offnum);
+
break;
+ }
Assert(ItemIdIsNormal(lp));
htup = (HeapTupleHeader) PageGetItem(dp, lp);
@@ -726,7 +749,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* item. This can happen if the loop in heap_page_prune caused us to
* visit the dead successor of a redirect item before visiting the
* redirect item. We can clean up by setting the redirect item to
- * DEAD state.
+ * DEAD state or LP_UNUSED if the table has no indexes.
*/
heap_prune_record_dead(prstate, rootoffnum);
}
@@ -767,6 +790,17 @@ heap_prune_record_redirect(PruneState *prstate,
static void
heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
{
+ /*
+ * If the relation has no indexes, we can remove dead tuples during
+ * pruning instead of marking their line pointers dead. Set this tuple's
+ * line pointer LP_UNUSED. We hint that tables with indexes are more
+ * likely.
+ */
+ if (unlikely(prstate->no_indexes))
+ {
+ heap_prune_record_unused(prstate, offnum);
+ return;
+ }
Assert(prstate->ndead < MaxHeapTuplesPerPage);
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
@@ -903,13 +937,24 @@ heap_page_prune_execute(Buffer buffer,
#ifdef USE_ASSERT_CHECKING
/*
- * Only heap-only tuples can become LP_UNUSED during pruning. They
- * don't need to be left in place as LP_DEAD items until VACUUM gets
- * around to doing index vacuuming.
+ * If there are any items being marked LP_DEAD, then the relation must
+ * have indexes, so every item being marked LP_UNUSED must refer to a
+ * heap-only tuple. But if there are no items being marked LP_DEAD,
+ * then it's possible that the relation has no indexes, in which case
+ * all we know is that the line pointer shouldn't already be
+ * LP_UNUSED.
*/
- Assert(ItemIdHasStorage(lp) && ItemIdIsNormal(lp));
- htup = (HeapTupleHeader) PageGetItem(page, lp);
- Assert(HeapTupleHeaderIsHeapOnly(htup));
+ if (ndead > 0)
+ {
+ Assert(ItemIdHasStorage(lp) && ItemIdIsNormal(lp));
+ htup = (HeapTupleHeader) PageGetItem(page, lp);
+ Assert(HeapTupleHeaderIsHeapOnly(htup));
+ }
+ else
+ {
+ Assert(ItemIdIsUsed(lp));
+ }
+
#endif
ItemIdSetUnused(lp);
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index abbba8947fa..e9636b2c47e 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -250,7 +250,8 @@ static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
bool sharelock, Buffer vmbuffer);
static void lazy_scan_prune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
- LVPagePruneState *prunestate);
+ LVPagePruneState *prunestate,
+ bool *recordfreespace);
static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
bool *hastup, bool *recordfreespace);
@@ -830,8 +831,10 @@ lazy_scan_heap(LVRelState *vacrel)
next_fsm_block_to_vacuum = 0;
VacDeadItems *dead_items = vacrel->dead_items;
Buffer vmbuffer = InvalidBuffer;
+ int tuples_already_deleted;
bool next_unskippable_allvis,
skipping_current_range;
+ bool recordfreespace;
const int initprog_index[] = {
PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_TOTAL_HEAP_BLKS,
@@ -959,8 +962,7 @@ lazy_scan_heap(LVRelState *vacrel)
page = BufferGetPage(buf);
if (!ConditionalLockBufferForCleanup(buf))
{
- bool hastup,
- recordfreespace;
+ bool hastup;
LockBuffer(buf, BUFFER_LOCK_SHARE);
@@ -1010,6 +1012,8 @@ lazy_scan_heap(LVRelState *vacrel)
continue;
}
+ tuples_already_deleted = vacrel->tuples_deleted;
+
/*
* Prune, freeze, and count tuples.
*
@@ -1019,7 +1023,7 @@ lazy_scan_heap(LVRelState *vacrel)
* were pruned some time earlier. Also considers freezing XIDs in the
* tuple headers of remaining items with storage.
*/
- lazy_scan_prune(vacrel, buf, blkno, page, &prunestate);
+ lazy_scan_prune(vacrel, buf, blkno, page, &prunestate, &recordfreespace);
Assert(!prunestate.all_visible || !prunestate.has_lpdead_items);
@@ -1027,69 +1031,6 @@ lazy_scan_heap(LVRelState *vacrel)
if (prunestate.hastup)
vacrel->nonempty_pages = blkno + 1;
- if (vacrel->nindexes == 0)
- {
- /*
- * Consider the need to do page-at-a-time heap vacuuming when
- * using the one-pass strategy now.
- *
- * The one-pass strategy will never call lazy_vacuum(). The steps
- * performed here can be thought of as the one-pass equivalent of
- * a call to lazy_vacuum().
- */
- if (prunestate.has_lpdead_items)
- {
- Size freespace;
-
- lazy_vacuum_heap_page(vacrel, blkno, buf, 0, vmbuffer);
-
- /* Forget the LP_DEAD items that we just vacuumed */
- dead_items->num_items = 0;
-
- /*
- * Now perform FSM processing for blkno, and move on to next
- * page.
- *
- * Our call to lazy_vacuum_heap_page() will have considered if
- * it's possible to set all_visible/all_frozen independently
- * of lazy_scan_prune(). Note that prunestate was invalidated
- * by lazy_vacuum_heap_page() call.
- */
- freespace = PageGetHeapFreeSpace(page);
-
- UnlockReleaseBuffer(buf);
- RecordPageWithFreeSpace(vacrel->rel, blkno, freespace);
-
- /*
- * Periodically perform FSM vacuuming to make newly-freed
- * space visible on upper FSM pages. FreeSpaceMapVacuumRange()
- * vacuums the portion of the freespace map covering heap
- * pages from start to end - 1. Include the block we just
- * vacuumed by passing it blkno + 1. Overflow isn't an issue
- * because MaxBlockNumber + 1 is InvalidBlockNumber which
- * causes FreeSpaceMapVacuumRange() to vacuum freespace map
- * pages covering the remainder of the relation.
- */
- if (blkno - next_fsm_block_to_vacuum >= VACUUM_FSM_EVERY_PAGES)
- {
- FreeSpaceMapVacuumRange(vacrel->rel, next_fsm_block_to_vacuum,
- blkno + 1);
- next_fsm_block_to_vacuum = blkno + 1;
- }
-
- continue;
- }
-
- /*
- * There was no call to lazy_vacuum_heap_page() because pruning
- * didn't encounter/create any LP_DEAD items that needed to be
- * vacuumed. Prune state has not been invalidated, so proceed
- * with prunestate-driven visibility map and FSM steps (just like
- * the two-pass strategy).
- */
- Assert(dead_items->num_items == 0);
- }
-
/*
* Handle setting visibility map bit based on information from the VM
* (as of last lazy_scan_skip() call), and from prunestate
@@ -1200,38 +1141,45 @@ lazy_scan_heap(LVRelState *vacrel)
/*
* Final steps for block: drop cleanup lock, record free space in the
- * FSM
+ * FSM.
+ *
+ * If we will likely do index vacuuming, wait until
+ * lazy_vacuum_heap_rel() to save free space. This doesn't just save
+ * us some cycles; it also allows us to record any additional free
+ * space that lazy_vacuum_heap_page() will make available in cases
+ * where it's possible to truncate the page's line pointer array.
+ *
+ * Note: It's not in fact 100% certain that we really will call
+ * lazy_vacuum_heap_rel() -- lazy_vacuum() might yet opt to skip index
+ * vacuuming (and so must skip heap vacuuming). This is deemed okay
+ * because it only happens in emergencies, or when there is very
+ * little free space anyway. (Besides, we start recording free space
+ * in the FSM once index vacuuming has been abandoned.)
*/
- if (prunestate.has_lpdead_items && vacrel->do_index_vacuuming)
- {
- /*
- * Wait until lazy_vacuum_heap_rel() to save free space. This
- * doesn't just save us some cycles; it also allows us to record
- * any additional free space that lazy_vacuum_heap_page() will
- * make available in cases where it's possible to truncate the
- * page's line pointer array.
- *
- * Note: It's not in fact 100% certain that we really will call
- * lazy_vacuum_heap_rel() -- lazy_vacuum() might yet opt to skip
- * index vacuuming (and so must skip heap vacuuming). This is
- * deemed okay because it only happens in emergencies, or when
- * there is very little free space anyway. (Besides, we start
- * recording free space in the FSM once index vacuuming has been
- * abandoned.)
- *
- * Note: The one-pass (no indexes) case is only supposed to make
- * it this far when there were no LP_DEAD items during pruning.
- */
- Assert(vacrel->nindexes > 0);
- UnlockReleaseBuffer(buf);
- }
- else
+ if (recordfreespace)
{
Size freespace = PageGetHeapFreeSpace(page);
UnlockReleaseBuffer(buf);
RecordPageWithFreeSpace(vacrel->rel, blkno, freespace);
}
+ else
+ UnlockReleaseBuffer(buf);
+
+ /*
+ * Periodically perform FSM vacuuming to make newly-freed space
+ * visible on upper FSM pages. This is done after vacuuming if the
+ * table has indexes.
+ */
+ if (vacrel->nindexes == 0 &&
+ vacrel->tuples_deleted > tuples_already_deleted &&
+ (blkno - next_fsm_block_to_vacuum >= VACUUM_FSM_EVERY_PAGES))
+ {
+ FreeSpaceMapVacuumRange(vacrel->rel, next_fsm_block_to_vacuum,
+ blkno);
+ next_fsm_block_to_vacuum = blkno;
+ }
+
}
vacrel->blkno = InvalidBlockNumber;
@@ -1543,7 +1491,8 @@ lazy_scan_prune(LVRelState *vacrel,
Buffer buf,
BlockNumber blkno,
Page page,
- LVPagePruneState *prunestate)
+ LVPagePruneState *prunestate,
+ bool *recordfreespace)
{
Relation rel = vacrel->rel;
OffsetNumber offnum,
@@ -1587,7 +1536,8 @@ lazy_scan_prune(LVRelState *vacrel,
* lpdead_items's final value can be thought of as the number of tuples
* that were deleted from indexes.
*/
- heap_page_prune(rel, buf, vacrel->vistest, &presult, &vacrel->offnum);
+ heap_page_prune(rel, buf, vacrel->vistest, vacrel->nindexes == 0,
+ &presult, &vacrel->offnum);
/*
* Now scan the page to collect LP_DEAD items and check for tuples
@@ -1598,6 +1548,7 @@ lazy_scan_prune(LVRelState *vacrel,
prunestate->all_visible = true;
prunestate->all_frozen = true;
prunestate->visibility_cutoff_xid = InvalidTransactionId;
+ *recordfreespace = false;
for (offnum = FirstOffsetNumber;
offnum <= maxoff;
@@ -1918,6 +1869,15 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->lpdead_items += lpdead_items;
vacrel->live_tuples += live_tuples;
vacrel->recently_dead_tuples += recently_dead_tuples;
+
+ /*
+ * If we will not do index vacuuming, either because we have no indexes,
+ * because there is nothing to vacuum, or because do_index_vacuuming is
+ * false, make sure we update the freespace map.
+ */
+ if (vacrel->nindexes == 0 ||
+ !vacrel->do_index_vacuuming || lpdead_items == 0)
+ *recordfreespace = true;
}
/*
@@ -2516,7 +2476,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
bool all_frozen;
LVSavedErrInfo saved_err_info;
- Assert(vacrel->nindexes == 0 || vacrel->do_index_vacuuming);
+ Assert(vacrel->do_index_vacuuming);
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_VACUUMED, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 932ec0d6f2b..4a2aebedb57 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -320,6 +320,7 @@ struct GlobalVisState;
extern void heap_page_prune_opt(Relation relation, Buffer buffer);
extern void heap_page_prune(Relation relation, Buffer buffer,
struct GlobalVisState *vistest,
+ bool no_indexes,
PruneResult *presult,
OffsetNumber *off_loc);
extern void heap_page_prune_execute(Buffer buffer,
--
2.37.2
From 5da8d22ccbdbda1f47f41badb432eb66d7b86206 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Mon, 13 Nov 2023 16:47:08 -0500
Subject: [PATCH v6 2/5] Indicate rel truncation unsafe in lazy_scan[no]prune
Both lazy_scan_prune() and lazy_scan_noprune() must determine whether or
not there are tuples on the page making rel truncation unsafe.
LVRelState->nonempty_pages is updated to reflect this.
Previously, both functions set an output parameter or output parameter
member, hastup, to indicate that nonempty_pages should be updated to
reflect the latest non-removable page. There doesn't seem to be any
reason to wait until lazy_scan_[no]prune() returns to update
nonempty_pages. Plenty of other counters in the LVRelState are updated
in lazy_scan_[no]prune(). This allows us to get rid of the output
parameter hastup.
Discussion: https://postgr.es/m/CAAKRu_b_9N2i9fn02P9t_oeLqcEj4%3DE-EK20bZ69nThYWna-Xg%40mail.gmail.com
---
src/backend/access/heap/vacuumlazy.c | 52 +++++++++++++++-------------
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index e9636b2c47e..a5b9319fbcc 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -217,7 +217,6 @@ typedef struct LVRelState
*/
typedef struct LVPagePruneState
{
- bool hastup; /* Page prevents rel truncation? */
bool has_lpdead_items; /* includes existing LP_DEAD items */
/*
@@ -254,7 +253,7 @@ static void lazy_scan_prune(LVRelState *vacrel, Buffer buf,
bool *recordfreespace);
static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
- bool *hastup, bool *recordfreespace);
+ bool *recordfreespace);
static void lazy_vacuum(LVRelState *vacrel);
static bool lazy_vacuum_all_indexes(LVRelState *vacrel);
static void lazy_vacuum_heap_rel(LVRelState *vacrel);
@@ -962,8 +961,6 @@ lazy_scan_heap(LVRelState *vacrel)
page = BufferGetPage(buf);
if (!ConditionalLockBufferForCleanup(buf))
{
- bool hastup;
-
LockBuffer(buf, BUFFER_LOCK_SHARE);
/* Check for new or empty pages before lazy_scan_noprune call */
@@ -974,20 +971,21 @@ lazy_scan_heap(LVRelState *vacrel)
continue;
}
- /* Collect LP_DEAD items in dead_items array, count tuples */
- if (lazy_scan_noprune(vacrel, buf, blkno, page, &hastup,
+ /*
+ * Collect LP_DEAD items in dead_items array, count tuples,
+ * determine if rel truncation is safe
+ */
+ if (lazy_scan_noprune(vacrel, buf, blkno, page,
&recordfreespace))
{
Size freespace = 0;
/*
* Processed page successfully (without cleanup lock) -- just
- * need to perform rel truncation and FSM steps, much like the
- * lazy_scan_prune case. Don't bother trying to match its
- * visibility map setting steps, though.
+ * need to update the FSM, much like the lazy_scan_prune case.
+ * Don't bother trying to match its visibility map setting
+ * steps, though.
*/
- if (hastup)
- vacrel->nonempty_pages = blkno + 1;
if (recordfreespace)
freespace = PageGetHeapFreeSpace(page);
UnlockReleaseBuffer(buf);
@@ -1021,16 +1019,13 @@ lazy_scan_heap(LVRelState *vacrel)
* dead_items array. This includes LP_DEAD line pointers that we
* pruned ourselves, as well as existing LP_DEAD line pointers that
* were pruned some time earlier. Also considers freezing XIDs in the
- * tuple headers of remaining items with storage.
+ * tuple headers of remaining items with storage. It also determines
+ * if truncating this block is safe.
*/
lazy_scan_prune(vacrel, buf, blkno, page, &prunestate, &recordfreespace);
Assert(!prunestate.all_visible || !prunestate.has_lpdead_items);
- /* Remember the location of the last page with nonremovable tuples */
- if (prunestate.hastup)
- vacrel->nonempty_pages = blkno + 1;
-
/*
* Handle setting visibility map bit based on information from the VM
* (as of last lazy_scan_skip() call), and from prunestate
@@ -1504,6 +1499,7 @@ lazy_scan_prune(LVRelState *vacrel,
live_tuples,
recently_dead_tuples;
HeapPageFreeze pagefrz;
+ bool hastup = false;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1543,7 +1539,6 @@ 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
*/
- prunestate->hastup = false;
prunestate->has_lpdead_items = false;
prunestate->all_visible = true;
prunestate->all_frozen = true;
@@ -1571,7 +1566,7 @@ lazy_scan_prune(LVRelState *vacrel,
if (ItemIdIsRedirected(itemid))
{
/* page makes rel truncation unsafe */
- prunestate->hastup = true;
+ hastup = true;
continue;
}
@@ -1701,7 +1696,7 @@ lazy_scan_prune(LVRelState *vacrel,
break;
}
- prunestate->hastup = true; /* page makes rel truncation unsafe */
+ hastup = true; /* page makes rel truncation unsafe */
/* Tuple with storage -- consider need to freeze */
if (heap_prepare_freeze_tuple(htup, &vacrel->cutoffs, &pagefrz,
@@ -1878,6 +1873,10 @@ lazy_scan_prune(LVRelState *vacrel,
if (vacrel->nindexes == 0 ||
!vacrel->do_index_vacuuming || lpdead_items == 0)
*recordfreespace = true;
+
+ /* Can't truncate this page */
+ if (hastup)
+ vacrel->nonempty_pages = blkno + 1;
}
/*
@@ -1895,7 +1894,6 @@ lazy_scan_prune(LVRelState *vacrel,
* one or more tuples on the page. We always return true for non-aggressive
* callers.
*
- * See lazy_scan_prune for an explanation of hastup return flag.
* recordfreespace flag instructs caller on whether or not it should do
* generic FSM processing for page.
*/
@@ -1904,7 +1902,6 @@ lazy_scan_noprune(LVRelState *vacrel,
Buffer buf,
BlockNumber blkno,
Page page,
- bool *hastup,
bool *recordfreespace)
{
OffsetNumber offnum,
@@ -1913,6 +1910,7 @@ lazy_scan_noprune(LVRelState *vacrel,
live_tuples,
recently_dead_tuples,
missed_dead_tuples;
+ bool hastup;
HeapTupleHeader tupleheader;
TransactionId NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
MultiXactId NoFreezePageRelminMxid = vacrel->NewRelminMxid;
@@ -1920,7 +1918,7 @@ lazy_scan_noprune(LVRelState *vacrel,
Assert(BufferGetBlockNumber(buf) == blkno);
- *hastup = false; /* for now */
+ hastup = false; /* for now */
*recordfreespace = false; /* for now */
lpdead_items = 0;
@@ -1944,7 +1942,7 @@ lazy_scan_noprune(LVRelState *vacrel,
if (ItemIdIsRedirected(itemid))
{
- *hastup = true;
+ hastup = true;
continue;
}
@@ -1958,7 +1956,7 @@ lazy_scan_noprune(LVRelState *vacrel,
continue;
}
- *hastup = true; /* page prevents rel truncation */
+ hastup = true; /* page prevents rel truncation */
tupleheader = (HeapTupleHeader) PageGetItem(page, itemid);
if (heap_tuple_should_freeze(tupleheader, &vacrel->cutoffs,
&NoFreezePageRelfrozenXid,
@@ -2060,7 +2058,7 @@ lazy_scan_noprune(LVRelState *vacrel,
* but it beats having to maintain specialized heap vacuuming code
* forever, for vanishingly little benefit.)
*/
- *hastup = true;
+ hastup = true;
missed_dead_tuples += lpdead_items;
}
@@ -2116,6 +2114,10 @@ lazy_scan_noprune(LVRelState *vacrel,
if (missed_dead_tuples > 0)
vacrel->missed_dead_pages++;
+ /* Can't truncate this page */
+ if (hastup)
+ vacrel->nonempty_pages = blkno + 1;
+
/* Caller won't need to call lazy_scan_prune with same page */
return true;
}
--
2.37.2
From eba3ec6ec6ab602b4204f35d8b38f1792c3f9f73 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Tue, 9 Jan 2024 17:18:06 -0500
Subject: [PATCH v6 5/5] Move freespace map update into lazy_scan_[no]prune()
Previously, lazy_scan_heap() updated the freespace map after calling
both lazy_scan_prune() and lazy_scan_noprune() if any space has been
reclaimed. They both had an output parameter recordfreespace to enable
lazy_scan_heap() to do this. Instead, update the freespace map in
lazy_scan_prune() and lazy_scan_noprune(). With this change, all
page-specific processing is done in lazy_scan_prune(),
lazy_scan_noprune(), and lazy_scan_new_or_empty(). This simplifies
lazy_scan_heap().
---
src/backend/access/heap/vacuumlazy.c | 118 +++++++++++++--------------
1 file changed, 58 insertions(+), 60 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a554a650d62..bfcb7d73459 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -232,11 +232,9 @@ static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
bool sharelock, Buffer vmbuffer);
static void lazy_scan_prune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
- Buffer vmbuffer, bool all_visible_according_to_vm,
- bool *recordfreespace);
+ Buffer vmbuffer, bool all_visible_according_to_vm);
static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf,
- BlockNumber blkno, Page page,
- bool *recordfreespace);
+ BlockNumber blkno, Page page);
static void lazy_vacuum(LVRelState *vacrel);
static bool lazy_vacuum_all_indexes(LVRelState *vacrel);
static void lazy_vacuum_heap_rel(LVRelState *vacrel);
@@ -816,7 +814,6 @@ lazy_scan_heap(LVRelState *vacrel)
int tuples_already_deleted;
bool next_unskippable_allvis,
skipping_current_range;
- bool recordfreespace;
const int initprog_index[] = {
PROGRESS_VACUUM_PHASE,
PROGRESS_VACUUM_TOTAL_HEAP_BLKS,
@@ -955,24 +952,14 @@ lazy_scan_heap(LVRelState *vacrel)
/*
* Collect LP_DEAD items in dead_items array, count tuples,
- * determine if rel truncation is safe
+ * determine if rel truncation is safe, and update the FSM.
*/
- if (lazy_scan_noprune(vacrel, buf, blkno, page,
- &recordfreespace))
+ if (lazy_scan_noprune(vacrel, buf, blkno, page))
{
- Size freespace = 0;
-
/*
- * Processed page successfully (without cleanup lock) -- just
- * need to update the FSM, much like the lazy_scan_prune case.
- * Don't bother trying to match its visibility map setting
- * steps, though.
+ * Processed page successfully (without cleanup lock). Buffer
+ * lock and pin released.
*/
- if (recordfreespace)
- freespace = PageGetHeapFreeSpace(page);
- UnlockReleaseBuffer(buf);
- if (recordfreespace)
- RecordPageWithFreeSpace(vacrel->rel, blkno, freespace);
continue;
}
@@ -1002,38 +989,11 @@ lazy_scan_heap(LVRelState *vacrel)
* pruned ourselves, as well as existing LP_DEAD line pointers that
* were pruned some time earlier. Also considers freezing XIDs in the
* tuple headers of remaining items with storage. It also determines
- * if truncating this block is safe.
+ * if truncating this block is safe and updates the FSM (if relevant).
+ * Buffer is returned with lock and pin released.
*/
lazy_scan_prune(vacrel, buf, blkno, page,
- vmbuffer, all_visible_according_to_vm,
- &recordfreespace);
-
- /*
- * Final steps for block: drop cleanup lock, record free space in the
- * FSM.
- *
- * If we will likely do index vacuuming, wait until
- * lazy_vacuum_heap_rel() to save free space. This doesn't just save
- * us some cycles; it also allows us to record any additional free
- * space that lazy_vacuum_heap_page() will make available in cases
- * where it's possible to truncate the page's line pointer array.
- *
- * Note: It's not in fact 100% certain that we really will call
- * lazy_vacuum_heap_rel() -- lazy_vacuum() might yet opt to skip index
- * vacuuming (and so must skip heap vacuuming). This is deemed okay
- * because it only happens in emergencies, or when there is very
- * little free space anyway. (Besides, we start recording free space
- * in the FSM once index vacuuming has been abandoned.)
- */
- if (recordfreespace)
- {
- Size freespace = PageGetHeapFreeSpace(page);
-
- UnlockReleaseBuffer(buf);
- RecordPageWithFreeSpace(vacrel->rel, blkno, freespace);
- }
- else
- UnlockReleaseBuffer(buf);
+ vmbuffer, all_visible_according_to_vm);
/*
* Periodically perform FSM vacuuming to make newly-freed space
@@ -1361,8 +1321,7 @@ lazy_scan_prune(LVRelState *vacrel,
BlockNumber blkno,
Page page,
Buffer vmbuffer,
- bool all_visible_according_to_vm,
- bool *recordfreespace)
+ bool all_visible_according_to_vm)
{
Relation rel = vacrel->rel;
OffsetNumber offnum,
@@ -1377,6 +1336,7 @@ lazy_scan_prune(LVRelState *vacrel,
bool hastup = false;
bool all_visible,
all_frozen;
+ bool recordfreespace;
TransactionId visibility_cutoff_xid;
int64 fpi_before = pgWalUsage.wal_fpi;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
@@ -1419,7 +1379,7 @@ lazy_scan_prune(LVRelState *vacrel,
* the visibility cutoff xid for recovery conflicts.
*/
visibility_cutoff_xid = InvalidTransactionId;
- *recordfreespace = false;
+ recordfreespace = false;
/*
* We will update the VM after collecting LP_DEAD items and freezing
@@ -1758,7 +1718,7 @@ lazy_scan_prune(LVRelState *vacrel,
*/
if (vacrel->nindexes == 0 ||
!vacrel->do_index_vacuuming || lpdead_items == 0)
- *recordfreespace = true;
+ recordfreespace = true;
/* Can't truncate this page */
if (hastup)
@@ -1873,6 +1833,32 @@ lazy_scan_prune(LVRelState *vacrel,
VISIBILITYMAP_ALL_VISIBLE |
VISIBILITYMAP_ALL_FROZEN);
}
+
+ /*
+ * Final steps for block: drop cleanup lock, record free space in the FSM.
+ *
+ * If we will likely do index vacuuming, wait until lazy_vacuum_heap_rel()
+ * to save free space. This doesn't just save us some cycles; it also
+ * allows us to record any additional free space that
+ * lazy_vacuum_heap_page() will make available in cases where it's
+ * possible to truncate the page's line pointer array.
+ *
+ * Note: It's not in fact 100% certain that we really will call
+ * lazy_vacuum_heap_rel() -- lazy_vacuum() might yet opt to skip index
+ * vacuuming (and so must skip heap vacuuming). This is deemed okay
+ * because it only happens in emergencies, or when there is very little
+ * free space anyway. (Besides, we start recording free space in the FSM
+ * once index vacuuming has been abandoned.)
+ */
+ if (recordfreespace)
+ {
+ Size freespace = PageGetHeapFreeSpace(page);
+
+ UnlockReleaseBuffer(buf);
+ RecordPageWithFreeSpace(vacrel->rel, blkno, freespace);
+ }
+ else
+ UnlockReleaseBuffer(buf);
}
/*
@@ -1897,8 +1883,7 @@ static bool
lazy_scan_noprune(LVRelState *vacrel,
Buffer buf,
BlockNumber blkno,
- Page page,
- bool *recordfreespace)
+ Page page)
{
OffsetNumber offnum,
maxoff;
@@ -1907,6 +1892,8 @@ lazy_scan_noprune(LVRelState *vacrel,
recently_dead_tuples,
missed_dead_tuples;
bool hastup;
+ bool recordfreespace;
+ Size freespace = 0;
HeapTupleHeader tupleheader;
TransactionId NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
MultiXactId NoFreezePageRelminMxid = vacrel->NewRelminMxid;
@@ -1915,7 +1902,7 @@ lazy_scan_noprune(LVRelState *vacrel,
Assert(BufferGetBlockNumber(buf) == blkno);
hastup = false; /* for now */
- *recordfreespace = false; /* for now */
+ recordfreespace = false; /* for now */
lpdead_items = 0;
live_tuples = 0;
@@ -2058,7 +2045,7 @@ lazy_scan_noprune(LVRelState *vacrel,
missed_dead_tuples += lpdead_items;
}
- *recordfreespace = true;
+ recordfreespace = true;
}
else if (lpdead_items == 0)
{
@@ -2066,7 +2053,7 @@ lazy_scan_noprune(LVRelState *vacrel,
* Won't be vacuuming this page later, so record page's freespace in
* the FSM now
*/
- *recordfreespace = true;
+ recordfreespace = true;
}
else
{
@@ -2098,7 +2085,7 @@ lazy_scan_noprune(LVRelState *vacrel,
* Assume that we'll go on to vacuum this heap page during final pass
* over the heap. Don't record free space until then.
*/
- *recordfreespace = false;
+ recordfreespace = false;
}
/*
@@ -2114,7 +2101,18 @@ lazy_scan_noprune(LVRelState *vacrel,
if (hastup)
vacrel->nonempty_pages = blkno + 1;
- /* Caller won't need to call lazy_scan_prune with same page */
+ /*
+ * Since we processed page successfully without cleanup lock, caller won't
+ * need to call lazy_scan_prune() with the same page. Now, we just need to
+ * update the FSM, much like the lazy_scan_prune case. Don't bother trying
+ * to match its visibility map setting steps, though.
+ */
+ if (recordfreespace)
+ freespace = PageGetHeapFreeSpace(page);
+ UnlockReleaseBuffer(buf);
+ if (recordfreespace)
+ RecordPageWithFreeSpace(vacrel->rel, blkno, freespace);
+
return true;
}
--
2.37.2
From 1f636ac362b7a31065eef466965a5aad5862de28 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
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
