Attached v7 is rebased over the commit you just made to remove
LVPagePruneState->hastup.
On Thu, Jan 11, 2024 at 11:54 AM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Wed, Jan 10, 2024 at 5:28 PM Melanie Plageman
> <melanieplage...@gmail.com> wrote:
> > Yes, the options I can think of are:
> >
> > 1) rename the parameter to "immed_reap" or similar and make very clear
> > in heap_page_prune_opt() that you are not to pass true.
> > 2) make immediate reaping work for on-access pruning. I would need a
> > low cost way to find out if there are any indexes on the table. Do you
> > think this is possible? Should we just rename the parameter for now
> > and think about that later?
>
> I don't think we can implement (2), because:
>
> robert.haas=# create table test (a int);
> CREATE TABLE
> robert.haas=# begin;
> BEGIN
> robert.haas=*# select * from test;
> a
> ---
> (0 rows)
>
> <in another window>
>
> robert.haas=# create index on test (a);
> CREATE INDEX
>
> In English, the lock we hold during regular table access isn't strong
> enough to foreclose concurrent addition of an index.
Ah, I see.
> So renaming the parameter seems like the way to go. How about
> "mark_unused_now"?
I've done this in attached v7.
> > > - if (!ItemIdIsUsed(itemid) || ItemIdIsDead(itemid))
> > > + if (!ItemIdIsUsed(itemid))
> > > continue;
> > >
> > > There is a bit of overhead here for the !no_indexes case. I assume it
> > > doesn't matter.
> >
> > Right. Should be okay. Alternatively, and I'm not saying this is a
> > good idea, but we could throw this into the loop in heap_page_prune()
> > which calls heap_prune_chain():
> >
> > + if (ItemIdIsDead(itemid) && prstate.no_indexes)
> > + {
> > + heap_prune_record_unused(&prstate, offnum);
> > + continue;
> > + }
> >
> > I think that is correct?
>
> Wouldn't that be wrong in any case where heap_prune_chain() calls
> heap_prune_record_dead()?
Hmm. But previously, we skipped heap_prune_chain() for already dead
line pointers. In my patch, I rely on the test in the loop in
heap_prune_chain() to set those LP_UNUSED if mark_unused_now is true
(previously the below code just broke out of the loop).
/*
* Likewise, a dead line pointer can't be part of the chain. (We
* already eliminated the case of dead root tuple outside this
* function.)
*/
if (ItemIdIsDead(lp))
{
/*
* If the caller set mark_unused_now true, 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->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
break;
}
so wouldn't what I suggested simply set the item LP_UNSED that before
invoking heap_prune_chain()? Thereby achieving the same result without
invoking heap_prune_chain() for already dead line pointers? I could be
missing something. That heap_prune_chain() logic sometimes gets me
turned around.
> > Yes, so I preferred it in the body of heap_prune_chain() (the caller).
> > Andres didn't like the extra level of indentation. I could wrap
> > heap_record_dead() and heap_record_unused(), but I couldn't really
> > think of a good wrapper name. heap_record_dead_or_unused() seems long
> > and literal. But, it might be the best I can do. I can't think of a
> > general word which encompasses both the transition to death and to
> > disposal.
>
> I'm sure we could solve the wordsmithing problem but I think it's
> clearer if the heap_prune_record_* functions don't get clever.
I've gone with my heap_record_dead_or_unused() suggestion.
- Melanie
From dedbd202dfd800c3ac8b06b58c5d16a036b4a3e2 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Fri, 5 Jan 2024 11:12:58 -0500
Subject: [PATCH v7 2/4] 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 85c0919dc71..82495ef8082 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)
{
@@ -1881,6 +1775,115 @@ lazy_scan_prune(LVRelState *vacrel,
if (vacrel->nindexes == 0 ||
!vacrel->do_index_vacuuming || lpdead_items == 0)
*recordfreespace = true;
+
+ 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 34081d04bf63af011ec94ed3203a45a916f96b06 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Tue, 9 Jan 2024 17:17:01 -0500
Subject: [PATCH v7 3/4] 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 82495ef8082..12c2fc78a43 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];
@@ -1435,14 +1419,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))
@@ -1529,13 +1521,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;
}
@@ -1547,14 +1539,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:
@@ -1565,7 +1557,7 @@ lazy_scan_prune(LVRelState *vacrel,
* pruning.)
*/
recently_dead_tuples++;
- prunestate->all_visible = false;
+ all_visible = false;
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
@@ -1576,11 +1568,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
@@ -1610,7 +1602,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;
}
/*
@@ -1628,7 +1620,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))
{
/*
@@ -1666,11 +1658,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
{
@@ -1693,7 +1685,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 */
}
@@ -1706,16 +1698,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
@@ -1728,7 +1721,6 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerData tmp;
vacrel->lpdead_item_pages++;
- prunestate->has_lpdead_items = true;
ItemPointerSetBlockNumber(&tmp, blkno);
@@ -1753,7 +1745,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 */
@@ -1776,19 +1768,20 @@ lazy_scan_prune(LVRelState *vacrel,
!vacrel->do_index_vacuuming || lpdead_items == 0)
*recordfreespace = true;
- 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;
}
@@ -1808,7 +1801,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);
}
@@ -1841,7 +1834,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);
@@ -1854,16 +1847,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))
{
@@ -1878,7 +1871,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 f582eb59e7d..e0b0ed53d66 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
From 9edfd48edbbc4022370bc232fd680fd548a9dd67 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Tue, 9 Jan 2024 15:00:24 -0500
Subject: [PATCH v7 1/4] 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, mark_unused_now,
which indicates that dead line pointers should be set to LP_UNUSED
during pruning, allowing earlier reaping of tuples.
We only enable this for vacuum's pruning. On access pruning always
passes mark_unused_now == false.
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 | 79 +++++++++++---
src/backend/access/heap/vacuumlazy.c | 153 ++++++++++-----------------
src/include/access/heapam.h | 1 +
3 files changed, 126 insertions(+), 107 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 3e0a1a260e6..be83f7bdf05 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 mark_unused_now;
TransactionId new_prune_xid; /* new prune hint value for page */
TransactionId snapshotConflictHorizon; /* latest xid removed */
@@ -67,6 +69,7 @@ static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
static void heap_prune_record_redirect(PruneState *prstate,
OffsetNumber offnum, OffsetNumber rdoffnum);
static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
@@ -148,7 +151,13 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
{
PruneResult presult;
- heap_page_prune(relation, buffer, vistest, &presult, NULL);
+ /*
+ * For now, pass mark_unused_now == false regardless of whether or
+ * not the relation has indexes, since we cannot safely determine
+ * that during on-access pruning with the current implementation.
+ */
+ heap_page_prune(relation, buffer, vistest, false,
+ &presult, NULL);
/*
* Report the number of tuples reclaimed to pgstats. This is
@@ -193,6 +202,9 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
* (see heap_prune_satisfies_vacuum and
* HeapTupleSatisfiesVacuum).
*
+ * mark_unused_now 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 +215,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
void
heap_page_prune(Relation relation, Buffer buffer,
GlobalVisState *vistest,
+ bool mark_unused_now,
PruneResult *presult,
OffsetNumber *off_loc)
{
@@ -227,6 +240,7 @@ heap_page_prune(Relation relation, Buffer buffer,
prstate.new_prune_xid = InvalidTransactionId;
prstate.rel = relation;
prstate.vistest = vistest;
+ prstate.mark_unused_now = mark_unused_now;
prstate.snapshotConflictHorizon = InvalidTransactionId;
prstate.nredirected = prstate.ndead = prstate.nunused = 0;
memset(prstate.marked, 0, sizeof(prstate.marked));
@@ -306,9 +320,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 +595,17 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* function.)
*/
if (ItemIdIsDead(lp))
+ {
+ /*
+ * If the caller set mark_unused_now true, 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->mark_unused_now))
+ heap_prune_record_unused(prstate, offnum);
+
break;
+ }
Assert(ItemIdIsNormal(lp));
htup = (HeapTupleHeader) PageGetItem(dp, lp);
@@ -715,7 +739,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
* redirect the root to the correct chain member.
*/
if (i >= nchain)
- heap_prune_record_dead(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum);
else
heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
}
@@ -726,9 +750,9 @@ 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 caller indicated.
*/
- heap_prune_record_dead(prstate, rootoffnum);
+ heap_prune_record_dead_or_unused(prstate, rootoffnum);
}
return ndeleted;
@@ -774,6 +798,27 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
prstate->marked[offnum] = true;
}
+/*
+ * Depending on whether or not the caller set mark_unused_now to true, record that a
+ * line pointer should be marked LP_DEAD or LP_UNUSED. There are other cases in
+ * which we will mark line pointers LP_UNUSED, but we will not mark line
+ * pointers LP_DEAD if mark_unused_now is true.
+ */
+static void
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+{
+ /*
+ * If the caller set mark_unused_now to true, 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 this option is less
+ * likely.
+ */
+ if (unlikely(prstate->mark_unused_now))
+ heap_prune_record_unused(prstate, offnum);
+ else
+ heap_prune_record_dead(prstate, offnum);
+}
+
/* Record line pointer to be marked unused */
static void
heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum)
@@ -903,13 +948,23 @@ 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.
+ * During pruning, the caller may have passed mark_unused_now == true,
+ * which allows would-be dead items to be set LP_UNUSED instead. This
+ * is only possible if the relation has no indexes. If there are any
+ * dead items, then mark_unused_now was not true and every item being
+ * marked LP_UNUSED must refer to a heap-only tuple.
*/
- 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 b63cad1335f..85c0919dc71 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -249,7 +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);
+ LVPagePruneState *prunestate,
+ bool *recordfreespace);
static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
bool *recordfreespace);
@@ -829,8 +830,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,
@@ -958,8 +961,6 @@ lazy_scan_heap(LVRelState *vacrel)
page = BufferGetPage(buf);
if (!ConditionalLockBufferForCleanup(buf))
{
- bool recordfreespace;
-
LockBuffer(buf, BUFFER_LOCK_SHARE);
/* Check for new or empty pages before lazy_scan_noprune call */
@@ -1009,6 +1010,8 @@ lazy_scan_heap(LVRelState *vacrel)
continue;
}
+ tuples_already_deleted = vacrel->tuples_deleted;
+
/*
* Prune, freeze, and count tuples.
*
@@ -1019,73 +1022,10 @@ 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);
+ lazy_scan_prune(vacrel, buf, blkno, page, &prunestate, &recordfreespace);
Assert(!prunestate.all_visible || !prunestate.has_lpdead_items);
- 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
@@ -1196,38 +1136,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;
@@ -1539,7 +1486,8 @@ lazy_scan_prune(LVRelState *vacrel,
Buffer buf,
BlockNumber blkno,
Page page,
- LVPagePruneState *prunestate)
+ LVPagePruneState *prunestate,
+ bool *recordfreespace)
{
Relation rel = vacrel->rel;
OffsetNumber offnum,
@@ -1583,8 +1531,13 @@ lazy_scan_prune(LVRelState *vacrel,
* in presult.ndeleted. It should not be confused with lpdead_items;
* lpdead_items's final value can be thought of as the number of tuples
* that were deleted from indexes.
+ *
+ * If the relation has no indexes, we can immediately mark would-be dead
+ * items LP_UNUSED, so mark_unused_now should be true if no indexes and
+ * false otherwise.
*/
- 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
@@ -1594,6 +1547,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 +1872,15 @@ lazy_scan_prune(LVRelState *vacrel,
/* Can't truncate this page */
if (hastup)
vacrel->nonempty_pages = blkno + 1;
+
+ /*
+ * 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;
}
/*
@@ -2519,7 +2482,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..4b133f68593 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 mark_unused_now,
PruneResult *presult,
OffsetNumber *off_loc);
extern void heap_page_prune_execute(Buffer buffer,
--
2.37.2
From d58851de77c61b70abb6e37b8b7192cba1df756e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Tue, 9 Jan 2024 17:18:06 -0500
Subject: [PATCH v7 4/4] 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 12c2fc78a43..cacd87cf0a9 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];
@@ -1423,7 +1383,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
@@ -1766,7 +1726,7 @@ lazy_scan_prune(LVRelState *vacrel,
*/
if (vacrel->nindexes == 0 ||
!vacrel->do_index_vacuuming || lpdead_items == 0)
- *recordfreespace = true;
+ recordfreespace = true;
Assert(!all_visible || lpdead_items == 0);
@@ -1877,6 +1837,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);
}
/*
@@ -1901,8 +1887,7 @@ static bool
lazy_scan_noprune(LVRelState *vacrel,
Buffer buf,
BlockNumber blkno,
- Page page,
- bool *recordfreespace)
+ Page page)
{
OffsetNumber offnum,
maxoff;
@@ -1911,6 +1896,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;
@@ -1919,7 +1906,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;
@@ -2062,7 +2049,7 @@ lazy_scan_noprune(LVRelState *vacrel,
missed_dead_tuples += lpdead_items;
}
- *recordfreespace = true;
+ recordfreespace = true;
}
else if (lpdead_items == 0)
{
@@ -2070,7 +2057,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
{
@@ -2102,7 +2089,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;
}
/*
@@ -2118,7 +2105,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
