public inbox for [email protected]
help / color / mirror / Atom feedFrom: Melanie Plageman <[email protected]>
To: Andres Freund <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
Date: Fri, 5 Sep 2025 18:20:21 -0400
Message-ID: <CAAKRu_Yz9x0sejBa5ov_LJ5sMOSKM3AeKOFUg+fQpNqyMmxwRA@mail.gmail.com> (raw)
In-Reply-To: <tvvtfoxz5ykpsctxjbzxg3nldnzfc7geplrt2z2s54pmgto27y@hbijsndifu45>
References: <[email protected]>
<CAAKRu_a-aVGxNEdkJt+96HGryQXuQNuXe+PhR0KcnUhXSOPBJw@mail.gmail.com>
<[email protected]>
<CAAKRu_ZH8kL0Zm0j7m7DC9fzk7ru7yf9rm2pEQRvx1iXX25aPQ@mail.gmail.com>
<CAAKRu_bGCgUuhmy1Mzkw3yCmbGcjNQAjV=OmjuW6hz90PuXKWA@mail.gmail.com>
<CALdSSPhAU56g1gGVT0+wG8RrSWE6qW8TOfNJS1HNAWX6wPgbFA@mail.gmail.com>
<CAAKRu_YD0ecXeAh+DmJpzQOJwcRzmMyGdcc5W_0pEF78rYSJkQ@mail.gmail.com>
<CALdSSPhu7WZd+EfQDha1nz=DC93OtY1=UFEdWwSZsASka_2eRQ@mail.gmail.com>
<CAAKRu_a2zU7672weJCGzAE2K44cCwnvsb-BwPh8ET3n1bsKfPQ@mail.gmail.com>
<CAAKRu_Yc1VKM+iuKuJzncPXCYNqQz_jUFBYXuDiPC5k9sUiiQQ@mail.gmail.com>
<tvvtfoxz5ykpsctxjbzxg3nldnzfc7geplrt2z2s54pmgto27y@hbijsndifu45>
Thanks for the review!
On Tue, Sep 2, 2025 at 7:54 PM Andres Freund <[email protected]> wrote:
>
> On 2025-09-02 19:11:01 -0400, Melanie Plageman wrote:
> > From dd98177294011ee93cac122405516abd89f4e393 Mon Sep 17 00:00:00 2001
> > From: Melanie Plageman <[email protected]>
> > Date: Wed, 27 Aug 2025 08:50:15 -0400
> > Subject: [PATCH v8 01/22] Remove unneeded VM pin from VM replay
I didn't push it yet because I did a new version that actually
eliminates the asserts in heap_multi_insert() before calling
visibilitymap_set() -- since they are redundant with checks inside
visibilitymap_set(). 0001 of attached v9 is what I plan to push,
barring any objections.
> > From 7c5cb3edf89735eaa8bee9ca46111bd6c554720b Mon Sep 17 00:00:00 2001
> > From: Melanie Plageman <[email protected]>
> > Date: Wed, 27 Aug 2025 10:07:29 -0400
> > Subject: [PATCH v8 02/22] Add assert and log message to visibilitymap_set
I pushed this.
> From an abstraction POV I don't love that heapam now is responsible for
> acquiring and releasing the lock. But that ship already kind of has sailed, as
> heapam.c is already responsible for releasing the vm buffer etc...
>
> I've wondered about splitting the responsibilities up into multiple
> visibilitymap_set_* functions, so that heapam.c wouldn't need to acquire the
> lock and set the LSN. But it's probably not worth it.
Yea, I explored heap wrappers coupling heap operations related to
setting the VM along with the VM updates [1], but the results weren't
appealing. Setting the heap LSN and marking the heap buffer dirty and
such happens in a different place in different callers because it is
happening as part of the operations that actually end up rendering the
page all-visible.
And a VM-only helper would literally just acquire and release the lock
and set the LSN on the vm page -- which I don't think is worth it.
> > + /*
> > + * Now read and update the VM block. Even if we skipped updating the heap
> > + * page due to the file being dropped or truncated later in recovery, it's
> > + * still safe to update the visibility map. Any WAL record that clears
> > + * the visibility map bit does so before checking the page LSN, so any
> > + * bits that need to be cleared will still be cleared.
> > + *
> > + * It is only okay to set the VM bits without holding the heap page lock
> > + * because we can expect no other writers of this page.
> > + */
> > + if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET &&
> > + XLogReadBufferForRedoExtended(record, 1, RBM_ZERO_ON_ERROR, false,
> > + &vmbuffer) == BLK_NEEDS_REDO)
> > + {
> > + Relation reln = CreateFakeRelcacheEntry(rlocator);
> > +
> > + Assert(visibilitymap_pin_ok(blkno, vmbuffer));
> > + visibilitymap_set_vmbyte(reln, blkno,
> > + vmbuffer,
> > + VISIBILITYMAP_ALL_VISIBLE |
> > + VISIBILITYMAP_ALL_FROZEN);
> > +
> > + /*
> > + * It is not possible that the VM was already set for this heap page,
> > + * so the vmbuffer must have been modified and marked dirty.
> > + */
> > + Assert(BufferIsDirty(vmbuffer));
>
> How about making visibilitymap_set_vmbyte() return whether it needed to do
> something? This seems somewhat indirect...
It does return the state of the previous bits. But, I am specifically
asserting that the buffer is dirty because I am about to set the page
LSN. So I don't just care that changes were made, I care that we
remembered to mark the buffer dirty.
> I think it might be good to encapsulate this code into a helper in
> visibilitymap.c, there will be more callers in the subsequent patches.
By the end of the set, the different callers have different
expectations (some don't expect the buffer to have been dirtied
necessarily) and where they do the various related operations is
spread out depending on the caller. I just couldn't come up with a
helper solution I liked.
That being said, I definitely don't think it's needed for this patch
(logging setting the VM in xl_heap_multi_insert()).
> > +uint8
> > +visibilitymap_set_vmbyte(Relation rel, BlockNumber heapBlk,
> > + Buffer vmBuf, uint8 flags)
>
> Why is it named vmbyte? This actually just sets the two bits corresponding to
> the buffer, not the entire byte. So it seems somewhat misleading to reference
> byte.
Renamed it to visibilitymap_set_vmbits.
> > Instead of emitting a separate xl_heap_visible record for each page that
> > is rendered all-visible by vacuum's third phase, include the updates to
> > the VM in the already emitted xl_heap_prune record.
>
> Reading through the change I didn't particularly like that there's another
> optional field in xl_heap_prune, as it seemed liked something that should be
> encoded in flags. Of course there aren't enough flag bits available. But
> that made me look at the rest of the record: Uh, what do we use the reason
> field for? As far as I can tell f83d709760d8 added it without introducing any
> users? It doesn't even seem to be set.
yikes, you are right about the "reason" member. Attached 0002 removes
it, and I'll go ahead and fix it in the back branches too. I can't
fathom how that slipped through the cracks. We do pass the PruneReason
for setting the rmgr info about what type of record it is (i.e. if it
is one emitted by vacuum phase I, phase III, or on-access pruning).
But we don't need or use a separate member.. I went back and tried to
figure out what the rationale was, but I couldn't find anything.
As for the VM flags being an optional unaligned member -- in v9, I've
expanded the flags member to a uint16 to make room for the extra
flags. Seems we've been surviving with using up 2 bytes this long.
> > @@ -51,10 +52,15 @@ heap_xlog_prune_freeze(XLogReaderState *record)
> > (xlrec.flags & (XLHP_HAS_REDIRECTIONS | XLHP_HAS_DEAD_ITEMS)) == 0);
> >
> > /*
> > - * We are about to remove and/or freeze tuples. In Hot Standby mode,
> > - * ensure that there are no queries running for which the removed tuples
> > - * are still visible or which still consider the frozen xids as running.
> > - * The conflict horizon XID comes after xl_heap_prune.
> > + * After xl_heap_prune is the optional snapshot conflict horizon.
> > + *
> > + * In Hot Standby mode, we must ensure that there are no running queries
> > + * which would conflict with the changes in this record. If pruning, that
> > + * means we cannot remove tuples still visible to transactions on the
> > + * standby. If freezing, that means we cannot freeze tuples with xids that
> > + * are still considered running on the standby. And for setting the VM, we
> > + * cannot do so if the page isn't all-visible to all transactions on the
> > + * standby.
> > */
>
> I'm a bit confused by this new comment - it sounds like we're deciding whether
> to remove tuple versions, but that decision has long been made, no?
Well, the comment is a revision of a comment that was already there on
essentially why replaying this record could cause recovery conflicts.
It mentioned pruning and freezing, so I expanded it to mention setting
the VM. Taking into account your confusion, I tried rewording it in
attached v9.
> > + if (heap_page_is_all_visible_except_lpdead(vacrel->rel, buffer,
> > + vacrel->cutoffs.OldestXmin,
> > + deadoffsets, num_offsets,
> > + &all_frozen, &visibility_cutoff_xid,
> > + &vacrel->offnum))
>
> I am rather confused - we never can set all-visible if there are any LP_DEAD
> items left. If the idea is that we are removing the LP_DEAD items in
> lazy_vacuum_heap_page() - what guarantees that all LP_DEAD items are being
> removed? Couldn't some tuples get marked LP_DEAD by on-access pruning, after
> vacuum visited the page and collected dead items?
>
> Ugh, I see - it works because we pass in the set of dead items. I think that
> makes the name *really* misleading, it's not except LP_DEAD, it's except the
> offsets passed in, no?
>
> But then you actually check that the set of dead items didn't change - what
> guarantees that?
So, I pass in the deadoffsets we got from the TIDStore. If the only
dead items on the page are exactly those dead items, then the page
will be all-visible as soon as we set those LP_UNUSED -- which we do
unconditionally. And we have the lock on the page, so no one can
on-access prune and make new dead items while we are in
lazy_vacuum_heap_page().
Given your confusion, I've refactored this and used a different
approach -- I explicitly check the passed-in deadoffsets array when I
encounter a dead item and see if it is there. That should hopefully
make it more clear.
> I didn't look at the later patches, except that I did notice this:
<--snip-->
> Why are we manually pinning the vm buffer here? Shouldn't the xlog machinery
> have done so, as you noticed in one of the early on patches?
Fixed. Thanks!
- Melanie
[1] [1] https://www.postgresql.org/message-id/flat/CAAKRu_Yj%3DyrL%2BgGGsqfYVQcYn7rDp6hDeoF1vN453JDp8dEY%2Bw...
Attachments:
[text/x-patch] v9-0002-Remove-unused-xl_heap_prune-member-reason.patch (1.1K, 2-v9-0002-Remove-unused-xl_heap_prune-member-reason.patch)
download | inline diff:
From df9b87d0a1a973c0c655f5ba858485795ff98951 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 3 Sep 2025 15:02:58 -0400
Subject: [PATCH v9 02/22] Remove unused xl_heap_prune member, reason
f83d709760d8 refactored xl_heap_prune and added an unused member,
reason. While PruneReason is used when constructing this WAL record to
set the WAL record definition, it doesn't need to be stored in a
separate field in the record. Remove it.
Author: Melanie Plageman <[email protected]>
Reported-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/tvvtfoxz5ykpsctxjbzxg3nldnzfc7geplrt2z2s54pmgto27y%40hbijsndifu45
---
src/include/access/heapam_xlog.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 277df6b3cf0..d4c0625b632 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -284,7 +284,6 @@ typedef struct xl_heap_update
*/
typedef struct xl_heap_prune
{
- uint8 reason;
uint8 flags;
/*
--
2.43.0
[text/x-patch] v9-0005-Eliminate-xl_heap_visible-from-vacuum-phase-III.patch (28.4K, 3-v9-0005-Eliminate-xl_heap_visible-from-vacuum-phase-III.patch)
download | inline diff:
From 81b134346c1a981382d1eb915472aa3f26bb3586 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 18 Jun 2025 12:42:13 -0400
Subject: [PATCH v9 05/22] Eliminate xl_heap_visible from vacuum phase III
Instead of emitting a separate xl_heap_visible record for each page that
is rendered all-visible by vacuum's third phase, include the updates to
the VM in the already emitted xl_heap_prune record.
The visibilitymap bits are stored in the flags member of the
xl_heap_prune record.
Author: Melanie Plageman <[email protected]>
Reviewed-by: Kirill Reshke <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/flat/CAAKRu_ZMw6Npd_qm2KM%2BFwQ3cMOMx1Dh3VMhp8-V7SOLxdK9-g%40mail.gmail.com
---
src/backend/access/heap/heapam_xlog.c | 145 ++++++++++++++++++----
src/backend/access/heap/pruneheap.c | 66 ++++++++--
src/backend/access/heap/vacuumlazy.c | 164 +++++++++++++++++--------
src/backend/access/rmgrdesc/heapdesc.c | 7 +-
src/include/access/heapam.h | 9 ++
src/include/access/heapam_xlog.h | 36 ++++--
6 files changed, 330 insertions(+), 97 deletions(-)
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index 0820f7d052d..11c11929ed9 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -35,7 +35,9 @@ heap_xlog_prune_freeze(XLogReaderState *record)
Buffer buffer;
RelFileLocator rlocator;
BlockNumber blkno;
- XLogRedoAction action;
+ Buffer vmbuffer = InvalidBuffer;
+ uint8 vmflags = 0;
+ Size freespace = 0;
XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno);
memcpy(&xlrec, maindataptr, SizeOfHeapPrune);
@@ -50,11 +52,17 @@ heap_xlog_prune_freeze(XLogReaderState *record)
Assert((xlrec.flags & XLHP_CLEANUP_LOCK) != 0 ||
(xlrec.flags & (XLHP_HAS_REDIRECTIONS | XLHP_HAS_DEAD_ITEMS)) == 0);
+ vmflags = xlrec.flags & VISIBILITYMAP_VALID_BITS;
+
/*
- * We are about to remove and/or freeze tuples. In Hot Standby mode,
- * ensure that there are no queries running for which the removed tuples
- * are still visible or which still consider the frozen xids as running.
- * The conflict horizon XID comes after xl_heap_prune.
+ * After xl_heap_prune is the optional snapshot conflict horizon.
+ *
+ * In Hot Standby mode, we must ensure that there are no running queries
+ * which would conflict with the changes in this record. That means we
+ * can't replay this record if it removes tuples that are still visible to
+ * transactions on the standby, freeze tuples with xids that are still
+ * considered running on the standby, or set a page as all-visible in the
+ * VM if it isn't all-visible to all transactions on the standby.
*/
if ((xlrec.flags & XLHP_HAS_CONFLICT_HORIZON) != 0)
{
@@ -71,12 +79,12 @@ heap_xlog_prune_freeze(XLogReaderState *record)
}
/*
- * If we have a full-page image, restore it and we're done.
+ * If we have a full-page image of the heap block, restore it and we're
+ * done with the heap block.
*/
- action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL,
- (xlrec.flags & XLHP_CLEANUP_LOCK) != 0,
- &buffer);
- if (action == BLK_NEEDS_REDO)
+ if (XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL,
+ (xlrec.flags & XLHP_CLEANUP_LOCK) != 0,
+ &buffer) == BLK_NEEDS_REDO)
{
Page page = BufferGetPage(buffer);
OffsetNumber *redirected;
@@ -89,6 +97,9 @@ heap_xlog_prune_freeze(XLogReaderState *record)
Size datalen;
xlhp_freeze_plan *plans;
OffsetNumber *frz_offsets;
+ bool do_prune;
+ bool mark_buffer_dirty;
+ bool set_heap_lsn;
char *dataptr = XLogRecGetBlockData(record, 0, &datalen);
heap_xlog_deserialize_prune_and_freeze(dataptr, xlrec.flags,
@@ -97,11 +108,18 @@ heap_xlog_prune_freeze(XLogReaderState *record)
&ndead, &nowdead,
&nunused, &nowunused);
+ do_prune = nredirected > 0 || ndead > 0 || nunused > 0;
+ set_heap_lsn = mark_buffer_dirty = do_prune || nplans > 0;
+
+ /* Ensure the record does something */
+ Assert(do_prune || nplans > 0 ||
+ vmflags & VISIBILITYMAP_VALID_BITS);
+
/*
* Update all line pointers per the record, and repair fragmentation
* if needed.
*/
- if (nredirected > 0 || ndead > 0 || nunused > 0)
+ if (do_prune)
heap_page_prune_execute(buffer,
(xlrec.flags & XLHP_CLEANUP_LOCK) == 0,
redirected, nredirected,
@@ -138,36 +156,117 @@ heap_xlog_prune_freeze(XLogReaderState *record)
/* There should be no more data */
Assert((char *) frz_offsets == dataptr + datalen);
+ /*
+ * Now set PD_ALL_VISIBLE, if required. We'll only do this if we are
+ * also going to set bits in the VM later.
+ *
+ * We must never end up with the VM bit set and the page-level
+ * PD_ALL_VISIBLE bit clear. If that were to occur, a subsequent page
+ * modification would fail to clear the VM bit.
+ */
+ if ((vmflags & VISIBILITYMAP_VALID_BITS) && !PageIsAllVisible(page))
+ {
+ PageSetAllVisible(page);
+
+ /*
+ * If the only change to the heap page is setting PD_ALL_VISIBLE,
+ * we can avoid setting the page LSN unless checksums or
+ * wal_log_hints are enabled.
+ */
+ set_heap_lsn = XLogHintBitIsNeeded() ? true : set_heap_lsn;
+ mark_buffer_dirty = true;
+ }
+
/*
* Note: we don't worry about updating the page's prunability hints.
* At worst this will cause an extra prune cycle to occur soon.
*/
- PageSetLSN(page, lsn);
- MarkBufferDirty(buffer);
+ if (mark_buffer_dirty)
+ MarkBufferDirty(buffer);
+ if (set_heap_lsn)
+ PageSetLSN(page, lsn);
}
/*
- * If we released any space or line pointers, update the free space map.
+ * If we released any space or line pointers or will be setting a page in
+ * the visibility map, measure the page's freespace to later update the
+ * freespace map.
+ *
+ * Even if we are just updating the VM (and thus not freeing up any
+ * space), we'll still update the FSM for this page. Since FSM is not
+ * WAL-logged and only updated heuristically, it easily becomes stale in
+ * standbys. If the standby is later promoted and runs VACUUM, it will
+ * skip updating individual free space figures for pages that became
+ * all-visible (or all-frozen, depending on the vacuum mode,) which is
+ * troublesome when FreeSpaceMapVacuum propagates too optimistic free
+ * space values to upper FSM layers; later inserters try to use such pages
+ * only to find out that they are unusable. This can cause long stalls
+ * when there are many such pages.
+ *
+ * Forestall those problems by updating FSM's idea about a page that is
+ * becoming all-visible or all-frozen.
*
* Do this regardless of a full-page image being applied, since the FSM
* data is not in the page anyway.
+ *
+ * We want to avoid holding an exclusive lock on the heap buffer while
+ * doing IO (either of the FSM or the VM), so we'll release the lock on
+ * the heap buffer before doing either.
*/
if (BufferIsValid(buffer))
{
- if (xlrec.flags & (XLHP_HAS_REDIRECTIONS |
- XLHP_HAS_DEAD_ITEMS |
- XLHP_HAS_NOW_UNUSED_ITEMS))
- {
- Size freespace = PageGetHeapFreeSpace(BufferGetPage(buffer));
+ if ((xlrec.flags & (XLHP_HAS_REDIRECTIONS |
+ XLHP_HAS_DEAD_ITEMS |
+ XLHP_HAS_NOW_UNUSED_ITEMS)) ||
+ vmflags & VISIBILITYMAP_VALID_BITS)
+ freespace = PageGetHeapFreeSpace(BufferGetPage(buffer));
+
+ UnlockReleaseBuffer(buffer);
+ }
+
+ /*
+ * Read and update the VM block. Even if we skipped updating the heap page
+ * due to the file being dropped or truncated later in recovery, it's
+ * still safe to update the visibility map. Any WAL record that clears
+ * the visibility map bit does so before checking the page LSN, so any
+ * bits that need to be cleared will still be cleared.
+ *
+ * Note that it is _only_ okay that we do not hold a lock on the heap page
+ * because we are in recovery and can expect no other writers to clear
+ * PD_ALL_VISIBLE before we are able to update the VM.
+ */
+ if (vmflags & VISIBILITYMAP_VALID_BITS &&
+ XLogReadBufferForRedoExtended(record, 1,
+ RBM_ZERO_ON_ERROR,
+ false,
+ &vmbuffer) == BLK_NEEDS_REDO)
+ {
+ Page vmpage = BufferGetPage(vmbuffer);
+ uint8 old_vmbits = 0;
+ Relation reln = CreateFakeRelcacheEntry(rlocator);
- UnlockReleaseBuffer(buffer);
+ /* initialize the page if it was read as zeros */
+ if (PageIsNew(vmpage))
+ PageInit(vmpage, BLCKSZ, 0);
+
+ old_vmbits = visibilitymap_set_vmbits(reln, blkno, vmbuffer, vmflags);
- XLogRecordPageWithFreeSpace(rlocator, blkno, freespace);
+ /* Only set VM page LSN if we modified the page */
+ if (old_vmbits != vmflags)
+ {
+ Assert(BufferIsDirty(vmbuffer));
+ PageSetLSN(BufferGetPage(vmbuffer), lsn);
}
- else
- UnlockReleaseBuffer(buffer);
+
+ FreeFakeRelcacheEntry(reln);
}
+
+ if (BufferIsValid(vmbuffer))
+ UnlockReleaseBuffer(vmbuffer);
+
+ if (freespace > 0)
+ XLogRecordPageWithFreeSpace(rlocator, blkno, freespace);
}
/*
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7ebd22f00a3..f0b33d1b696 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -21,6 +21,7 @@
#include "access/transam.h"
#include "access/xlog.h"
#include "access/xloginsert.h"
+#include "access/visibilitymapdefs.h"
#include "commands/vacuum.h"
#include "executor/instrument.h"
#include "miscadmin.h"
@@ -835,6 +836,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
conflict_xid = prstate.latest_xid_removed;
log_heap_prune_and_freeze(relation, buffer,
+ InvalidBuffer, 0, false,
conflict_xid,
true, reason,
prstate.frozen, prstate.nfrozen,
@@ -2030,14 +2032,18 @@ heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
*
* This is used for several different page maintenance operations:
*
- * - Page pruning, in VACUUM's 1st pass or on access: Some items are
+ * - Page pruning, in vacuum phase I or on-access: Some items are
* redirected, some marked dead, and some removed altogether.
*
- * - Freezing: Items are marked as 'frozen'.
+ * - Freezing: During vacuum phase I, items are marked as 'frozen'
*
- * - Vacuum, 2nd pass: Items that are already LP_DEAD are marked as unused.
+ * - Reaping: During vacuum phase III, items that are already LP_DEAD are
+ * marked as unused.
*
- * They have enough commonalities that we use a single WAL record for them
+ * - VM updates: After vacuum phase III, the heap page may be marked
+ * all-visible and all-frozen.
+ *
+ * These changes all happen together, so we use a singel WAL record for them
* all.
*
* If replaying the record requires a cleanup lock, pass cleanup_lock = true.
@@ -2045,12 +2051,23 @@ heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
* replaying 'unused' items depends on whether they were all previously marked
* as dead.
*
+ * If the VM is being updated, vmflags will contain the bits to set. In this
+ * case, vmbuffer should already have been updated and marked dirty and should
+ * still be pinned and locked.
+ *
+ * set_pd_all_vis indicates that we set PD_ALL_VISIBLE and thus should update
+ * the page LSN when checksums/wal_log_hints are enabled even if we did not
+ * prune or freeze tuples on the page.
+ *
* Note: This function scribbles on the 'frozen' array.
*
* Note: This is called in a critical section, so careful what you do here.
*/
void
log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ Buffer vmbuffer,
+ uint8 vmflags,
+ bool set_pd_all_vis,
TransactionId conflict_xid,
bool cleanup_lock,
PruneReason reason,
@@ -2062,6 +2079,7 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
xl_heap_prune xlrec;
XLogRecPtr recptr;
uint8 info;
+ uint8 regbuf_flags;
/* The following local variables hold data registered in the WAL record: */
xlhp_freeze_plan plans[MaxHeapTuplesPerPage];
@@ -2070,8 +2088,21 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
xlhp_prune_items dead_items;
xlhp_prune_items unused_items;
OffsetNumber frz_offsets[MaxHeapTuplesPerPage];
+ bool do_prune = nredirected > 0 || ndead > 0 || nunused > 0;
+
+ Assert((vmflags & VISIBILITYMAP_VALID_BITS) == vmflags);
+ xlrec.flags = vmflags;
- xlrec.flags = 0;
+ regbuf_flags = REGBUF_STANDARD;
+
+ /*
+ * We can avoid an FPI if the only modification we are making to the heap
+ * page is to set PD_ALL_VISIBLE and checksums/wal_log_hints are disabled.
+ */
+ if (!do_prune &&
+ nfrozen == 0 &&
+ (!set_pd_all_vis || !XLogHintBitIsNeeded()))
+ regbuf_flags |= REGBUF_NO_IMAGE;
/*
* Prepare data for the buffer. The arrays are not actually in the
@@ -2079,7 +2110,11 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
* page image, the arrays can be omitted.
*/
XLogBeginInsert();
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+ XLogRegisterBuffer(0, buffer, regbuf_flags);
+
+ if (vmflags & VISIBILITYMAP_VALID_BITS)
+ XLogRegisterBuffer(1, vmbuffer, 0);
+
if (nfrozen > 0)
{
int nplans;
@@ -2168,5 +2203,22 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
}
recptr = XLogInsert(RM_HEAP2_ID, info);
- PageSetLSN(BufferGetPage(buffer), recptr);
+ if (vmflags & VISIBILITYMAP_VALID_BITS)
+ {
+ Assert(BufferIsDirty(vmbuffer));
+ PageSetLSN(BufferGetPage(vmbuffer), recptr);
+ }
+
+ /*
+ * If pruning or freezing tuples or setting the page all-visible when
+ * checksums or wal_hint_bits are enabled, we must bump the LSN. Torn
+ * pages are possible if we update PD_ALL_VISIBLE without bumping the LSN,
+ * but this is deemed okay for page hint updates.
+ */
+ if (do_prune || nfrozen > 0 ||
+ (set_pd_all_vis && XLogHintBitIsNeeded()))
+ {
+ Assert(BufferIsDirty(buffer));
+ PageSetLSN(BufferGetPage(buffer), recptr);
+ }
}
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 7f6f684bc63..a50652ca5a0 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -463,11 +463,13 @@ static void dead_items_add(LVRelState *vacrel, BlockNumber blkno, OffsetNumber *
int num_offsets);
static void dead_items_reset(LVRelState *vacrel);
static void dead_items_cleanup(LVRelState *vacrel);
-static bool heap_page_is_all_visible(Relation rel, Buffer buf,
- TransactionId OldestXmin,
- bool *all_frozen,
- TransactionId *visibility_cutoff_xid,
- OffsetNumber *logging_offnum);
+static bool heap_page_would_be_all_visible(Relation rel, Buffer buf,
+ TransactionId OldestXmin,
+ OffsetNumber *deadoffsets,
+ int ndeadoffsets,
+ bool *all_frozen,
+ TransactionId *visibility_cutoff_xid,
+ OffsetNumber *logging_offnum);
static void update_relstats_all_indexes(LVRelState *vacrel);
static void vacuum_error_callback(void *arg);
static void update_vacuum_error_info(LVRelState *vacrel,
@@ -2846,8 +2848,11 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
OffsetNumber unused[MaxHeapTuplesPerPage];
int nunused = 0;
TransactionId visibility_cutoff_xid;
+ TransactionId conflict_xid = InvalidTransactionId;
bool all_frozen;
LVSavedErrInfo saved_err_info;
+ uint8 vmflags = 0;
+ bool set_pd_all_vis = false;
Assert(vacrel->do_index_vacuuming);
@@ -2858,6 +2863,20 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
VACUUM_ERRCB_PHASE_VACUUM_HEAP, blkno,
InvalidOffsetNumber);
+ if (heap_page_would_be_all_visible(vacrel->rel, buffer,
+ vacrel->cutoffs.OldestXmin,
+ deadoffsets, num_offsets,
+ &all_frozen, &visibility_cutoff_xid,
+ &vacrel->offnum))
+ {
+ vmflags |= VISIBILITYMAP_ALL_VISIBLE;
+ if (all_frozen)
+ {
+ vmflags |= VISIBILITYMAP_ALL_FROZEN;
+ Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+ }
+ }
+
START_CRIT_SECTION();
for (int i = 0; i < num_offsets; i++)
@@ -2877,6 +2896,18 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
/* Attempt to truncate line pointer array now */
PageTruncateLinePointerArray(page);
+ if ((vmflags & VISIBILITYMAP_VALID_BITS) != 0)
+ {
+ Assert(!PageIsAllVisible(page));
+ set_pd_all_vis = true;
+ PageSetAllVisible(page);
+ LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
+ visibilitymap_set_vmbits(vacrel->rel,
+ blkno,
+ vmbuffer, vmflags);
+ conflict_xid = visibility_cutoff_xid;
+ }
+
/*
* Mark buffer dirty before we write WAL.
*/
@@ -2886,7 +2917,10 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
if (RelationNeedsWAL(vacrel->rel))
{
log_heap_prune_and_freeze(vacrel->rel, buffer,
- InvalidTransactionId,
+ vmbuffer,
+ vmflags,
+ set_pd_all_vis,
+ conflict_xid,
false, /* no cleanup lock required */
PRUNE_VACUUM_CLEANUP,
NULL, 0, /* frozen */
@@ -2895,39 +2929,12 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
unused, nunused);
}
- /*
- * End critical section, so we safely can do visibility tests (which
- * possibly need to perform IO and allocate memory!). If we crash now the
- * page (including the corresponding vm bit) might not be marked all
- * visible, but that's fine. A later vacuum will fix that.
- */
END_CRIT_SECTION();
- /*
- * Now that we have removed the LP_DEAD items from the page, once again
- * check if the page has become all-visible. The page is already marked
- * dirty, exclusively locked, and, if needed, a full page image has been
- * emitted.
- */
- Assert(!PageIsAllVisible(page));
- if (heap_page_is_all_visible(vacrel->rel, buffer, vacrel->cutoffs.OldestXmin,
- &all_frozen, &visibility_cutoff_xid, &vacrel->offnum))
+ if ((vmflags & VISIBILITYMAP_ALL_VISIBLE) != 0)
{
- uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
-
- if (all_frozen)
- {
- Assert(!TransactionIdIsValid(visibility_cutoff_xid));
- flags |= VISIBILITYMAP_ALL_FROZEN;
- }
-
- PageSetAllVisible(page);
- visibilitymap_set(vacrel->rel, blkno, buffer,
- InvalidXLogRecPtr,
- vmbuffer, visibility_cutoff_xid,
- flags);
-
/* Count the newly set VM page for logging */
+ LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK);
vacrel->vm_new_visible_pages++;
if (all_frozen)
vacrel->vm_new_visible_frozen_pages++;
@@ -3594,40 +3601,85 @@ dead_items_cleanup(LVRelState *vacrel)
}
/*
- * Check if every tuple in the given page in buf is visible to all current and
- * future transactions.
+ * Wrapper for heap_page_would_be_all_visible() which can be used for
+ * callers that expect no LP_DEAD on the page.
+ */
+bool
+heap_page_is_all_visible(Relation rel, Buffer buf,
+ TransactionId OldestXmin,
+ bool *all_frozen,
+ TransactionId *visibility_cutoff_xid,
+ OffsetNumber *logging_offnum)
+{
+
+ return heap_page_would_be_all_visible(rel, buf, OldestXmin,
+ NULL, 0,
+ all_frozen,
+ visibility_cutoff_xid,
+ logging_offnum);
+}
+
+/*
+ * Determines whether or not the heap page in buf is all-visible other than
+ * the dead line pointers referred to by the provided deadoffsets array.
*
- * OldestXmin is used to determine visibility.
+ * deadoffsets are the offsets the caller knows about and already removed
+ * associated index entries. Vacuum will call this before setting those line
+ * pointers LP_UNUSED. So, if there are no new LP_DEAD items, then the page
+ * can be set all-visible in the VM by the caller.
+ *
+ * Returns true if the page is all-visible other than the provided
+ * deadoffsets and false otherwise.
*
- * Sets *all_frozen to true if every tuple on this page is frozen.
+ * OldestXmin is used to determine visibility.
*
- * Sets *visibility_cutoff_xid to the highest xmin amongst the visible tuples.
- * It is only valid if the page is all-visible.
+ * *all_frozen is an output parameter indicating to the caller if every tuple
+ * on the page is frozen.
*
* *logging_offnum will have the OffsetNumber of the current tuple being
* processed for vacuum's error callback system.
*
- * This is a stripped down version of lazy_scan_prune(). If you change
- * anything here, make sure that everything stays in sync. Note that an
- * assertion calls us to verify that everybody still agrees. Be sure to avoid
- * introducing new side-effects here.
+ * *visibility_cutoff_xid is an output parameter with the highest xmin amongst the
+ * visible tuples. It is only valid if the page is all-visible.
+ *
+ * Callers looking to verify that the page is already all-visible can call
+ * heap_page_is_all_visible().
+ *
+ * This is similar logic to that in heap_prune_record_unchanged_lp_normal() If
+ * you change anything here, make sure that everything stays in sync. Note
+ * that an assertion calls us to verify that everybody still agrees. Be sure
+ * to avoid introducing new side-effects here.
*/
static bool
-heap_page_is_all_visible(Relation rel, Buffer buf,
- TransactionId OldestXmin,
- bool *all_frozen,
- TransactionId *visibility_cutoff_xid,
- OffsetNumber *logging_offnum)
+heap_page_would_be_all_visible(Relation rel, Buffer buf,
+ TransactionId OldestXmin,
+ OffsetNumber *deadoffsets,
+ int ndeadoffsets,
+ bool *all_frozen,
+ TransactionId *visibility_cutoff_xid,
+ OffsetNumber *logging_offnum)
{
Page page = BufferGetPage(buf);
BlockNumber blockno = BufferGetBlockNumber(buf);
OffsetNumber offnum,
maxoff;
bool all_visible = true;
+ int matched_dead_count = 0;
*visibility_cutoff_xid = InvalidTransactionId;
*all_frozen = true;
+ Assert(ndeadoffsets == 0 || deadoffsets);
+
+#ifdef USE_ASSERT_CHECKING
+ /* Confirm input deadoffsets[] is strictly sorted */
+ if (ndeadoffsets > 1)
+ {
+ for (int i = 1; i < ndeadoffsets; i++)
+ Assert(deadoffsets[i - 1] < deadoffsets[i]);
+ }
+#endif
+
maxoff = PageGetMaxOffsetNumber(page);
for (offnum = FirstOffsetNumber;
offnum <= maxoff && all_visible;
@@ -3655,9 +3707,15 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
*/
if (ItemIdIsDead(itemid))
{
- all_visible = false;
- *all_frozen = false;
- break;
+ if (!deadoffsets ||
+ matched_dead_count >= ndeadoffsets ||
+ deadoffsets[matched_dead_count] != offnum)
+ {
+ *all_frozen = all_visible = false;
+ break;
+ }
+ matched_dead_count++;
+ continue;
}
Assert(ItemIdIsNormal(itemid));
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index b48d7dc1d24..c95d30dfe8d 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -103,7 +103,7 @@ plan_elem_desc(StringInfo buf, void *plan, void *data)
* code, the latter of which is used in frontend (pg_waldump) code.
*/
void
-heap_xlog_deserialize_prune_and_freeze(char *cursor, uint8 flags,
+heap_xlog_deserialize_prune_and_freeze(char *cursor, uint16 flags,
int *nplans, xlhp_freeze_plan **plans,
OffsetNumber **frz_offsets,
int *nredirected, OffsetNumber **redirected,
@@ -279,7 +279,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
TransactionId conflict_xid;
memcpy(&conflict_xid, rec + SizeOfHeapPrune, sizeof(TransactionId));
-
appendStringInfo(buf, "snapshotConflictHorizon: %u",
conflict_xid);
}
@@ -287,6 +286,10 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
appendStringInfo(buf, ", isCatalogRel: %c",
xlrec->flags & XLHP_IS_CATALOG_REL ? 'T' : 'F');
+ if (xlrec->flags & VISIBILITYMAP_VALID_BITS)
+ appendStringInfo(buf, ", vm_flags: 0x%02X",
+ xlrec->flags & VISIBILITYMAP_VALID_BITS);
+
if (XLogRecHasBlockData(record, 0))
{
Size datalen;
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index a2bd5a897f8..8b47295efa2 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -344,6 +344,12 @@ extern void heap_inplace_update_and_unlock(Relation relation,
Buffer buffer);
extern void heap_inplace_unlock(Relation relation,
HeapTuple oldtup, Buffer buffer);
+
+extern bool heap_page_is_all_visible(Relation rel, Buffer buf,
+ TransactionId OldestXmin,
+ bool *all_frozen,
+ TransactionId *visibility_cutoff_xid,
+ OffsetNumber *logging_offnum);
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
const struct VacuumCutoffs *cutoffs,
HeapPageFreeze *pagefrz,
@@ -388,6 +394,9 @@ extern void heap_page_prune_execute(Buffer buffer, bool lp_truncate_only,
OffsetNumber *nowunused, int nunused);
extern void heap_get_root_tuples(Page page, OffsetNumber *root_offsets);
extern void log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ Buffer vmbuffer,
+ uint8 vmflags,
+ bool vm_modified_heap_page,
TransactionId conflict_xid,
bool cleanup_lock,
PruneReason reason,
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index d4c0625b632..d8508593e7c 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -249,7 +249,7 @@ typedef struct xl_heap_update
* Main data section:
*
* xl_heap_prune
- * uint8 flags
+ * uint16 flags
* TransactionId snapshot_conflict_horizon
*
* Block 0 data section:
@@ -284,7 +284,7 @@ typedef struct xl_heap_update
*/
typedef struct xl_heap_prune
{
- uint8 flags;
+ uint16 flags;
/*
* If XLHP_HAS_CONFLICT_HORIZON is set, the conflict horizon XID follows,
@@ -292,10 +292,22 @@ typedef struct xl_heap_prune
*/
} xl_heap_prune;
-#define SizeOfHeapPrune (offsetof(xl_heap_prune, flags) + sizeof(uint8))
+#define SizeOfHeapPrune (offsetof(xl_heap_prune, flags) + sizeof(uint16))
+
+/*
+ * The xl_heap_prune record's flags may also contain which VM bits to set. As
+ * such, (1 << 0) and (1 << 1) are reserved for VISIBILITYMAP_ALL_VISIBLE and
+ * VISIBILITYMAP_ALL_FROZEN.
+ */
-/* to handle recovery conflict during logical decoding on standby */
-#define XLHP_IS_CATALOG_REL (1 << 1)
+/*
+ * To handle recovery conflict during logical decoding on standby, we must know
+ * if the table is a catalog table. Note that in visibilitymapdefs.h
+ * VISIBLITYMAP_XLOG_CATALOG_REL is also defined as (1 << 2). xl_heap_prune
+ * records should use XLHP_IS_CATALOG_REL, not VISIBILIYTMAP_XLOG_CATALOG_REL --
+ * even if they only contain updates to the VM.
+ */
+#define XLHP_IS_CATALOG_REL (1 << 2)
/*
* Does replaying the record require a cleanup-lock?
@@ -305,7 +317,7 @@ typedef struct xl_heap_prune
* marks LP_DEAD line pointers as unused without moving any tuple data, an
* ordinary exclusive lock is sufficient.
*/
-#define XLHP_CLEANUP_LOCK (1 << 2)
+#define XLHP_CLEANUP_LOCK (1 << 3)
/*
* If we remove or freeze any entries that contain xids, we need to include a
@@ -313,22 +325,22 @@ typedef struct xl_heap_prune
* there are no queries running for which the removed tuples are still
* visible, or which still consider the frozen XIDs as running.
*/
-#define XLHP_HAS_CONFLICT_HORIZON (1 << 3)
+#define XLHP_HAS_CONFLICT_HORIZON (1 << 4)
/*
* Indicates that an xlhp_freeze_plans sub-record and one or more
* xlhp_freeze_plan sub-records are present.
*/
-#define XLHP_HAS_FREEZE_PLANS (1 << 4)
+#define XLHP_HAS_FREEZE_PLANS (1 << 5)
/*
* XLHP_HAS_REDIRECTIONS, XLHP_HAS_DEAD_ITEMS, and XLHP_HAS_NOW_UNUSED_ITEMS
* indicate that xlhp_prune_items sub-records with redirected, dead, and
* unused item offsets are present.
*/
-#define XLHP_HAS_REDIRECTIONS (1 << 5)
-#define XLHP_HAS_DEAD_ITEMS (1 << 6)
-#define XLHP_HAS_NOW_UNUSED_ITEMS (1 << 7)
+#define XLHP_HAS_REDIRECTIONS (1 << 6)
+#define XLHP_HAS_DEAD_ITEMS (1 << 7)
+#define XLHP_HAS_NOW_UNUSED_ITEMS (1 << 8)
/*
* xlhp_freeze_plan describes how to freeze a group of one or more heap tuples
@@ -497,7 +509,7 @@ extern XLogRecPtr log_heap_visible(Relation rel, Buffer heap_buffer,
uint8 vmflags);
/* in heapdesc.c, so it can be shared between frontend/backend code */
-extern void heap_xlog_deserialize_prune_and_freeze(char *cursor, uint8 flags,
+extern void heap_xlog_deserialize_prune_and_freeze(char *cursor, uint16 flags,
int *nplans, xlhp_freeze_plan **plans,
OffsetNumber **frz_offsets,
int *nredirected, OffsetNumber **redirected,
--
2.43.0
[text/x-patch] v9-0001-Remove-unneeded-VM-pin-from-VM-replay.patch (2.5K, 4-v9-0001-Remove-unneeded-VM-pin-from-VM-replay.patch)
download | inline diff:
From 686edbfbe6556da8cdd6219fd9cd270ccfc9bb32 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 27 Aug 2025 08:50:15 -0400
Subject: [PATCH v9 01/22] Remove unneeded VM pin from VM replay
Previously, heap_xlog_visible() called visibilitymap_pin() even after
getting a buffer from XLogReadBufferForRedoExtended() -- which returns a
pinned buffer containing the specified block of the visibility map.
This would just have resulted in visibilitymap_pin() returning early
since the specified page was already present and pinned, but it was
confusing extraneous code, so remove it.
It appears to be an oversight in 2c03216.
While we are at it, remove two VM-related redundant asserts in the COPY
FREEZE code path. visibilitymap_set() already asserts that
PD_ALL_VISIBLE is set on the heap page and checks that the vmbuffer
contains the bits corresponding to the specified heap block, so callers
do not also need to check this.
Author: Melanie Plageman <[email protected]>
Reported-by: Melanie Plageman <[email protected]>
Reported-by: Kirill Reshke <[email protected]>
Reviewed-by: Kirill Reshke <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CALdSSPhu7WZd%2BEfQDha1nz%3DDC93OtY1%3DUFEdWwSZsASka_2eRQ%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 3 ---
src/backend/access/heap/heapam_xlog.c | 1 -
2 files changed, 4 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e3e7307ef5f..4c5ae205a7a 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2647,9 +2647,6 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
*/
if (all_frozen_set)
{
- Assert(PageIsAllVisible(page));
- Assert(visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer));
-
/*
* It's fine to use InvalidTransactionId here - this is only used
* when HEAP_INSERT_FROZEN is specified, which intentionally
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index 5d48f071f53..cf843277938 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -295,7 +295,6 @@ heap_xlog_visible(XLogReaderState *record)
LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK);
reln = CreateFakeRelcacheEntry(rlocator);
- visibilitymap_pin(reln, blkno, &vmbuffer);
visibilitymap_set(reln, blkno, InvalidBuffer, lsn, vmbuffer,
xlrec->snapshotConflictHorizon, vmbits);
--
2.43.0
[text/x-patch] v9-0003-Eliminate-xl_heap_visible-in-COPY-FREEZE.patch (11.3K, 5-v9-0003-Eliminate-xl_heap_visible-in-COPY-FREEZE.patch)
download | inline diff:
From 7b6222f1670a0078c32383e64fb3782f555a6564 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 17 Jun 2025 17:22:10 -0400
Subject: [PATCH v9 03/22] Eliminate xl_heap_visible in COPY FREEZE
Instead of emitting a separate WAL record for setting the VM bits in
xl_heap_visible, specify the changes to make to the VM block in the
xl_heap_multi_insert record instead.
Author: Melanie Plageman <[email protected]>
Reviewed-by: Kirill Reshke <[email protected]>
Discussion: https://postgr.es/m/flat/CAAKRu_ZMw6Npd_qm2KM%2BFwQ3cMOMx1Dh3VMhp8-V7SOLxdK9-g%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 47 ++++++++++-------
src/backend/access/heap/heapam_xlog.c | 43 +++++++++++++++-
src/backend/access/heap/visibilitymap.c | 67 ++++++++++++++++++++++++-
src/backend/access/rmgrdesc/heapdesc.c | 5 ++
src/include/access/visibilitymap.h | 2 +
5 files changed, 144 insertions(+), 20 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 4c5ae205a7a..893a739009a 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2504,9 +2504,6 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
/*
* If the page is all visible, need to clear that, unless we're only
* going to add further frozen rows to it.
- *
- * If we're only adding already frozen rows to a previously empty
- * page, mark it as all-visible.
*/
if (PageIsAllVisible(page) && !(options & HEAP_INSERT_FROZEN))
{
@@ -2516,8 +2513,22 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
BufferGetBlockNumber(buffer),
vmbuffer, VISIBILITYMAP_VALID_BITS);
}
+
+ /*
+ * If we're only adding already frozen rows to a previously empty
+ * page, mark it as all-frozen and update the visibility map. We're
+ * already holding a pin on the vmbuffer.
+ */
else if (all_frozen_set)
+ {
PageSetAllVisible(page);
+ LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
+ visibilitymap_set_vmbits(relation,
+ BufferGetBlockNumber(buffer),
+ vmbuffer,
+ VISIBILITYMAP_ALL_VISIBLE |
+ VISIBILITYMAP_ALL_FROZEN);
+ }
/*
* XXX Should we set PageSetPrunable on this page ? See heap_insert()
@@ -2565,6 +2576,12 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
xlrec->flags = 0;
if (all_visible_cleared)
xlrec->flags = XLH_INSERT_ALL_VISIBLE_CLEARED;
+
+ /*
+ * We don't have to worry about including a conflict xid in the
+ * WAL record as HEAP_INSERT_FROZEN intentionally violates
+ * visibility rules.
+ */
if (all_frozen_set)
xlrec->flags = XLH_INSERT_ALL_FROZEN_SET;
@@ -2627,7 +2644,10 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
XLogBeginInsert();
XLogRegisterData(xlrec, tupledata - scratch.data);
+
XLogRegisterBuffer(0, buffer, REGBUF_STANDARD | bufflags);
+ if (all_frozen_set)
+ XLogRegisterBuffer(1, vmbuffer, 0);
XLogRegisterBufData(0, tupledata, totaldatalen);
@@ -2637,26 +2657,17 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
recptr = XLogInsert(RM_HEAP2_ID, info);
PageSetLSN(page, recptr);
+ if (all_frozen_set)
+ {
+ Assert(BufferIsDirty(vmbuffer));
+ PageSetLSN(BufferGetPage(vmbuffer), recptr);
+ }
}
END_CRIT_SECTION();
- /*
- * If we've frozen everything on the page, update the visibilitymap.
- * We're already holding pin on the vmbuffer.
- */
if (all_frozen_set)
- {
- /*
- * It's fine to use InvalidTransactionId here - this is only used
- * when HEAP_INSERT_FROZEN is specified, which intentionally
- * violates visibility rules.
- */
- visibilitymap_set(relation, BufferGetBlockNumber(buffer), buffer,
- InvalidXLogRecPtr, vmbuffer,
- InvalidTransactionId,
- VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
- }
+ LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK);
UnlockReleaseBuffer(buffer);
ndone += nthispage;
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index cf843277938..0820f7d052d 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -551,6 +551,7 @@ heap_xlog_multi_insert(XLogReaderState *record)
int i;
bool isinit = (XLogRecGetInfo(record) & XLOG_HEAP_INIT_PAGE) != 0;
XLogRedoAction action;
+ Buffer vmbuffer = InvalidBuffer;
/*
* Insertion doesn't overwrite MVCC data, so no conflict processing is
@@ -571,11 +572,11 @@ heap_xlog_multi_insert(XLogReaderState *record)
if (xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED)
{
Relation reln = CreateFakeRelcacheEntry(rlocator);
- Buffer vmbuffer = InvalidBuffer;
visibilitymap_pin(reln, blkno, &vmbuffer);
visibilitymap_clear(reln, blkno, vmbuffer, VISIBILITYMAP_VALID_BITS);
ReleaseBuffer(vmbuffer);
+ vmbuffer = InvalidBuffer;
FreeFakeRelcacheEntry(reln);
}
@@ -662,6 +663,46 @@ heap_xlog_multi_insert(XLogReaderState *record)
if (BufferIsValid(buffer))
UnlockReleaseBuffer(buffer);
+ buffer = InvalidBuffer;
+
+ /*
+ * Now read and update the VM block. Even if we skipped updating the heap
+ * page due to the file being dropped or truncated later in recovery, it's
+ * still safe to update the visibility map. Any WAL record that clears
+ * the visibility map bit does so before checking the page LSN, so any
+ * bits that need to be cleared will still be cleared.
+ *
+ * It is only okay to set the VM bits without holding the heap page lock
+ * because we can expect no other writers of this page.
+ */
+ if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET &&
+ XLogReadBufferForRedoExtended(record, 1, RBM_ZERO_ON_ERROR, false,
+ &vmbuffer) == BLK_NEEDS_REDO)
+ {
+ Page vmpage = BufferGetPage(vmbuffer);
+ Relation reln = CreateFakeRelcacheEntry(rlocator);
+
+ /* initialize the page if it was read as zeros */
+ if (PageIsNew(vmpage))
+ PageInit(vmpage, BLCKSZ, 0);
+
+ visibilitymap_set_vmbits(reln, blkno,
+ vmbuffer,
+ VISIBILITYMAP_ALL_VISIBLE |
+ VISIBILITYMAP_ALL_FROZEN);
+
+ /*
+ * It is not possible that the VM was already set for this heap page,
+ * so the vmbuffer must have been modified and marked dirty.
+ */
+ Assert(BufferIsDirty(vmbuffer));
+ PageSetLSN(BufferGetPage(vmbuffer), lsn);
+ FreeFakeRelcacheEntry(reln);
+ }
+
+ if (BufferIsValid(vmbuffer))
+ UnlockReleaseBuffer(vmbuffer);
+
/*
* If the page is running low on free space, update the FSM as well.
* Arbitrarily, our definition of "low" is less than 20%. We can't do much
diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index 7306c16f05c..bb8dfd8910a 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -14,7 +14,8 @@
* visibilitymap_clear - clear bits for one page in the visibility map
* visibilitymap_pin - pin a map page for setting a bit
* visibilitymap_pin_ok - check whether correct map page is already pinned
- * visibilitymap_set - set a bit in a previously pinned page
+ * visibilitymap_set - set bit(s) in a previously pinned page and log
+ * visibilitymap_set_vmbits - set bit(s) in a pinned page
* visibilitymap_get_status - get status of bits
* visibilitymap_count - count number of bits set in visibility map
* visibilitymap_prepare_truncate -
@@ -321,6 +322,70 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
return status;
}
+/*
+ * Set flags in the VM block contained in the passed in vmBuf.
+ *
+ * This function is for callers which include the VM changes in the same WAL
+ * record as the modifications of the heap page which rendered it all-visible.
+ * Callers separately logging the VM changes should invoke visibilitymap_set()
+ * instead.
+ *
+ * Caller must have pinned and exclusive locked the correct block of the VM in
+ * vmBuf. This block should contain the VM bits for the given heapBlk.
+ *
+ * During normal operation (i.e. not recovery), this should be called in a
+ * critical section which also makes any necessary changes to the heap page
+ * and, if relevant, emits WAL.
+ *
+ * Caller is responsible for WAL logging the changes to the VM buffer and for
+ * making any changes needed to the associated heap page. This includes
+ * maintaining any invariants such as ensuring the buffer containing heapBlk
+ * is pinned and exclusive locked.
+ */
+uint8
+visibilitymap_set_vmbits(Relation rel, BlockNumber heapBlk,
+ Buffer vmBuf, uint8 flags)
+{
+ BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk);
+ uint32 mapByte = HEAPBLK_TO_MAPBYTE(heapBlk);
+ uint8 mapOffset = HEAPBLK_TO_OFFSET(heapBlk);
+ Page page;
+ uint8 *map;
+ uint8 status;
+
+#ifdef TRACE_VISIBILITYMAP
+ elog(DEBUG1, "vm_set flags 0x%02X for %s %d",
+ flags, RelationGetRelationName(rel), heapBlk);
+#endif
+
+ /* Call in same critical section where WAL is emitted. */
+ Assert(InRecovery || CritSectionCount > 0);
+
+ /* Flags should be valid. Also never clear bits with this function */
+ Assert((flags & VISIBILITYMAP_VALID_BITS) == flags);
+
+ /* Must never set all_frozen bit without also setting all_visible bit */
+ Assert(flags != VISIBILITYMAP_ALL_FROZEN);
+
+ /* Check that we have the right VM page pinned */
+ if (!BufferIsValid(vmBuf) || BufferGetBlockNumber(vmBuf) != mapBlock)
+ elog(ERROR, "wrong VM buffer passed to visibilitymap_set");
+
+ Assert(BufferIsExclusiveLocked(vmBuf));
+
+ page = BufferGetPage(vmBuf);
+ map = (uint8 *) PageGetContents(page);
+
+ status = (map[mapByte] >> mapOffset) & VISIBILITYMAP_VALID_BITS;
+ if (flags != status)
+ {
+ map[mapByte] |= (flags << mapOffset);
+ MarkBufferDirty(vmBuf);
+ }
+
+ return status;
+}
+
/*
* visibilitymap_get_status - get status of bits
*
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 82b62c95de5..b48d7dc1d24 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -16,6 +16,7 @@
#include "access/heapam_xlog.h"
#include "access/rmgrdesc_utils.h"
+#include "access/visibilitymapdefs.h"
#include "storage/standbydefs.h"
/*
@@ -354,6 +355,10 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
appendStringInfo(buf, "ntuples: %d, flags: 0x%02X", xlrec->ntuples,
xlrec->flags);
+ if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET)
+ appendStringInfo(buf, ", vm_flags: 0x%02X",
+ VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
+
if (XLogRecHasBlockData(record, 0) && !isinit)
{
appendStringInfoString(buf, ", offsets:");
diff --git a/src/include/access/visibilitymap.h b/src/include/access/visibilitymap.h
index be21c6dd1a3..fc7056a91ea 100644
--- a/src/include/access/visibilitymap.h
+++ b/src/include/access/visibilitymap.h
@@ -37,6 +37,8 @@ extern uint8 visibilitymap_set(Relation rel,
Buffer vmBuf,
TransactionId cutoff_xid,
uint8 flags);
+extern uint8 visibilitymap_set_vmbits(Relation rel, BlockNumber heapBlk,
+ Buffer vmBuf, uint8 flags);
extern uint8 visibilitymap_get_status(Relation rel, BlockNumber heapBlk, Buffer *vmbuf);
extern void visibilitymap_count(Relation rel, BlockNumber *all_visible, BlockNumber *all_frozen);
extern BlockNumber visibilitymap_prepare_truncate(Relation rel,
--
2.43.0
[text/x-patch] v9-0004-Make-heap_page_is_all_visible-independent-of-LVRe.patch (5.4K, 6-v9-0004-Make-heap_page_is_all_visible-independent-of-LVRe.patch)
download | inline diff:
From abd46a0e574456401cb34380236673239c317361 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 18 Jun 2025 15:48:51 -0400
Subject: [PATCH v9 04/22] Make heap_page_is_all_visible independent of
LVRelState
Future commits will use this function inside of pruneheap.c where we do
not have access to the LVRelState. We only need a few parameters from
the LVRelState, so just pass those in explicitly.
Author: Melanie Plageman <[email protected]>
Reviewed-by: Kirill Reshke <[email protected]>
Discussion: https://postgr.es/m/flat/CAAKRu_ZMw6Npd_qm2KM%2BFwQ3cMOMx1Dh3VMhp8-V7SOLxdK9-g%40mail.gmail.com
---
src/backend/access/heap/vacuumlazy.c | 48 ++++++++++++++++++----------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 932701d8420..7f6f684bc63 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -463,8 +463,11 @@ static void dead_items_add(LVRelState *vacrel, BlockNumber blkno, OffsetNumber *
int num_offsets);
static void dead_items_reset(LVRelState *vacrel);
static void dead_items_cleanup(LVRelState *vacrel);
-static bool heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
- TransactionId *visibility_cutoff_xid, bool *all_frozen);
+static bool heap_page_is_all_visible(Relation rel, Buffer buf,
+ TransactionId OldestXmin,
+ bool *all_frozen,
+ TransactionId *visibility_cutoff_xid,
+ OffsetNumber *logging_offnum);
static void update_relstats_all_indexes(LVRelState *vacrel);
static void vacuum_error_callback(void *arg);
static void update_vacuum_error_info(LVRelState *vacrel,
@@ -2009,8 +2012,9 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(presult.lpdead_items == 0);
- if (!heap_page_is_all_visible(vacrel, buf,
- &debug_cutoff, &debug_all_frozen))
+ if (!heap_page_is_all_visible(vacrel->rel, buf,
+ vacrel->cutoffs.OldestXmin, &debug_all_frozen,
+ &debug_cutoff, &vacrel->offnum))
Assert(false);
Assert(presult.all_frozen == debug_all_frozen);
@@ -2906,8 +2910,8 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
* emitted.
*/
Assert(!PageIsAllVisible(page));
- if (heap_page_is_all_visible(vacrel, buffer, &visibility_cutoff_xid,
- &all_frozen))
+ if (heap_page_is_all_visible(vacrel->rel, buffer, vacrel->cutoffs.OldestXmin,
+ &all_frozen, &visibility_cutoff_xid, &vacrel->offnum))
{
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
@@ -3590,10 +3594,18 @@ dead_items_cleanup(LVRelState *vacrel)
}
/*
- * Check if every tuple in the given page is visible to all current and future
- * transactions. Also return the visibility_cutoff_xid which is the highest
- * xmin amongst the visible tuples. Set *all_frozen to true if every tuple
- * on this page is frozen.
+ * Check if every tuple in the given page in buf is visible to all current and
+ * future transactions.
+ *
+ * OldestXmin is used to determine visibility.
+ *
+ * Sets *all_frozen to true if every tuple on this page is frozen.
+ *
+ * Sets *visibility_cutoff_xid to the highest xmin amongst the visible tuples.
+ * It is only valid if the page is all-visible.
+ *
+ * *logging_offnum will have the OffsetNumber of the current tuple being
+ * processed for vacuum's error callback system.
*
* This is a stripped down version of lazy_scan_prune(). If you change
* anything here, make sure that everything stays in sync. Note that an
@@ -3601,9 +3613,11 @@ dead_items_cleanup(LVRelState *vacrel)
* introducing new side-effects here.
*/
static bool
-heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
+heap_page_is_all_visible(Relation rel, Buffer buf,
+ TransactionId OldestXmin,
+ bool *all_frozen,
TransactionId *visibility_cutoff_xid,
- bool *all_frozen)
+ OffsetNumber *logging_offnum)
{
Page page = BufferGetPage(buf);
BlockNumber blockno = BufferGetBlockNumber(buf);
@@ -3626,7 +3640,7 @@ heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
* Set the offset number so that we can display it along with any
* error that occurred while processing this tuple.
*/
- vacrel->offnum = offnum;
+ *logging_offnum = offnum;
itemid = PageGetItemId(page, offnum);
/* Unused or redirect line pointers are of no interest */
@@ -3650,9 +3664,9 @@ heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
tuple.t_data = (HeapTupleHeader) PageGetItem(page, itemid);
tuple.t_len = ItemIdGetLength(itemid);
- tuple.t_tableOid = RelationGetRelid(vacrel->rel);
+ tuple.t_tableOid = RelationGetRelid(rel);
- switch (HeapTupleSatisfiesVacuum(&tuple, vacrel->cutoffs.OldestXmin,
+ switch (HeapTupleSatisfiesVacuum(&tuple, OldestXmin,
buf))
{
case HEAPTUPLE_LIVE:
@@ -3673,7 +3687,7 @@ heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
*/
xmin = HeapTupleHeaderGetXmin(tuple.t_data);
if (!TransactionIdPrecedes(xmin,
- vacrel->cutoffs.OldestXmin))
+ OldestXmin))
{
all_visible = false;
*all_frozen = false;
@@ -3708,7 +3722,7 @@ heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
} /* scan along page */
/* Clear the offset information once we have processed the given page. */
- vacrel->offnum = InvalidOffsetNumber;
+ *logging_offnum = InvalidOffsetNumber;
return all_visible;
}
--
2.43.0
[text/x-patch] v9-0006-Use-xl_heap_prune-record-for-setting-empty-pages-.patch (5.8K, 7-v9-0006-Use-xl_heap_prune-record-for-setting-empty-pages-.patch)
download | inline diff:
From 15eb77d2b54d4856d6dd392c48cb68d6721d20ff Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 18 Jun 2025 12:42:19 -0400
Subject: [PATCH v9 06/22] Use xl_heap_prune record for setting empty pages
all-visible
As part of a project to eliminate xl_heap_visible records, eliminate
their usage in phase I vacuum of empty pages.
---
src/backend/access/heap/pruneheap.c | 14 +++++--
src/backend/access/heap/vacuumlazy.c | 55 ++++++++++++++++++----------
src/include/access/heapam.h | 1 +
3 files changed, 47 insertions(+), 23 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f0b33d1b696..373986b204a 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -836,6 +836,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
conflict_xid = prstate.latest_xid_removed;
log_heap_prune_and_freeze(relation, buffer,
+ false,
InvalidBuffer, 0, false,
conflict_xid,
true, reason,
@@ -2055,6 +2056,9 @@ heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
* case, vmbuffer should already have been updated and marked dirty and should
* still be pinned and locked.
*
+ * force_heap_fpi indicates that a full page image of the heap block should be
+ * forced.
+ *
* set_pd_all_vis indicates that we set PD_ALL_VISIBLE and thus should update
* the page LSN when checksums/wal_log_hints are enabled even if we did not
* prune or freeze tuples on the page.
@@ -2065,6 +2069,7 @@ heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
*/
void
log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ bool force_heap_fpi,
Buffer vmbuffer,
uint8 vmflags,
bool set_pd_all_vis,
@@ -2095,13 +2100,16 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
regbuf_flags = REGBUF_STANDARD;
+ if (force_heap_fpi)
+ regbuf_flags |= REGBUF_FORCE_IMAGE;
+
/*
* We can avoid an FPI if the only modification we are making to the heap
* page is to set PD_ALL_VISIBLE and checksums/wal_log_hints are disabled.
*/
- if (!do_prune &&
- nfrozen == 0 &&
- (!set_pd_all_vis || !XLogHintBitIsNeeded()))
+ else if (!do_prune &&
+ nfrozen == 0 &&
+ (!set_pd_all_vis || !XLogHintBitIsNeeded()))
regbuf_flags |= REGBUF_NO_IMAGE;
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a50652ca5a0..edd28123b7d 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1877,33 +1877,47 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
*/
if (!PageIsAllVisible(page))
{
+ uint8 new_vmbits = VISIBILITYMAP_ALL_VISIBLE |
+ VISIBILITYMAP_ALL_FROZEN;
+
START_CRIT_SECTION();
- /* mark buffer dirty before writing a WAL record */
+ PageSetAllVisible(page);
MarkBufferDirty(buf);
- /*
- * It's possible that another backend has extended the heap,
- * initialized the page, and then failed to WAL-log the page due
- * to an ERROR. Since heap extension is not WAL-logged, recovery
- * might try to replay our record setting the page all-visible and
- * find that the page isn't initialized, which will cause a PANIC.
- * To prevent that, check whether the page has been previously
- * WAL-logged, and if not, do that now.
- */
- if (RelationNeedsWAL(vacrel->rel) &&
- PageGetLSN(page) == InvalidXLogRecPtr)
- log_newpage_buffer(buf, true);
+ LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
+ visibilitymap_set_vmbits(vacrel->rel, blkno,
+ vmbuffer, new_vmbits);
+
+ if (RelationNeedsWAL(vacrel->rel))
+ {
+ /*
+ * It's possible that another backend has extended the heap,
+ * initialized the page, and then failed to WAL-log the page
+ * due to an ERROR. Since heap extension is not WAL-logged,
+ * recovery might try to replay our record setting the page
+ * all-visible and find that the page isn't initialized, which
+ * will cause a PANIC. To prevent that, if the page hasn't
+ * been previously WAL-logged, force a heap FPI.
+ */
+ log_heap_prune_and_freeze(vacrel->rel, buf,
+ PageGetLSN(page) == InvalidXLogRecPtr,
+ vmbuffer,
+ new_vmbits,
+ true,
+ InvalidTransactionId,
+ false, PRUNE_VACUUM_SCAN,
+ NULL, 0,
+ NULL, 0,
+ NULL, 0,
+ NULL, 0);
+ }
- PageSetAllVisible(page);
- visibilitymap_set(vacrel->rel, blkno, buf,
- InvalidXLogRecPtr,
- vmbuffer, InvalidTransactionId,
- VISIBILITYMAP_ALL_VISIBLE |
- VISIBILITYMAP_ALL_FROZEN);
END_CRIT_SECTION();
- /* Count the newly all-frozen pages for logging */
+ LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK);
+
+ /* Count the newly all-frozen pages for logging. */
vacrel->vm_new_visible_pages++;
vacrel->vm_new_visible_frozen_pages++;
}
@@ -2917,6 +2931,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
if (RelationNeedsWAL(vacrel->rel))
{
log_heap_prune_and_freeze(vacrel->rel, buffer,
+ false,
vmbuffer,
vmflags,
set_pd_all_vis,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 8b47295efa2..e7129a644a1 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -394,6 +394,7 @@ extern void heap_page_prune_execute(Buffer buffer, bool lp_truncate_only,
OffsetNumber *nowunused, int nunused);
extern void heap_get_root_tuples(Page page, OffsetNumber *root_offsets);
extern void log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ bool force_heap_fpi,
Buffer vmbuffer,
uint8 vmflags,
bool vm_modified_heap_page,
--
2.43.0
[text/x-patch] v9-0007-Combine-lazy_scan_prune-VM-corruption-cases.patch (7.1K, 8-v9-0007-Combine-lazy_scan_prune-VM-corruption-cases.patch)
download | inline diff:
From c711696d07304ca3130a56dd9b068779c74e5ec2 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 28 May 2025 16:04:03 -0400
Subject: [PATCH v9 07/22] Combine lazy_scan_prune VM corruption cases
lazy_scan_prune() updates the visibility map after phase I of heap
vacuuming. It also checks and fixes corruption in the VM. The corruption
cases where mixed in with the normal visibility map update cases.
Careful study of the ordering of the current logic reveals that the
corruption cases can be reordered and extracted into a separate
function. This should result in no additional overhead when compared to
previous execution.
This reordering makes it clear which cases are about corruption and
which cases are normal VM updates. Separating them also makes it
possible to combine the normal cases in a future commit. This will make
the logic easier to understand and allow for further separation of the
logic to allow updating the VM in the same record as pruning and
freezing in phase I.
---
src/backend/access/heap/vacuumlazy.c | 114 +++++++++++++++++----------
1 file changed, 73 insertions(+), 41 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index edd28123b7d..1474835c74b 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -430,6 +430,12 @@ static void find_next_unskippable_block(LVRelState *vacrel, bool *skipsallvis);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
bool sharelock, Buffer vmbuffer);
+static bool identify_and_fix_vm_corruption(Relation relation,
+ BlockNumber heap_blk,
+ Buffer heap_buffer, Page heap_page,
+ bool heap_blk_known_av,
+ int64 nlpdead_items,
+ Buffer vmbuffer);
static int lazy_scan_prune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
Buffer vmbuffer, bool all_visible_according_to_vm,
@@ -1932,6 +1938,66 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
return false;
}
+/*
+ * When updating the visibility map after phase I heap vacuuming, we take the
+ * opportunity to identify and fix any VM corruption.
+ *
+ * heap_blk_known_av is the visibility status of the heap page collected
+ * while finding the next unskippable block in heap_vac_scan_next_block().
+ */
+static bool
+identify_and_fix_vm_corruption(Relation relation,
+ BlockNumber heap_blk,
+ Buffer heap_buffer, Page heap_page,
+ bool heap_blk_known_av,
+ int64 nlpdead_items,
+ Buffer vmbuffer)
+{
+ /*
+ * 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 heap_vac_scan_next_block() was called, so we must recheck
+ * with buffer lock before concluding that the VM is corrupt.
+ */
+ if (heap_blk_known_av && !PageIsAllVisible(heap_page) &&
+ visibilitymap_get_status(relation, heap_blk, &vmbuffer) != 0)
+ {
+ elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u",
+ RelationGetRelationName(relation), heap_blk);
+ visibilitymap_clear(relation, heap_blk, vmbuffer,
+ VISIBILITYMAP_VALID_BITS);
+ return true;
+ }
+
+ /*
+ * 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.
+ */
+ if (nlpdead_items > 0 && PageIsAllVisible(heap_page))
+ {
+ elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
+ RelationGetRelationName(relation), heap_blk);
+ PageClearAllVisible(heap_page);
+ MarkBufferDirty(heap_buffer);
+ visibilitymap_clear(relation, heap_blk, vmbuffer,
+ VISIBILITYMAP_VALID_BITS);
+ return true;
+ }
+
+ return false;
+}
+
+
/* qsort comparator for sorting OffsetNumbers */
static int
cmpOffsetNumbers(const void *a, const void *b)
@@ -2078,9 +2144,14 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
- * all_frozen variables
+ * all_frozen variables. Start by looking for any VM corruption.
*/
- if (!all_visible_according_to_vm && presult.all_visible)
+ if (identify_and_fix_vm_corruption(vacrel->rel, blkno, buf, page,
+ all_visible_according_to_vm, presult.lpdead_items, vmbuffer))
+ {
+ /* Don't update the VM if we just cleared corruption in it */
+ }
+ else if (!all_visible_according_to_vm && presult.all_visible)
{
uint8 old_vmbits;
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
@@ -2132,45 +2203,6 @@ lazy_scan_prune(LVRelState *vacrel,
}
}
- /*
- * 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 heap_vac_scan_next_block() 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 (presult.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);
- 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
--
2.43.0
[text/x-patch] v9-0009-Find-and-fix-VM-corruption-in-heap_page_prune_and.patch (11.7K, 9-v9-0009-Find-and-fix-VM-corruption-in-heap_page_prune_and.patch)
download | inline diff:
From 1b86b5724fc3468457f1e2d5d57df4c708080164 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 28 May 2025 16:45:59 -0400
Subject: [PATCH v9 09/22] Find and fix VM corruption in
heap_page_prune_and_freeze
Future commits will update the VM in the same critical section and WAL
record as pruning and freezing. For ease of review, this commit makes
one step toward doing this. It moves the VM corruption handling case to
heap_page_prune_and_freeze().
---
src/backend/access/heap/pruneheap.c | 87 +++++++++++++++++++++++++++-
src/backend/access/heap/vacuumlazy.c | 77 +++---------------------
src/include/access/heapam.h | 4 ++
3 files changed, 96 insertions(+), 72 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 373986b204a..5c08a5d44c7 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -21,7 +21,7 @@
#include "access/transam.h"
#include "access/xlog.h"
#include "access/xloginsert.h"
-#include "access/visibilitymapdefs.h"
+#include "access/visibilitymap.h"
#include "commands/vacuum.h"
#include "executor/instrument.h"
#include "miscadmin.h"
@@ -177,6 +177,13 @@ static void heap_prune_record_unchanged_lp_redirect(PruneState *prstate, OffsetN
static void page_verify_redirects(Page page);
+static bool identify_and_fix_vm_corruption(Relation relation,
+ BlockNumber heap_blk,
+ Buffer heap_buffer, Page heap_page,
+ bool heap_blk_known_av,
+ int64 nlpdead_items,
+ Buffer vmbuffer);
+
/*
* Optionally prune and repair fragmentation in the specified page.
@@ -261,7 +268,9 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
* not the relation has indexes, since we cannot safely determine
* that during on-access pruning with the current implementation.
*/
- heap_page_prune_and_freeze(relation, buffer, vistest, 0,
+ heap_page_prune_and_freeze(relation, buffer, false,
+ InvalidBuffer,
+ vistest, 0,
NULL, &presult, PRUNE_ON_ACCESS, &dummy_off_loc, NULL, NULL);
/*
@@ -294,6 +303,64 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
}
}
+/*
+ * When updating the visibility map after phase I heap vacuuming, we take the
+ * opportunity to identify and fix any VM corruption.
+ *
+ * heap_blk_known_av is the visibility status of the heap page collected
+ * while finding the next unskippable block in heap_vac_scan_next_block().
+ */
+static bool
+identify_and_fix_vm_corruption(Relation relation,
+ BlockNumber heap_blk,
+ Buffer heap_buffer, Page heap_page,
+ bool heap_blk_known_av,
+ int64 nlpdead_items,
+ Buffer vmbuffer)
+{
+ /*
+ * 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 heap_vac_scan_next_block() was called, so we must recheck
+ * with buffer lock before concluding that the VM is corrupt.
+ */
+ if (heap_blk_known_av && !PageIsAllVisible(heap_page) &&
+ visibilitymap_get_status(relation, heap_blk, &vmbuffer) != 0)
+ {
+ elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u",
+ RelationGetRelationName(relation), heap_blk);
+ visibilitymap_clear(relation, heap_blk, vmbuffer,
+ VISIBILITYMAP_VALID_BITS);
+ return true;
+ }
+
+ /*
+ * 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.
+ */
+ if (nlpdead_items > 0 && PageIsAllVisible(heap_page))
+ {
+ elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
+ RelationGetRelationName(relation), heap_blk);
+ PageClearAllVisible(heap_page);
+ MarkBufferDirty(heap_buffer);
+ visibilitymap_clear(relation, heap_blk, vmbuffer,
+ VISIBILITYMAP_VALID_BITS);
+ return true;
+ }
+
+ return false;
+}
/*
* Prune and repair fragmentation and potentially freeze tuples on the
@@ -314,6 +381,10 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
* HEAP_PRUNE_FREEZE option is not set, because at the moment only callers
* that also freeze need that information.
*
+ * blk_known_av is the visibility status of the heap block as of the last call
+ * to find_next_unskippable_block(). vmbuffer is the buffer that may already
+ * contain the required block of the visibility map.
+ *
* vistest is used to distinguish whether tuples are DEAD or RECENTLY_DEAD
* (see heap_prune_satisfies_vacuum).
*
@@ -349,6 +420,8 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
*/
void
heap_page_prune_and_freeze(Relation relation, Buffer buffer,
+ bool blk_known_av,
+ Buffer vmbuffer,
GlobalVisState *vistest,
int options,
struct VacuumCutoffs *cutoffs,
@@ -897,6 +970,16 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->lpdead_items = prstate.lpdead_items;
/* the presult->deadoffsets array was already filled in */
+ /*
+ * Clear any VM corruption. This does not need to be done in a critical
+ * section.
+ */
+ presult->vm_corruption = false;
+ if (options & HEAP_PAGE_PRUNE_UPDATE_VM)
+ presult->vm_corruption = identify_and_fix_vm_corruption(relation,
+ blockno, buffer, page,
+ blk_known_av,
+ prstate.lpdead_items, vmbuffer);
if (prstate.freeze)
{
if (presult->nfrozen > 0)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index cbe37369790..d49c71bc1b5 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -430,12 +430,6 @@ static void find_next_unskippable_block(LVRelState *vacrel, bool *skipsallvis);
static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
bool sharelock, Buffer vmbuffer);
-static bool identify_and_fix_vm_corruption(Relation relation,
- BlockNumber heap_blk,
- Buffer heap_buffer, Page heap_page,
- bool heap_blk_known_av,
- int64 nlpdead_items,
- Buffer vmbuffer);
static int lazy_scan_prune(LVRelState *vacrel, Buffer buf,
BlockNumber blkno, Page page,
Buffer vmbuffer, bool all_visible_according_to_vm,
@@ -1938,65 +1932,6 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
return false;
}
-/*
- * When updating the visibility map after phase I heap vacuuming, we take the
- * opportunity to identify and fix any VM corruption.
- *
- * heap_blk_known_av is the visibility status of the heap page collected
- * while finding the next unskippable block in heap_vac_scan_next_block().
- */
-static bool
-identify_and_fix_vm_corruption(Relation relation,
- BlockNumber heap_blk,
- Buffer heap_buffer, Page heap_page,
- bool heap_blk_known_av,
- int64 nlpdead_items,
- Buffer vmbuffer)
-{
- /*
- * 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 heap_vac_scan_next_block() was called, so we must recheck
- * with buffer lock before concluding that the VM is corrupt.
- */
- if (heap_blk_known_av && !PageIsAllVisible(heap_page) &&
- visibilitymap_get_status(relation, heap_blk, &vmbuffer) != 0)
- {
- elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u",
- RelationGetRelationName(relation), heap_blk);
- visibilitymap_clear(relation, heap_blk, vmbuffer,
- VISIBILITYMAP_VALID_BITS);
- return true;
- }
-
- /*
- * 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.
- */
- if (nlpdead_items > 0 && PageIsAllVisible(heap_page))
- {
- elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
- RelationGetRelationName(relation), heap_blk);
- PageClearAllVisible(heap_page);
- MarkBufferDirty(heap_buffer);
- visibilitymap_clear(relation, heap_blk, vmbuffer,
- VISIBILITYMAP_VALID_BITS);
- return true;
- }
-
- return false;
-}
-
/* qsort comparator for sorting OffsetNumbers */
static int
@@ -2055,11 +1990,14 @@ lazy_scan_prune(LVRelState *vacrel,
* tuples. Pruning will have determined whether or not the page is
* all-visible.
*/
- prune_options = HEAP_PAGE_PRUNE_FREEZE;
+ prune_options = HEAP_PAGE_PRUNE_FREEZE | HEAP_PAGE_PRUNE_UPDATE_VM;
if (vacrel->nindexes == 0)
prune_options |= HEAP_PAGE_PRUNE_MARK_UNUSED_NOW;
- heap_page_prune_and_freeze(rel, buf, vacrel->vistest, prune_options,
+ heap_page_prune_and_freeze(rel, buf,
+ all_visible_according_to_vm,
+ vmbuffer,
+ vacrel->vistest, prune_options,
&vacrel->cutoffs, &presult, PRUNE_VACUUM_SCAN,
&vacrel->offnum,
&vacrel->NewRelfrozenXid, &vacrel->NewRelminMxid);
@@ -2144,10 +2082,9 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Handle setting visibility map bit based on information from the VM (as
* of last heap_vac_scan_next_block() call), and from all_visible and
- * all_frozen variables. Start by looking for any VM corruption.
+ * all_frozen variables.
*/
- if (identify_and_fix_vm_corruption(vacrel->rel, blkno, buf, page,
- all_visible_according_to_vm, presult.lpdead_items, vmbuffer))
+ if (presult.vm_corruption)
{
/* Don't update the VM if we just cleared corruption in it */
}
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index e7129a644a1..0c7eb5e46f4 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -42,6 +42,7 @@
/* "options" flag bits for heap_page_prune_and_freeze */
#define HEAP_PAGE_PRUNE_MARK_UNUSED_NOW (1 << 0)
#define HEAP_PAGE_PRUNE_FREEZE (1 << 1)
+#define HEAP_PAGE_PRUNE_UPDATE_VM (1 << 2)
typedef struct BulkInsertStateData *BulkInsertState;
struct TupleTableSlot;
@@ -247,6 +248,7 @@ typedef struct PruneFreezeResult
bool all_visible;
bool all_frozen;
TransactionId vm_conflict_horizon;
+ bool vm_corruption;
/*
* Whether or not the page makes rel truncation unsafe. This is set to
@@ -380,6 +382,8 @@ extern TransactionId heap_index_delete_tuples(Relation rel,
struct GlobalVisState;
extern void heap_page_prune_opt(Relation relation, Buffer buffer);
extern void heap_page_prune_and_freeze(Relation relation, Buffer buffer,
+ bool blk_known_av,
+ Buffer vmbuffer,
struct GlobalVisState *vistest,
int options,
struct VacuumCutoffs *cutoffs,
--
2.43.0
[text/x-patch] v9-0008-Combine-vacuum-phase-I-VM-update-cases.patch (5.8K, 10-v9-0008-Combine-vacuum-phase-I-VM-update-cases.patch)
download | inline diff:
From 50ca8c73a62531f8d1b30886551c492023ea9e47 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 28 May 2025 16:35:36 -0400
Subject: [PATCH v9 08/22] Combine vacuum phase I VM update cases
We update the VM after phase I of vacuum -- either setting both the VM
bits when all bits are currently unset or setting just the frozen bit
when the all-visible bit is already set.
Those two cases shared much of the same code -- leading to unnecessary
duplication. This commit combines them, which is simpler and easier to
understand.
The combined case also happens to fix a longstanding bug where if we are
only setting an all-visible page all-frozen and checksums/wal_log_hints
are enabled, we would fail to set the buffer dirty before setting the
page LSN in visibilitymap_set().
---
src/backend/access/heap/vacuumlazy.c | 101 +++++++++------------------
1 file changed, 32 insertions(+), 69 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 1474835c74b..cbe37369790 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2151,11 +2151,26 @@ lazy_scan_prune(LVRelState *vacrel,
{
/* Don't update the VM if we just cleared corruption in it */
}
- else if (!all_visible_according_to_vm && presult.all_visible)
+
+ /*
+ * If the page isn't yet marked all-visible in the VM or it is and needs
+ * to me marked all-frozen, update the VM Note that all_frozen is only
+ * valid if all_visible is true, so we must check both all_visible and
+ * all_frozen.
+ */
+ else if (presult.all_visible &&
+ (!all_visible_according_to_vm ||
+ (presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))))
{
uint8 old_vmbits;
uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
+ /*
+ * If the page is all-frozen, we can pass InvalidTransactionId as our
+ * cutoff_xid, since a snapshotConflictHorizon sufficient to make
+ * everything safe for REDO was logged when the page's tuples were
+ * frozen.
+ */
if (presult.all_frozen)
{
Assert(!TransactionIdIsValid(presult.vm_conflict_horizon));
@@ -2168,21 +2183,29 @@ lazy_scan_prune(LVRelState *vacrel,
* 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.
+ * 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.
*/
- PageSetAllVisible(page);
- MarkBufferDirty(buf);
+ if (!PageIsAllVisible(page) || XLogHintBitIsNeeded())
+ {
+ PageSetAllVisible(page);
+ MarkBufferDirty(buf);
+ }
+
old_vmbits = visibilitymap_set(vacrel->rel, blkno, buf,
InvalidXLogRecPtr,
vmbuffer, presult.vm_conflict_horizon,
flags);
/*
+ * Even if we are only setting the all-frozen bit, there is a small
+ * chance that the VM was modified sometime between setting
+ * all_visible_according_to_vm and checking the visibility during
+ * pruning. Check the return value of old_vmbits to ensure the
+ * visibility map counters used for logging are accurate.
+ *
* If the page wasn't already set all-visible and/or all-frozen in the
* VM, count it as newly set for logging.
*/
@@ -2203,66 +2226,6 @@ 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 all_visible and all_frozen.
- */
- else if (all_visible_according_to_vm && presult.all_visible &&
- presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
- {
- uint8 old_vmbits;
-
- /*
- * 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
- */
- if (!PageIsAllVisible(page))
- {
- PageSetAllVisible(page);
- MarkBufferDirty(buf);
- }
-
- /*
- * Set the page all-frozen (and all-visible) in the VM.
- *
- * We can pass InvalidTransactionId as our cutoff_xid, since a
- * snapshotConflictHorizon sufficient to make everything safe for REDO
- * was logged when the page's tuples were frozen.
- */
- Assert(!TransactionIdIsValid(presult.vm_conflict_horizon));
- old_vmbits = visibilitymap_set(vacrel->rel, blkno, buf,
- InvalidXLogRecPtr,
- vmbuffer, InvalidTransactionId,
- VISIBILITYMAP_ALL_VISIBLE |
- VISIBILITYMAP_ALL_FROZEN);
-
- /*
- * The page was likely already set all-visible in the VM. However,
- * there is a small chance that it was modified sometime between
- * setting all_visible_according_to_vm and checking the visibility
- * during pruning. Check the return value of old_vmbits anyway to
- * ensure the visibility map counters used for logging are accurate.
- */
- if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
- {
- vacrel->vm_new_visible_pages++;
- vacrel->vm_new_visible_frozen_pages++;
- *vm_page_frozen = true;
- }
-
- /*
- * We already checked that the page was not set all-frozen in the VM
- * above, so we don't need to test the value of old_vmbits.
- */
- else
- {
- vacrel->vm_new_frozen_pages++;
- *vm_page_frozen = true;
- }
- }
-
return presult.ndeleted;
}
--
2.43.0
[text/x-patch] v9-0010-Keep-all_frozen-updated-too-in-heap_page_prune_an.patch (3.1K, 11-v9-0010-Keep-all_frozen-updated-too-in-heap_page_prune_an.patch)
download | inline diff:
From c947f3564585049b4349216cbbc57c42aaea8aaf Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 23 Jul 2025 16:01:24 -0400
Subject: [PATCH v9 10/22] Keep all_frozen updated too in
heap_page_prune_and_freeze
We previously relied on only using all-visible and all-frozen together
but it's best to keep them both updated.
Future commits will separate usage of these fields, so it is best not to
rely on all_visible for all_frozen's validity.
---
src/backend/access/heap/pruneheap.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 5c08a5d44c7..18eab8d0518 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -143,10 +143,6 @@ typedef struct
* whether to freeze the page or not. The all_visible and all_frozen
* values returned to the caller are adjusted to include LP_DEAD items at
* the end.
- *
- * all_frozen should only be considered valid if all_visible is also set;
- * we don't bother to clear the all_frozen flag every time we clear the
- * all_visible flag.
*/
bool all_visible;
bool all_frozen;
@@ -824,6 +820,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
}
+ Assert(!prstate.all_frozen || prstate.all_visible);
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -1468,7 +1465,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
if (!HeapTupleHeaderXminCommitted(htup))
{
- prstate->all_visible = false;
+ prstate->all_visible = prstate->all_frozen = false;
break;
}
@@ -1490,7 +1487,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
Assert(prstate->cutoffs);
if (!TransactionIdPrecedes(xmin, prstate->cutoffs->OldestXmin))
{
- prstate->all_visible = false;
+ prstate->all_visible = prstate->all_frozen = false;
break;
}
@@ -1503,7 +1500,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
case HEAPTUPLE_RECENTLY_DEAD:
prstate->recently_dead_tuples++;
- prstate->all_visible = false;
+ prstate->all_visible = prstate->all_frozen = false;
/*
* This tuple will soon become DEAD. Update the hint field so
@@ -1522,7 +1519,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
* assumption is a bit shaky, but it is what acquire_sample_rows()
* does, so be consistent.
*/
- prstate->all_visible = false;
+ prstate->all_visible = prstate->all_frozen = false;
/*
* If we wanted to optimize for aborts, we might consider marking
@@ -1540,7 +1537,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
* will commit and update the counters after we report.
*/
prstate->live_tuples++;
- prstate->all_visible = false;
+ prstate->all_visible = prstate->all_frozen = false;
/*
* This tuple may soon become DEAD. Update the hint field so that
--
2.43.0
[text/x-patch] v9-0011-Update-VM-in-pruneheap.c.patch (12.7K, 12-v9-0011-Update-VM-in-pruneheap.c.patch)
download | inline diff:
From fe909609c0d76b835430169a2b7579b0177ca2d1 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 2 Jun 2025 11:04:14 -0400
Subject: [PATCH v9 11/22] Update VM in pruneheap.c
As a step toward updating the VM in the same critical section and WAL
record as pruning and freezing (during phase I of vacuuming), first move
the VM update (still in its own critical section and WAL record) into
heap_page_prune_and_freeze(). This makes review easier.
---
src/backend/access/heap/pruneheap.c | 99 +++++++++++++++++++++++-----
src/backend/access/heap/vacuumlazy.c | 99 +++++-----------------------
src/include/access/heapam.h | 15 +++--
3 files changed, 106 insertions(+), 107 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 18eab8d0518..3483b5caff3 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -360,7 +360,8 @@ identify_and_fix_vm_corruption(Relation relation,
/*
* Prune and repair fragmentation and potentially freeze tuples on the
- * specified page.
+ * specified page. If the page's visibility status has changed, update it in
+ * the VM.
*
* Caller must have pin and buffer cleanup lock on the page. Note that we
* don't update the FSM information for page on caller's behalf. Caller might
@@ -436,6 +437,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
bool do_freeze;
bool do_prune;
bool do_hint;
+ uint8 vmflags = 0;
+ uint8 old_vmbits = 0;
bool hint_bit_fpi;
int64 fpi_before = pgWalUsage.wal_fpi;
@@ -936,7 +939,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*
* Now that freezing has been finalized, unset all_visible if there are
* any LP_DEAD items on the page. It needs to reflect the present state
- * of the page, as expected by our caller.
+ * of the page, as expected for updating the visibility map.
*/
if (prstate.all_visible && prstate.lpdead_items == 0)
{
@@ -952,31 +955,91 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->hastup = prstate.hastup;
/*
- * For callers planning to update the visibility map, the conflict horizon
- * for that record must be the newest xmin on the page. However, if the
- * page is completely frozen, there can be no conflict and the
- * vm_conflict_horizon should remain InvalidTransactionId. This includes
- * the case that we just froze all the tuples; the prune-freeze record
- * included the conflict XID already so the caller doesn't need it.
+ * If updating the visibility map, the conflict horizon for that record
+ * must be the newest xmin on the page. However, if the page is
+ * completely frozen, there can be no conflict and the vm_conflict_horizon
+ * should remain InvalidTransactionId. This includes the case that we
+ * just froze all the tuples; the prune-freeze record included the
+ * conflict XID already so the VM update record doesn't need it.
*/
if (presult->all_frozen)
presult->vm_conflict_horizon = InvalidTransactionId;
else
presult->vm_conflict_horizon = prstate.visibility_cutoff_xid;
- presult->lpdead_items = prstate.lpdead_items;
- /* the presult->deadoffsets array was already filled in */
-
/*
- * Clear any VM corruption. This does not need to be done in a critical
- * section.
+ * Handle setting visibility map bit based on information from the VM (as
+ * of last heap_vac_scan_next_block() call), and from all_visible and
+ * all_frozen variables.
*/
- presult->vm_corruption = false;
if (options & HEAP_PAGE_PRUNE_UPDATE_VM)
- presult->vm_corruption = identify_and_fix_vm_corruption(relation,
- blockno, buffer, page,
- blk_known_av,
- prstate.lpdead_items, vmbuffer);
+ {
+ if (identify_and_fix_vm_corruption(relation,
+ blockno, buffer, page,
+ blk_known_av,
+ prstate.lpdead_items, vmbuffer))
+ {
+ /* If we fix corruption, don't update the VM further */
+ }
+
+ /*
+ * If the page isn't yet marked all-visible in the VM or it is and
+ * needs to me marked all-frozen, update the VM Note that all_frozen
+ * is only valid if all_visible is true, so we must check both
+ * all_visible and all_frozen.
+ */
+ else if (presult->all_visible &&
+ (!blk_known_av ||
+ (presult->all_frozen && !VM_ALL_FROZEN(relation, blockno, &vmbuffer))))
+ {
+ Assert(prstate.lpdead_items == 0);
+ vmflags = VISIBILITYMAP_ALL_VISIBLE;
+
+ /*
+ * If the page is all-frozen, we can pass InvalidTransactionId as
+ * our cutoff_xid, since a snapshotConflictHorizon sufficient to
+ * make everything safe for REDO was logged when the page's tuples
+ * were frozen.
+ */
+ if (presult->all_frozen)
+ {
+ Assert(!TransactionIdIsValid(presult->vm_conflict_horizon));
+ vmflags |= VISIBILITYMAP_ALL_FROZEN;
+ }
+
+ /*
+ * It's possible for the VM bit to be clear and the page-level bit
+ * to be set if checksums are not enabled.
+ *
+ * And even if we are just planning to update the frozen bit in
+ * the VM, we shouldn't rely 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.
+ *
+ * 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.
+ */
+ if (!PageIsAllVisible(page) || XLogHintBitIsNeeded())
+ {
+ PageSetAllVisible(page);
+ MarkBufferDirty(buffer);
+ }
+
+ old_vmbits = visibilitymap_set(relation, blockno, buffer, InvalidXLogRecPtr,
+ vmbuffer, presult->vm_conflict_horizon,
+ vmflags);
+ }
+ }
+
+ presult->lpdead_items = prstate.lpdead_items;
+ /* the presult->deadoffsets array was already filled in */
+
+ presult->old_vmbits = old_vmbits;
+ presult->new_vmbits = vmflags;
+
if (prstate.freeze)
{
if (presult->nfrozen > 0)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d49c71bc1b5..05d3d2a3267 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1932,7 +1932,6 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
return false;
}
-
/* qsort comparator for sorting OffsetNumbers */
static int
cmpOffsetNumbers(const void *a, const void *b)
@@ -1948,7 +1947,8 @@ cmpOffsetNumbers(const void *a, const void *b)
* vmbuffer is the buffer containing the VM block with visibility information
* for the heap block, blkno. all_visible_according_to_vm is the saved
* visibility status of the heap block looked up earlier by the caller. We
- * won't rely entirely on this status, as it may be out of date.
+ * won't rely entirely on this status, as it may be out of date. These will be
+ * passed on to heap_page_prune_and_freeze() to use while setting the VM.
*
* *has_lpdead_items is set to true or false depending on whether, upon return
* from this function, any LP_DEAD items are still present on the page.
@@ -1977,6 +1977,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Prune all HOT-update chains and potentially freeze tuples on this page.
+ * Then, if the page's visibility status has changed, update the VM.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED.
@@ -1985,10 +1986,6 @@ lazy_scan_prune(LVRelState *vacrel,
* presult.ndeleted. It should not be confused with presult.lpdead_items;
* presult.lpdead_items's final value can be thought of as the number of
* tuples that were deleted from indexes.
- *
- * We will update the VM after collecting LP_DEAD items and freezing
- * tuples. Pruning will have determined whether or not the page is
- * all-visible.
*/
prune_options = HEAP_PAGE_PRUNE_FREEZE | HEAP_PAGE_PRUNE_UPDATE_VM;
if (vacrel->nindexes == 0)
@@ -2080,88 +2077,26 @@ lazy_scan_prune(LVRelState *vacrel,
Assert(!presult.all_visible || !(*has_lpdead_items));
/*
- * Handle setting visibility map bit based on information from the VM (as
- * of last heap_vac_scan_next_block() call), and from all_visible and
- * all_frozen variables.
+ * For the purposes of logging, count whether or not the page was newly
+ * set all-visible and, potentially, all-frozen.
*/
- if (presult.vm_corruption)
+ if ((presult.old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0 &&
+ (presult.new_vmbits & VISIBILITYMAP_ALL_VISIBLE) != 0)
{
- /* Don't update the VM if we just cleared corruption in it */
- }
-
- /*
- * If the page isn't yet marked all-visible in the VM or it is and needs
- * to me marked all-frozen, update the VM Note that all_frozen is only
- * valid if all_visible is true, so we must check both all_visible and
- * all_frozen.
- */
- else if (presult.all_visible &&
- (!all_visible_according_to_vm ||
- (presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))))
- {
- uint8 old_vmbits;
- uint8 flags = VISIBILITYMAP_ALL_VISIBLE;
-
- /*
- * If the page is all-frozen, we can pass InvalidTransactionId as our
- * cutoff_xid, since a snapshotConflictHorizon sufficient to make
- * everything safe for REDO was logged when the page's tuples were
- * frozen.
- */
- if (presult.all_frozen)
- {
- Assert(!TransactionIdIsValid(presult.vm_conflict_horizon));
- 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.
- *
- * 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.
- */
- if (!PageIsAllVisible(page) || XLogHintBitIsNeeded())
- {
- PageSetAllVisible(page);
- MarkBufferDirty(buf);
- }
-
- old_vmbits = visibilitymap_set(vacrel->rel, blkno, buf,
- InvalidXLogRecPtr,
- vmbuffer, presult.vm_conflict_horizon,
- flags);
-
- /*
- * Even if we are only setting the all-frozen bit, there is a small
- * chance that the VM was modified sometime between setting
- * all_visible_according_to_vm and checking the visibility during
- * pruning. Check the return value of old_vmbits to ensure the
- * visibility map counters used for logging are accurate.
- *
- * If the page wasn't already set all-visible and/or all-frozen in the
- * VM, count it as newly set for logging.
- */
- if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
- {
- vacrel->vm_new_visible_pages++;
- if (presult.all_frozen)
- {
- vacrel->vm_new_visible_frozen_pages++;
- *vm_page_frozen = true;
- }
- }
- else if ((old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0 &&
- presult.all_frozen)
+ vacrel->vm_new_visible_pages++;
+ if ((presult.new_vmbits & VISIBILITYMAP_ALL_FROZEN) != 0)
{
- vacrel->vm_new_frozen_pages++;
+ vacrel->vm_new_visible_frozen_pages++;
*vm_page_frozen = true;
}
}
+ else if ((presult.old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0 &&
+ (presult.new_vmbits & VISIBILITYMAP_ALL_FROZEN) != 0)
+ {
+ Assert((presult.new_vmbits & VISIBILITYMAP_ALL_VISIBLE) != 0);
+ vacrel->vm_new_frozen_pages++;
+ *vm_page_frozen = true;
+ }
return presult.ndeleted;
}
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 0c7eb5e46f4..b85648456e9 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -235,20 +235,21 @@ typedef struct PruneFreezeResult
int recently_dead_tuples;
/*
- * all_visible and all_frozen indicate if the all-visible and all-frozen
- * bits in the visibility map can be set for this page, after pruning.
+ * all_visible and all_frozen indicate the status of the page as reflected
+ * in the visibility map after pruning, freezing, and setting any pages
+ * all-visible in the visibility map.
*
- * vm_conflict_horizon is the newest xmin of live tuples on the page. The
- * caller can use it as the conflict horizon when setting the VM bits. It
- * is only valid if we froze some tuples (nfrozen > 0), and all_frozen is
- * true.
+ * vm_conflict_horizon is the newest xmin of live tuples on the page
+ * (older than OldestXmin). It will only be valid if we did not set the
+ * page all-frozen in the VM.
*
* These are only set if the HEAP_PRUNE_FREEZE option is set.
*/
bool all_visible;
bool all_frozen;
TransactionId vm_conflict_horizon;
- bool vm_corruption;
+ uint8 old_vmbits;
+ uint8 new_vmbits;
/*
* Whether or not the page makes rel truncation unsafe. This is set to
--
2.43.0
[text/x-patch] v9-0013-Rename-PruneState.freeze-to-attempt_freeze.patch (4.1K, 13-v9-0013-Rename-PruneState.freeze-to-attempt_freeze.patch)
download | inline diff:
From 96013b0fbfd3bf63d2940549e51317a89ee73b4e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 31 Jul 2025 14:07:51 -0400
Subject: [PATCH v9 13/22] Rename PruneState.freeze to attempt_freeze
This makes it more clear that this is to indicate the caller would like
heap_page_prune_and_freeze() to consider freezing tuples -- not that we
ultimately will end up freezing them.
Also rename local variable hint_bit_fpi to did_tuple_hint_fpi. This
makes it clear it is about tuple hints and not page hints and that it
indicates something that happened and not something that could happen.
---
src/backend/access/heap/pruneheap.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 683c1762c25..669c088ccff 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -43,7 +43,7 @@ typedef struct
/* whether or not dead items can be set LP_UNUSED during pruning */
bool mark_unused_now;
/* whether to attempt freezing tuples */
- bool freeze;
+ bool attempt_freeze;
/*
* Whether or not to consider updating the VM. There is some bookkeeping
@@ -452,7 +452,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
bool do_set_vm;
uint8 vmflags = 0;
uint8 old_vmbits = 0;
- bool hint_bit_fpi;
+ bool did_tuple_hint_fpi;
int64 fpi_before = pgWalUsage.wal_fpi;
bool all_frozen_except_lp_dead = false;
bool set_pd_all_visible = false;
@@ -460,7 +460,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/* Copy parameters to prstate */
prstate.vistest = vistest;
prstate.mark_unused_now = (options & HEAP_PAGE_PRUNE_MARK_UNUSED_NOW) != 0;
- prstate.freeze = (options & HEAP_PAGE_PRUNE_FREEZE) != 0;
+ prstate.attempt_freeze = (options & HEAP_PAGE_PRUNE_FREEZE) != 0;
prstate.consider_update_vm = (options & HEAP_PAGE_PRUNE_UPDATE_VM) != 0;
prstate.cutoffs = cutoffs;
@@ -485,7 +485,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/* initialize page freezing working state */
prstate.pagefrz.freeze_required = false;
- if (prstate.freeze)
+ if (prstate.attempt_freeze)
{
Assert(new_relfrozen_xid && new_relmin_mxid);
prstate.pagefrz.FreezePageRelfrozenXid = *new_relfrozen_xid;
@@ -535,7 +535,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* bookkeeping. Initializing all_visible to false allows skipping the work
* to update them in heap_prune_record_unchanged_lp_normal().
*/
- if (prstate.freeze)
+ if (prstate.attempt_freeze)
{
prstate.all_visible = true;
prstate.all_frozen = true;
@@ -653,7 +653,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* If checksums are enabled, heap_prune_satisfies_vacuum() may have caused
* an FPI to be emitted.
*/
- hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
+ did_tuple_hint_fpi = fpi_before != pgWalUsage.wal_fpi;
/*
* Process HOT chains.
@@ -770,7 +770,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* plans we prepared, or not.
*/
do_freeze = false;
- if (prstate.freeze)
+ if (prstate.attempt_freeze)
{
if (prstate.pagefrz.freeze_required)
{
@@ -803,7 +803,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
if (RelationNeedsWAL(relation))
{
- if (hint_bit_fpi)
+ if (did_tuple_hint_fpi)
do_freeze = true;
else if (do_prune)
{
@@ -1128,7 +1128,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->lpdead_items = prstate.lpdead_items;
/* the presult->deadoffsets array was already filled in */
- if (prstate.freeze)
+ if (prstate.attempt_freeze)
{
if (presult->nfrozen > 0)
{
@@ -1715,7 +1715,7 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
* to update the VM, we have to call heap_prepare_freeze_tuple() on every
* tuple to know whether or not the page will be totally frozen.
*/
- if (prstate->freeze)
+ if (prstate->attempt_freeze)
{
bool totally_frozen;
--
2.43.0
[text/x-patch] v9-0014-Remove-xl_heap_visible-entirely.patch (24.1K, 14-v9-0014-Remove-xl_heap_visible-entirely.patch)
download | inline diff:
From faf936042bbe225175e8bc6474d3617e70cb215d Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 18 Jun 2025 12:30:42 -0400
Subject: [PATCH v9 14/22] Remove xl_heap_visible entirely
There are now no users of this, so eliminate it entirely.
---
src/backend/access/common/bufmask.c | 3 +-
src/backend/access/heap/heapam.c | 54 +-------
src/backend/access/heap/heapam_xlog.c | 152 ++---------------------
src/backend/access/heap/pruneheap.c | 4 +-
src/backend/access/heap/vacuumlazy.c | 10 +-
src/backend/access/heap/visibilitymap.c | 109 +---------------
src/backend/access/rmgrdesc/heapdesc.c | 10 --
src/backend/replication/logical/decode.c | 1 -
src/backend/storage/ipc/standby.c | 12 +-
src/include/access/heapam_xlog.h | 20 ---
src/include/access/visibilitymap.h | 11 +-
src/include/access/visibilitymapdefs.h | 9 --
src/tools/pgindent/typedefs.list | 1 -
13 files changed, 32 insertions(+), 364 deletions(-)
diff --git a/src/backend/access/common/bufmask.c b/src/backend/access/common/bufmask.c
index bb260cffa68..1fff01383b3 100644
--- a/src/backend/access/common/bufmask.c
+++ b/src/backend/access/common/bufmask.c
@@ -56,8 +56,7 @@ mask_page_hint_bits(Page page)
/*
* During replay, if the page LSN has advanced past our XLOG record's LSN,
- * we don't mark the page all-visible. See heap_xlog_visible() for
- * details.
+ * we don't mark the page all-visible.
*/
PageClearAllVisible(page);
}
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 893a739009a..cb16bb0cbbd 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -40,6 +40,7 @@
#include "access/valid.h"
#include "access/visibilitymap.h"
#include "access/xloginsert.h"
+#include "access/xlogutils.h"
#include "catalog/pg_database.h"
#include "catalog/pg_database_d.h"
#include "commands/vacuum.h"
@@ -2523,11 +2524,11 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
{
PageSetAllVisible(page);
LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
- visibilitymap_set_vmbits(relation,
- BufferGetBlockNumber(buffer),
- vmbuffer,
- VISIBILITYMAP_ALL_VISIBLE |
- VISIBILITYMAP_ALL_FROZEN);
+ visibilitymap_set(relation,
+ BufferGetBlockNumber(buffer),
+ vmbuffer,
+ VISIBILITYMAP_ALL_VISIBLE |
+ VISIBILITYMAP_ALL_FROZEN);
}
/*
@@ -8798,49 +8799,6 @@ bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate)
return nblocksfavorable;
}
-/*
- * Perform XLogInsert for a heap-visible operation. 'block' is the block
- * being marked all-visible, and vm_buffer is the buffer containing the
- * corresponding visibility map block. Both should have already been modified
- * and dirtied.
- *
- * snapshotConflictHorizon comes from the largest xmin on the page being
- * marked all-visible. REDO routine uses it to generate recovery conflicts.
- *
- * If checksums or wal_log_hints are enabled, we may also generate a full-page
- * image of heap_buffer. Otherwise, we optimize away the FPI (by specifying
- * REGBUF_NO_IMAGE for the heap buffer), in which case the caller should *not*
- * update the heap page's LSN.
- */
-XLogRecPtr
-log_heap_visible(Relation rel, Buffer heap_buffer, Buffer vm_buffer,
- TransactionId snapshotConflictHorizon, uint8 vmflags)
-{
- xl_heap_visible xlrec;
- XLogRecPtr recptr;
- uint8 flags;
-
- Assert(BufferIsValid(heap_buffer));
- Assert(BufferIsValid(vm_buffer));
-
- xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
- xlrec.flags = vmflags;
- if (RelationIsAccessibleInLogicalDecoding(rel))
- xlrec.flags |= VISIBILITYMAP_XLOG_CATALOG_REL;
- XLogBeginInsert();
- XLogRegisterData(&xlrec, SizeOfHeapVisible);
-
- XLogRegisterBuffer(0, vm_buffer, 0);
-
- flags = REGBUF_STANDARD;
- if (!XLogHintBitIsNeeded())
- flags |= REGBUF_NO_IMAGE;
- XLogRegisterBuffer(1, heap_buffer, flags);
-
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_VISIBLE);
-
- return recptr;
-}
/*
* Perform XLogInsert for a heap-update operation. Caller must already
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index 11c11929ed9..ff3ad8b4cd2 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -53,6 +53,8 @@ heap_xlog_prune_freeze(XLogReaderState *record)
(xlrec.flags & (XLHP_HAS_REDIRECTIONS | XLHP_HAS_DEAD_ITEMS)) == 0);
vmflags = xlrec.flags & VISIBILITYMAP_VALID_BITS;
+ /* Must never set all_frozen bit without also setting all_visible bit */
+ Assert(vmflags != VISIBILITYMAP_ALL_FROZEN);
/*
* After xl_heap_prune is the optional snapshot conflict horizon.
@@ -250,7 +252,7 @@ heap_xlog_prune_freeze(XLogReaderState *record)
if (PageIsNew(vmpage))
PageInit(vmpage, BLCKSZ, 0);
- old_vmbits = visibilitymap_set_vmbits(reln, blkno, vmbuffer, vmflags);
+ old_vmbits = visibilitymap_set(reln, blkno, vmbuffer, vmflags);
/* Only set VM page LSN if we modified the page */
if (old_vmbits != vmflags)
@@ -269,142 +271,6 @@ heap_xlog_prune_freeze(XLogReaderState *record)
XLogRecordPageWithFreeSpace(rlocator, blkno, freespace);
}
-/*
- * Replay XLOG_HEAP2_VISIBLE records.
- *
- * The critical integrity requirement here is that we must never end up with
- * a situation where the visibility map bit is set, and the page-level
- * PD_ALL_VISIBLE bit is clear. If that were to occur, then a subsequent
- * page modification would fail to clear the visibility map bit.
- */
-static void
-heap_xlog_visible(XLogReaderState *record)
-{
- XLogRecPtr lsn = record->EndRecPtr;
- xl_heap_visible *xlrec = (xl_heap_visible *) XLogRecGetData(record);
- Buffer vmbuffer = InvalidBuffer;
- Buffer buffer;
- Page page;
- RelFileLocator rlocator;
- BlockNumber blkno;
- XLogRedoAction action;
-
- Assert((xlrec->flags & VISIBILITYMAP_XLOG_VALID_BITS) == xlrec->flags);
-
- XLogRecGetBlockTag(record, 1, &rlocator, NULL, &blkno);
-
- /*
- * If there are any Hot Standby transactions running that have an xmin
- * horizon old enough that this page isn't all-visible for them, they
- * might incorrectly decide that an index-only scan can skip a heap fetch.
- *
- * NB: It might be better to throw some kind of "soft" conflict here that
- * forces any index-only scan that is in flight to perform heap fetches,
- * rather than killing the transaction outright.
- */
- if (InHotStandby)
- ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
- xlrec->flags & VISIBILITYMAP_XLOG_CATALOG_REL,
- rlocator);
-
- /*
- * Read the heap page, if it still exists. If the heap file has dropped or
- * truncated later in recovery, we don't need to update the page, but we'd
- * better still update the visibility map.
- */
- action = XLogReadBufferForRedo(record, 1, &buffer);
- if (action == BLK_NEEDS_REDO)
- {
- /*
- * We don't bump the LSN of the heap page when setting the visibility
- * map bit (unless checksums or wal_hint_bits is enabled, in which
- * case we must). This exposes us to torn page hazards, but since
- * we're not inspecting the existing page contents in any way, we
- * don't care.
- */
- page = BufferGetPage(buffer);
-
- PageSetAllVisible(page);
-
- if (XLogHintBitIsNeeded())
- PageSetLSN(page, lsn);
-
- MarkBufferDirty(buffer);
- }
- else if (action == BLK_RESTORED)
- {
- /*
- * If heap block was backed up, we already restored it and there's
- * nothing more to do. (This can only happen with checksums or
- * wal_log_hints enabled.)
- */
- }
-
- if (BufferIsValid(buffer))
- {
- Size space = PageGetFreeSpace(BufferGetPage(buffer));
-
- UnlockReleaseBuffer(buffer);
-
- /*
- * Since FSM is not WAL-logged and only updated heuristically, it
- * easily becomes stale in standbys. If the standby is later promoted
- * and runs VACUUM, it will skip updating individual free space
- * figures for pages that became all-visible (or all-frozen, depending
- * on the vacuum mode,) which is troublesome when FreeSpaceMapVacuum
- * propagates too optimistic free space values to upper FSM layers;
- * later inserters try to use such pages only to find out that they
- * are unusable. This can cause long stalls when there are many such
- * pages.
- *
- * Forestall those problems by updating FSM's idea about a page that
- * is becoming all-visible or all-frozen.
- *
- * Do this regardless of a full-page image being applied, since the
- * FSM data is not in the page anyway.
- */
- if (xlrec->flags & VISIBILITYMAP_VALID_BITS)
- XLogRecordPageWithFreeSpace(rlocator, blkno, space);
- }
-
- /*
- * Even if we skipped the heap page update due to the LSN interlock, it's
- * still safe to update the visibility map. Any WAL record that clears
- * the visibility map bit does so before checking the page LSN, so any
- * bits that need to be cleared will still be cleared.
- */
- if (XLogReadBufferForRedoExtended(record, 0, RBM_ZERO_ON_ERROR, false,
- &vmbuffer) == BLK_NEEDS_REDO)
- {
- Page vmpage = BufferGetPage(vmbuffer);
- Relation reln;
- uint8 vmbits;
-
- /* initialize the page if it was read as zeros */
- if (PageIsNew(vmpage))
- PageInit(vmpage, BLCKSZ, 0);
-
- /* remove VISIBILITYMAP_XLOG_* */
- vmbits = xlrec->flags & VISIBILITYMAP_VALID_BITS;
-
- /*
- * XLogReadBufferForRedoExtended locked the buffer. But
- * visibilitymap_set will handle locking itself.
- */
- LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK);
-
- reln = CreateFakeRelcacheEntry(rlocator);
-
- visibilitymap_set(reln, blkno, InvalidBuffer, lsn, vmbuffer,
- xlrec->snapshotConflictHorizon, vmbits);
-
- ReleaseBuffer(vmbuffer);
- FreeFakeRelcacheEntry(reln);
- }
- else if (BufferIsValid(vmbuffer))
- UnlockReleaseBuffer(vmbuffer);
-}
-
/*
* Given an "infobits" field from an XLog record, set the correct bits in the
* given infomask and infomask2 for the tuple touched by the record.
@@ -785,15 +651,14 @@ heap_xlog_multi_insert(XLogReaderState *record)
if (PageIsNew(vmpage))
PageInit(vmpage, BLCKSZ, 0);
- visibilitymap_set_vmbits(reln, blkno,
- vmbuffer,
- VISIBILITYMAP_ALL_VISIBLE |
- VISIBILITYMAP_ALL_FROZEN);
-
/*
* It is not possible that the VM was already set for this heap page,
* so the vmbuffer must have been modified and marked dirty.
*/
+ visibilitymap_set(reln, blkno,
+ vmbuffer,
+ VISIBILITYMAP_ALL_VISIBLE |
+ VISIBILITYMAP_ALL_FROZEN);
Assert(BufferIsDirty(vmbuffer));
PageSetLSN(BufferGetPage(vmbuffer), lsn);
FreeFakeRelcacheEntry(reln);
@@ -1374,9 +1239,6 @@ heap2_redo(XLogReaderState *record)
case XLOG_HEAP2_PRUNE_VACUUM_CLEANUP:
heap_xlog_prune_freeze(record);
break;
- case XLOG_HEAP2_VISIBLE:
- heap_xlog_visible(record);
- break;
case XLOG_HEAP2_MULTI_INSERT:
heap_xlog_multi_insert(record);
break;
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 669c088ccff..ecc100c3362 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -980,8 +980,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
{
Assert(PageIsAllVisible(page));
LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
- old_vmbits = visibilitymap_set_vmbits(relation, blockno,
- vmbuffer, vmflags);
+ old_vmbits = visibilitymap_set(relation, blockno,
+ vmbuffer, vmflags);
if (old_vmbits == vmflags)
{
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 75c10ba20c6..2ff67d77cb4 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1886,8 +1886,8 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
MarkBufferDirty(buf);
LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
- visibilitymap_set_vmbits(vacrel->rel, blkno,
- vmbuffer, new_vmbits);
+ visibilitymap_set(vacrel->rel, blkno,
+ vmbuffer, new_vmbits);
if (RelationNeedsWAL(vacrel->rel))
{
@@ -2753,9 +2753,9 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
set_pd_all_vis = true;
PageSetAllVisible(page);
LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
- visibilitymap_set_vmbits(vacrel->rel,
- blkno,
- vmbuffer, vmflags);
+ visibilitymap_set(vacrel->rel,
+ blkno,
+ vmbuffer, vmflags);
conflict_xid = visibility_cutoff_xid;
}
diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index bb8dfd8910a..f7bad68ffc5 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -14,8 +14,7 @@
* visibilitymap_clear - clear bits for one page in the visibility map
* visibilitymap_pin - pin a map page for setting a bit
* visibilitymap_pin_ok - check whether correct map page is already pinned
- * visibilitymap_set - set bit(s) in a previously pinned page and log
- * visibilitymap_set_vmbits - set bit(s) in a pinned page
+ * visibilitymap_set - set bit(s) in a previously pinned page
* visibilitymap_get_status - get status of bits
* visibilitymap_count - count number of bits set in visibility map
* visibilitymap_prepare_truncate -
@@ -220,108 +219,6 @@ visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf)
return BufferIsValid(vmbuf) && BufferGetBlockNumber(vmbuf) == mapBlock;
}
-/*
- * visibilitymap_set - set bit(s) on a previously pinned page
- *
- * recptr is the LSN of the XLOG record we're replaying, if we're in recovery,
- * or InvalidXLogRecPtr in normal running. The VM page LSN is advanced to the
- * one provided; in normal running, we generate a new XLOG record and set the
- * page LSN to that value (though the heap page's LSN may *not* be updated;
- * see below). cutoff_xid is the largest xmin on the page being marked
- * all-visible; it is needed for Hot Standby, and can be InvalidTransactionId
- * if the page contains no tuples. It can also be set to InvalidTransactionId
- * when a page that is already all-visible is being marked all-frozen.
- *
- * Caller is expected to set the heap page's PD_ALL_VISIBLE bit before calling
- * this function. Except in recovery, caller should also pass the heap
- * buffer. When checksums are enabled and we're not in recovery, we must add
- * the heap buffer to the WAL chain to protect it from being torn.
- *
- * You must pass a buffer containing the correct map page to this function.
- * Call visibilitymap_pin first to pin the right one. This function doesn't do
- * any I/O.
- *
- * Returns the state of the page's VM bits before setting flags.
- */
-uint8
-visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
- XLogRecPtr recptr, Buffer vmBuf, TransactionId cutoff_xid,
- uint8 flags)
-{
- BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk);
- uint32 mapByte = HEAPBLK_TO_MAPBYTE(heapBlk);
- uint8 mapOffset = HEAPBLK_TO_OFFSET(heapBlk);
- Page page;
- uint8 *map;
- uint8 status;
-
-#ifdef TRACE_VISIBILITYMAP
- elog(DEBUG1, "vm_set flags 0x%02X for %s %d",
- flags, RelationGetRelationName(rel), heapBlk);
-#endif
-
- Assert(InRecovery || XLogRecPtrIsInvalid(recptr));
- Assert(InRecovery || PageIsAllVisible(BufferGetPage(heapBuf)));
- Assert((flags & VISIBILITYMAP_VALID_BITS) == flags);
-
- /* Must never set all_frozen bit without also setting all_visible bit */
- Assert(flags != VISIBILITYMAP_ALL_FROZEN);
-
- /* Check that we have the right heap page pinned, if present */
- if (BufferIsValid(heapBuf) && BufferGetBlockNumber(heapBuf) != heapBlk)
- elog(ERROR, "wrong heap buffer passed to visibilitymap_set");
-
- Assert(!BufferIsValid(heapBuf) || BufferIsExclusiveLocked(heapBuf));
-
- /* Check that we have the right VM page pinned */
- if (!BufferIsValid(vmBuf) || BufferGetBlockNumber(vmBuf) != mapBlock)
- elog(ERROR, "wrong VM buffer passed to visibilitymap_set");
-
- page = BufferGetPage(vmBuf);
- map = (uint8 *) PageGetContents(page);
- LockBuffer(vmBuf, BUFFER_LOCK_EXCLUSIVE);
-
- status = (map[mapByte] >> mapOffset) & VISIBILITYMAP_VALID_BITS;
- if (flags != status)
- {
- START_CRIT_SECTION();
-
- map[mapByte] |= (flags << mapOffset);
- MarkBufferDirty(vmBuf);
-
- if (RelationNeedsWAL(rel))
- {
- if (XLogRecPtrIsInvalid(recptr))
- {
- Assert(!InRecovery);
- recptr = log_heap_visible(rel, heapBuf, vmBuf, cutoff_xid, flags);
-
- /*
- * If data checksums are enabled (or wal_log_hints=on), we
- * need to protect the heap page from being torn.
- *
- * If not, then we must *not* update the heap page's LSN. In
- * this case, the FPI for the heap page was omitted from the
- * WAL record inserted above, so it would be incorrect to
- * update the heap page's LSN.
- */
- if (XLogHintBitIsNeeded())
- {
- Page heapPage = BufferGetPage(heapBuf);
-
- PageSetLSN(heapPage, recptr);
- }
- }
- PageSetLSN(page, recptr);
- }
-
- END_CRIT_SECTION();
- }
-
- LockBuffer(vmBuf, BUFFER_LOCK_UNLOCK);
- return status;
-}
-
/*
* Set flags in the VM block contained in the passed in vmBuf.
*
@@ -343,8 +240,8 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
* is pinned and exclusive locked.
*/
uint8
-visibilitymap_set_vmbits(Relation rel, BlockNumber heapBlk,
- Buffer vmBuf, uint8 flags)
+visibilitymap_set(Relation rel, BlockNumber heapBlk,
+ Buffer vmBuf, uint8 flags)
{
BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk);
uint32 mapByte = HEAPBLK_TO_MAPBYTE(heapBlk);
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index c95d30dfe8d..47998f1df15 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -343,13 +343,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
}
}
}
- else if (info == XLOG_HEAP2_VISIBLE)
- {
- xl_heap_visible *xlrec = (xl_heap_visible *) rec;
-
- appendStringInfo(buf, "snapshotConflictHorizon: %u, flags: 0x%02X",
- xlrec->snapshotConflictHorizon, xlrec->flags);
- }
else if (info == XLOG_HEAP2_MULTI_INSERT)
{
xl_heap_multi_insert *xlrec = (xl_heap_multi_insert *) rec;
@@ -454,9 +447,6 @@ heap2_identify(uint8 info)
case XLOG_HEAP2_PRUNE_VACUUM_CLEANUP:
id = "PRUNE_VACUUM_CLEANUP";
break;
- case XLOG_HEAP2_VISIBLE:
- id = "VISIBLE";
- break;
case XLOG_HEAP2_MULTI_INSERT:
id = "MULTI_INSERT";
break;
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index cc03f0706e9..2fdd4af90a8 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -454,7 +454,6 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
case XLOG_HEAP2_PRUNE_ON_ACCESS:
case XLOG_HEAP2_PRUNE_VACUUM_SCAN:
case XLOG_HEAP2_PRUNE_VACUUM_CLEANUP:
- case XLOG_HEAP2_VISIBLE:
case XLOG_HEAP2_LOCK_UPDATED:
break;
default:
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 4222bdab078..c619643e121 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -475,12 +475,12 @@ ResolveRecoveryConflictWithSnapshot(TransactionId snapshotConflictHorizon,
* If we get passed InvalidTransactionId then we do nothing (no conflict).
*
* This can happen when replaying already-applied WAL records after a
- * standby crash or restart, or when replaying an XLOG_HEAP2_VISIBLE
- * record that marks as frozen a page which was already all-visible. It's
- * also quite common with records generated during index deletion
- * (original execution of the deletion can reason that a recovery conflict
- * which is sufficient for the deletion operation must take place before
- * replay of the deletion record itself).
+ * standby crash or restart, or when replaying a record that marks as
+ * frozen a page which was already marked all-visible in the visibility
+ * map. It's also quite common with records generated during index
+ * deletion (original execution of the deletion can reason that a recovery
+ * conflict which is sufficient for the deletion operation must take place
+ * before replay of the deletion record itself).
*/
if (!TransactionIdIsValid(snapshotConflictHorizon))
return;
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index d8508593e7c..3672f372aa8 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -60,7 +60,6 @@
#define XLOG_HEAP2_PRUNE_ON_ACCESS 0x10
#define XLOG_HEAP2_PRUNE_VACUUM_SCAN 0x20
#define XLOG_HEAP2_PRUNE_VACUUM_CLEANUP 0x30
-#define XLOG_HEAP2_VISIBLE 0x40
#define XLOG_HEAP2_MULTI_INSERT 0x50
#define XLOG_HEAP2_LOCK_UPDATED 0x60
#define XLOG_HEAP2_NEW_CID 0x70
@@ -446,20 +445,6 @@ typedef struct xl_heap_inplace
#define MinSizeOfHeapInplace (offsetof(xl_heap_inplace, nmsgs) + sizeof(int))
-/*
- * This is what we need to know about setting a visibility map bit
- *
- * Backup blk 0: visibility map buffer
- * Backup blk 1: heap buffer
- */
-typedef struct xl_heap_visible
-{
- TransactionId snapshotConflictHorizon;
- uint8 flags;
-} xl_heap_visible;
-
-#define SizeOfHeapVisible (offsetof(xl_heap_visible, flags) + sizeof(uint8))
-
typedef struct xl_heap_new_cid
{
/*
@@ -503,11 +488,6 @@ extern void heap2_desc(StringInfo buf, XLogReaderState *record);
extern const char *heap2_identify(uint8 info);
extern void heap_xlog_logical_rewrite(XLogReaderState *r);
-extern XLogRecPtr log_heap_visible(Relation rel, Buffer heap_buffer,
- Buffer vm_buffer,
- TransactionId snapshotConflictHorizon,
- uint8 vmflags);
-
/* in heapdesc.c, so it can be shared between frontend/backend code */
extern void heap_xlog_deserialize_prune_and_freeze(char *cursor, uint16 flags,
int *nplans, xlhp_freeze_plan **plans,
diff --git a/src/include/access/visibilitymap.h b/src/include/access/visibilitymap.h
index fc7056a91ea..c5b1e1f7adb 100644
--- a/src/include/access/visibilitymap.h
+++ b/src/include/access/visibilitymap.h
@@ -15,7 +15,6 @@
#define VISIBILITYMAP_H
#include "access/visibilitymapdefs.h"
-#include "access/xlogdefs.h"
#include "storage/block.h"
#include "storage/buf.h"
#include "utils/relcache.h"
@@ -31,14 +30,8 @@ extern bool visibilitymap_clear(Relation rel, BlockNumber heapBlk,
extern void visibilitymap_pin(Relation rel, BlockNumber heapBlk,
Buffer *vmbuf);
extern bool visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf);
-extern uint8 visibilitymap_set(Relation rel,
- BlockNumber heapBlk, Buffer heapBuf,
- XLogRecPtr recptr,
- Buffer vmBuf,
- TransactionId cutoff_xid,
- uint8 flags);
-extern uint8 visibilitymap_set_vmbits(Relation rel, BlockNumber heapBlk,
- Buffer vmBuf, uint8 flags);
+extern uint8 visibilitymap_set(Relation rel, BlockNumber heapBlk,
+ Buffer vmBuf, uint8 flags);
extern uint8 visibilitymap_get_status(Relation rel, BlockNumber heapBlk, Buffer *vmbuf);
extern void visibilitymap_count(Relation rel, BlockNumber *all_visible, BlockNumber *all_frozen);
extern BlockNumber visibilitymap_prepare_truncate(Relation rel,
diff --git a/src/include/access/visibilitymapdefs.h b/src/include/access/visibilitymapdefs.h
index 5ad5c020877..e01bce4c99f 100644
--- a/src/include/access/visibilitymapdefs.h
+++ b/src/include/access/visibilitymapdefs.h
@@ -21,14 +21,5 @@
#define VISIBILITYMAP_ALL_FROZEN 0x02
#define VISIBILITYMAP_VALID_BITS 0x03 /* OR of all valid visibilitymap
* flags bits */
-/*
- * To detect recovery conflicts during logical decoding on a standby, we need
- * to know if a table is a user catalog table. For that we add an additional
- * bit into xl_heap_visible.flags, in addition to the above.
- *
- * NB: VISIBILITYMAP_XLOG_* may not be passed to visibilitymap_set().
- */
-#define VISIBILITYMAP_XLOG_CATALOG_REL 0x04
-#define VISIBILITYMAP_XLOG_VALID_BITS (VISIBILITYMAP_VALID_BITS | VISIBILITYMAP_XLOG_CATALOG_REL)
#endif /* VISIBILITYMAPDEFS_H */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index a13e8162890..b4c880c083f 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -4273,7 +4273,6 @@ xl_heap_prune
xl_heap_rewrite_mapping
xl_heap_truncate
xl_heap_update
-xl_heap_visible
xl_invalid_page
xl_invalid_page_key
xl_invalidations
--
2.43.0
[text/x-patch] v9-0015-Rename-GlobalVisTestIsRemovableXid-to-GlobalVisXi.patch (7.1K, 15-v9-0015-Rename-GlobalVisTestIsRemovableXid-to-GlobalVisXi.patch)
download | inline diff:
From 00012be836b472c2f0185b1c037cf29b480e5507 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 18 Jul 2025 16:30:04 -0400
Subject: [PATCH v9 15/22] Rename GlobalVisTestIsRemovableXid() to
GlobalVisXidVisibleToAll()
Currently, we only use GlobalVisTestIsRemovableXid() to check if a
tuple's xmax is visible to all, meaning we can remove it. But future
commits will use GlobalVisTestIsRemovableXid() to test if a tuple's xmin
is visible to all for the purposes of determining if setting the page
all-visible in the VM. In that case, it makes more sense to call the
function GlobalVisXidVisibleToAll().
---
src/backend/access/heap/heapam_visibility.c | 6 +++---
src/backend/access/heap/pruneheap.c | 14 +++++++-------
src/backend/access/spgist/spgvacuum.c | 2 +-
src/backend/storage/ipc/procarray.c | 13 ++++++-------
src/include/utils/snapmgr.h | 4 ++--
5 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/src/backend/access/heap/heapam_visibility.c b/src/backend/access/heap/heapam_visibility.c
index 05f6946fe60..4ebc8abdbeb 100644
--- a/src/backend/access/heap/heapam_visibility.c
+++ b/src/backend/access/heap/heapam_visibility.c
@@ -1447,7 +1447,7 @@ HeapTupleSatisfiesNonVacuumable(HeapTuple htup, Snapshot snapshot,
{
Assert(TransactionIdIsValid(dead_after));
- if (GlobalVisTestIsRemovableXid(snapshot->vistest, dead_after))
+ if (GlobalVisXidVisibleToAll(snapshot->vistest, dead_after))
res = HEAPTUPLE_DEAD;
}
else
@@ -1512,8 +1512,8 @@ HeapTupleIsSurelyDead(HeapTuple htup, GlobalVisState *vistest)
return false;
/* Deleter committed, so tuple is dead if the XID is old enough. */
- return GlobalVisTestIsRemovableXid(vistest,
- HeapTupleHeaderGetRawXmax(tuple));
+ return GlobalVisXidVisibleToAll(vistest,
+ HeapTupleHeaderGetRawXmax(tuple));
}
/*
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ecc100c3362..73ca4e88c1f 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -231,7 +231,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
*/
vistest = GlobalVisTestFor(relation);
- if (!GlobalVisTestIsRemovableXid(vistest, prune_xid))
+ if (!GlobalVisXidVisibleToAll(vistest, prune_xid))
return;
/*
@@ -574,9 +574,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* Determining HTSV only once for each tuple is required for correctness,
* to deal with cases where running HTSV twice could result in different
* results. For example, RECENTLY_DEAD can turn to DEAD if another
- * checked item causes GlobalVisTestIsRemovableFullXid() to update the
- * horizon, or INSERT_IN_PROGRESS can change to DEAD if the inserting
- * transaction aborts.
+ * checked item causes GlobalVisXidVisibleToAll() to update the horizon,
+ * or INSERT_IN_PROGRESS can change to DEAD if the inserting transaction
+ * aborts.
*
* It's also good for performance. Most commonly tuples within a page are
* stored at decreasing offsets (while the items are stored at increasing
@@ -1173,11 +1173,11 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
* Determine whether or not the tuple is considered dead when compared
* with the provided GlobalVisState. On-access pruning does not provide
* VacuumCutoffs. And for vacuum, even if the tuple's xmax is not older
- * than OldestXmin, GlobalVisTestIsRemovableXid() could find the row dead
- * if the GlobalVisState has been updated since the beginning of vacuuming
+ * than OldestXmin, GlobalVisXidVisibleToAll() could find the row dead if
+ * the GlobalVisState has been updated since the beginning of vacuuming
* the relation.
*/
- if (GlobalVisTestIsRemovableXid(prstate->vistest, dead_after))
+ if (GlobalVisXidVisibleToAll(prstate->vistest, dead_after))
return HEAPTUPLE_DEAD;
return res;
diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c
index 8f8a1ad7796..496cca69410 100644
--- a/src/backend/access/spgist/spgvacuum.c
+++ b/src/backend/access/spgist/spgvacuum.c
@@ -536,7 +536,7 @@ vacuumRedirectAndPlaceholder(Relation index, Relation heaprel, Buffer buffer)
*/
if (dt->tupstate == SPGIST_REDIRECT &&
(!TransactionIdIsValid(dt->xid) ||
- GlobalVisTestIsRemovableXid(vistest, dt->xid)))
+ GlobalVisXidVisibleToAll(vistest, dt->xid)))
{
dt->tupstate = SPGIST_PLACEHOLDER;
Assert(opaque->nRedirection > 0);
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 200f72c6e25..f67f01c17c2 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -4181,8 +4181,7 @@ GlobalVisUpdate(void)
* See comment for GlobalVisState for details.
*/
bool
-GlobalVisTestIsRemovableFullXid(GlobalVisState *state,
- FullTransactionId fxid)
+GlobalVisFullXidVisibleToAll(GlobalVisState *state, FullTransactionId fxid)
{
/*
* If fxid is older than maybe_needed bound, it definitely is visible to
@@ -4223,7 +4222,7 @@ GlobalVisTestIsRemovableFullXid(GlobalVisState *state,
* relfrozenxid).
*/
bool
-GlobalVisTestIsRemovableXid(GlobalVisState *state, TransactionId xid)
+GlobalVisXidVisibleToAll(GlobalVisState *state, TransactionId xid)
{
FullTransactionId fxid;
@@ -4237,7 +4236,7 @@ GlobalVisTestIsRemovableXid(GlobalVisState *state, TransactionId xid)
*/
fxid = FullXidRelativeTo(state->definitely_needed, xid);
- return GlobalVisTestIsRemovableFullXid(state, fxid);
+ return GlobalVisFullXidVisibleToAll(state, fxid);
}
/*
@@ -4251,12 +4250,12 @@ GlobalVisCheckRemovableFullXid(Relation rel, FullTransactionId fxid)
state = GlobalVisTestFor(rel);
- return GlobalVisTestIsRemovableFullXid(state, fxid);
+ return GlobalVisFullXidVisibleToAll(state, fxid);
}
/*
* Convenience wrapper around GlobalVisTestFor() and
- * GlobalVisTestIsRemovableXid(), see their comments.
+ * GlobalVisTestIsVisibleXid(), see their comments.
*/
bool
GlobalVisCheckRemovableXid(Relation rel, TransactionId xid)
@@ -4265,7 +4264,7 @@ GlobalVisCheckRemovableXid(Relation rel, TransactionId xid)
state = GlobalVisTestFor(rel);
- return GlobalVisTestIsRemovableXid(state, xid);
+ return GlobalVisXidVisibleToAll(state, xid);
}
/*
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index 604c1f90216..a0ea2cfcea2 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -100,8 +100,8 @@ extern char *ExportSnapshot(Snapshot snapshot);
*/
typedef struct GlobalVisState GlobalVisState;
extern GlobalVisState *GlobalVisTestFor(Relation rel);
-extern bool GlobalVisTestIsRemovableXid(GlobalVisState *state, TransactionId xid);
-extern bool GlobalVisTestIsRemovableFullXid(GlobalVisState *state, FullTransactionId fxid);
+extern bool GlobalVisXidVisibleToAll(GlobalVisState *state, TransactionId xid);
+extern bool GlobalVisFullXidVisibleToAll(GlobalVisState *state, FullTransactionId fxid);
extern bool GlobalVisCheckRemovableXid(Relation rel, TransactionId xid);
extern bool GlobalVisCheckRemovableFullXid(Relation rel, FullTransactionId fxid);
--
2.43.0
[text/x-patch] v9-0016-Use-GlobalVisState-to-determine-page-level-visibi.patch (10.8K, 16-v9-0016-Use-GlobalVisState-to-determine-page-level-visibi.patch)
download | inline diff:
From 438ce859c03936b016e1345be5e9d5950d96f514 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 29 Jul 2025 14:38:24 -0400
Subject: [PATCH v9 16/22] Use GlobalVisState to determine page level
visibility
During pruning and during vacuum's third phase, we try to determine if
the whole page can be set all-visible in the visibility map. Instead of
using OldestXmin to determine if all the tuples on a page are visible to
everyone, use the GlobalVisState. This allows us to start setting the VM
during on-access pruning in a future commit.
It is possible for the GlobalVisState to change during the course of a
vacuum. In all but extraordinary cases, it moves forward, meaning more
pages could potentially be set in the VM.
Because comparing a transaction ID to the GlobalVisState requires more
operations than comparing it to another single transaction ID, we now
wait until after examining all the tuples on the page and if we have
maintained the visibility_cutoff_xid, we compare that to the
GlobalVisState just once per page. This works because if the page is
all-visible and has live, committed tuples on it, the
visibility_cutoff_xid will contain the newest xmin on the page. If
everyone can see it, the page is truly all-visible.
Doing this may mean we examine more tuples' xmins than before, as we may
have set all_visible to false sooner when encountering a live tuple
newer than OldestXmin. However, these extra comparisons were found not
to be significant in a profile.
---
src/backend/access/heap/heapam_visibility.c | 28 ++++++++++++
src/backend/access/heap/pruneheap.c | 48 +++++++++------------
src/backend/access/heap/vacuumlazy.c | 19 ++++----
src/include/access/heapam.h | 4 +-
4 files changed, 60 insertions(+), 39 deletions(-)
diff --git a/src/backend/access/heap/heapam_visibility.c b/src/backend/access/heap/heapam_visibility.c
index 4ebc8abdbeb..edd529dc3c0 100644
--- a/src/backend/access/heap/heapam_visibility.c
+++ b/src/backend/access/heap/heapam_visibility.c
@@ -1189,6 +1189,34 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
return res;
}
+/*
+ * Nearly the same as HeapTupleSatisfiesVacuum, but uses a GlobalVisState to
+ * determine whether or not a tuple is HEAPTUPLE_DEAD Or
+ * HEAPTUPLE_RECENTLY_DEAD. It serves the same purpose but can be used by
+ * callers that have not calculated a single OldestXmin value.
+ */
+HTSV_Result
+HeapTupleSatisfiesVacuumGlobalVis(HeapTuple htup, GlobalVisState *vistest,
+ Buffer buffer)
+{
+ TransactionId dead_after = InvalidTransactionId;
+ HTSV_Result res;
+
+ res = HeapTupleSatisfiesVacuumHorizon(htup, buffer, &dead_after);
+
+ if (res == HEAPTUPLE_RECENTLY_DEAD)
+ {
+ Assert(TransactionIdIsValid(dead_after));
+
+ if (GlobalVisXidVisibleToAll(vistest, dead_after))
+ res = HEAPTUPLE_DEAD;
+ }
+ else
+ Assert(!TransactionIdIsValid(dead_after));
+
+ return res;
+}
+
/*
* Work horse for HeapTupleSatisfiesVacuum and similar routines.
*
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 73ca4e88c1f..273e9412a01 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -141,10 +141,9 @@ typedef struct
* all_visible and all_frozen indicate if the all-visible and all-frozen
* bits in the visibility map can be set for this page after pruning.
*
- * visibility_cutoff_xid is the newest xmin of live tuples on the page.
- * The caller can use it as the conflict horizon, when setting the VM
- * bits. It is only valid if we froze some tuples, and all_frozen is
- * true.
+ * visibility_cutoff_xid is the newest xmin of live tuples on the page. It
+ * can be used as the conflict horizon, when setting the VM or when
+ * freezing all the live tuples on the page.
*
* NOTE: all_visible and all_frozen don't include LP_DEAD items until
* directly before updating the VM. We ignore LP_DEAD items when deciding
@@ -553,14 +552,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/*
* The visibility cutoff xid is the newest xmin of live, committed tuples
- * older than OldestXmin on the page. This field is only kept up-to-date
- * if the page is all-visible. As soon as a tuple is encountered that is
- * not visible to all, this field is unmaintained. As long as it is
- * maintained, it can be used to calculate the snapshot conflict horizon.
- * This is most likely to happen when updating the VM and/or freezing all
- * live tuples on the page. It is updated before returning to the caller
- * because vacuum does assert-build only validation on the page using this
- * field.
+ * on the page older than the visibility horizon represented in the
+ * GlobalVisState.
+ *
+ * If we encounter an uncommitted tuple, this field is unmaintained. If
+ * the page is being set all-visible or when freezing all live tuples on
+ * the page, it is used to calculate the snapshot conflict horizon.
*/
prstate.visibility_cutoff_xid = InvalidTransactionId;
@@ -756,6 +753,16 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.ndead > 0 ||
prstate.nunused > 0;
+ /*
+ * After processing all the live tuples on the page, if the newest xmin
+ * amongst them is not visible to everyone, the page cannot be
+ * all-visible.
+ */
+ if (prstate.all_visible &&
+ TransactionIdIsNormal(prstate.visibility_cutoff_xid) &&
+ !GlobalVisXidVisibleToAll(prstate.vistest, prstate.visibility_cutoff_xid))
+ prstate.all_visible = prstate.all_frozen = false;
+
/*
* Even if we don't prune anything, if we found a new value for the
* pd_prune_xid field or the page was marked full, we will update those
@@ -1099,12 +1106,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(cutoffs);
-
Assert(prstate.lpdead_items == 0);
if (!heap_page_is_all_visible(relation, buffer,
- cutoffs->OldestXmin,
+ prstate.vistest,
&debug_all_frozen,
&debug_cutoff, off_loc))
Assert(false);
@@ -1629,19 +1634,6 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
*/
xmin = HeapTupleHeaderGetXmin(htup);
- /*
- * For now always use prstate->cutoffs for this test, because
- * we only update 'all_visible' when freezing is requested. We
- * could use GlobalVisTestIsRemovableXid instead, if a
- * non-freezing caller wanted to set the VM bit.
- */
- Assert(prstate->cutoffs);
- if (!TransactionIdPrecedes(xmin, prstate->cutoffs->OldestXmin))
- {
- prstate->all_visible = prstate->all_frozen = false;
- break;
- }
-
/* Track newest xmin on page. */
if (TransactionIdFollows(xmin, prstate->visibility_cutoff_xid) &&
TransactionIdIsNormal(xmin))
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 2ff67d77cb4..7558ac697f1 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -464,7 +464,7 @@ static void dead_items_add(LVRelState *vacrel, BlockNumber blkno, OffsetNumber *
static void dead_items_reset(LVRelState *vacrel);
static void dead_items_cleanup(LVRelState *vacrel);
static bool heap_page_would_be_all_visible(Relation rel, Buffer buf,
- TransactionId OldestXmin,
+ GlobalVisState *vistest,
OffsetNumber *deadoffsets,
int ndeadoffsets,
bool *all_frozen,
@@ -2715,7 +2715,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
InvalidOffsetNumber);
if (heap_page_would_be_all_visible(vacrel->rel, buffer,
- vacrel->cutoffs.OldestXmin,
+ vacrel->vistest,
deadoffsets, num_offsets,
&all_frozen, &visibility_cutoff_xid,
&vacrel->offnum))
@@ -3458,13 +3458,13 @@ dead_items_cleanup(LVRelState *vacrel)
*/
bool
heap_page_is_all_visible(Relation rel, Buffer buf,
- TransactionId OldestXmin,
+ GlobalVisState *vistest,
bool *all_frozen,
TransactionId *visibility_cutoff_xid,
OffsetNumber *logging_offnum)
{
- return heap_page_would_be_all_visible(rel, buf, OldestXmin,
+ return heap_page_would_be_all_visible(rel, buf, vistest,
NULL, 0,
all_frozen,
visibility_cutoff_xid,
@@ -3483,7 +3483,7 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
* Returns true if the page is all-visible other than the provided
* deadoffsets and false otherwise.
*
- * OldestXmin is used to determine visibility.
+ * vistest is used to determine visibility.
*
* *all_frozen is an output parameter indicating to the caller if every tuple
* on the page is frozen.
@@ -3504,7 +3504,7 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
*/
static bool
heap_page_would_be_all_visible(Relation rel, Buffer buf,
- TransactionId OldestXmin,
+ GlobalVisState *vistest,
OffsetNumber *deadoffsets,
int ndeadoffsets,
bool *all_frozen,
@@ -3576,8 +3576,8 @@ heap_page_would_be_all_visible(Relation rel, Buffer buf,
tuple.t_len = ItemIdGetLength(itemid);
tuple.t_tableOid = RelationGetRelid(rel);
- switch (HeapTupleSatisfiesVacuum(&tuple, OldestXmin,
- buf))
+ switch (HeapTupleSatisfiesVacuumGlobalVis(&tuple, vistest,
+ buf))
{
case HEAPTUPLE_LIVE:
{
@@ -3596,8 +3596,7 @@ heap_page_would_be_all_visible(Relation rel, Buffer buf,
* that everyone sees it as committed?
*/
xmin = HeapTupleHeaderGetXmin(tuple.t_data);
- if (!TransactionIdPrecedes(xmin,
- OldestXmin))
+ if (!GlobalVisXidVisibleToAll(vistest, xmin))
{
all_visible = false;
*all_frozen = false;
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 0b9bb1c9b13..4278f351bdf 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -342,7 +342,7 @@ extern void heap_inplace_unlock(Relation relation,
HeapTuple oldtup, Buffer buffer);
extern bool heap_page_is_all_visible(Relation rel, Buffer buf,
- TransactionId OldestXmin,
+ GlobalVisState *vistest,
bool *all_frozen,
TransactionId *visibility_cutoff_xid,
OffsetNumber *logging_offnum);
@@ -415,6 +415,8 @@ extern TM_Result HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
Buffer buffer);
extern HTSV_Result HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
Buffer buffer);
+extern HTSV_Result HeapTupleSatisfiesVacuumGlobalVis(HeapTuple htup,
+ GlobalVisState *vistest, Buffer buffer);
extern HTSV_Result HeapTupleSatisfiesVacuumHorizon(HeapTuple htup, Buffer buffer,
TransactionId *dead_after);
extern void HeapTupleSetHintBits(HeapTupleHeader tuple, Buffer buffer,
--
2.43.0
[text/x-patch] v9-0012-Eliminate-xl_heap_visible-from-vacuum-phase-I-pru.patch (29.0K, 17-v9-0012-Eliminate-xl_heap_visible-from-vacuum-phase-I-pru.patch)
download | inline diff:
From 73cfbe246ba075db052afd207749a7c66ec1a9bc Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 18 Jun 2025 12:41:00 -0400
Subject: [PATCH v9 12/22] Eliminate xl_heap_visible from vacuum phase I
prune/freeze
Instead of emitting a separate WAL record for every block rendered
all-visible/frozen by vacuum's phase I, include the changes to the VM in
the xl_heap_prune record already emitted.
This is only enabled for vacuum's prune/freeze work, not for on-access
pruning.
---
src/backend/access/heap/pruneheap.c | 456 ++++++++++++++++-----------
src/backend/access/heap/vacuumlazy.c | 30 --
src/include/access/heapam.h | 15 +-
3 files changed, 279 insertions(+), 222 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 3483b5caff3..683c1762c25 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -44,6 +44,13 @@ typedef struct
bool mark_unused_now;
/* whether to attempt freezing tuples */
bool freeze;
+
+ /*
+ * Whether or not to consider updating the VM. There is some bookkeeping
+ * that must be maintained if we would like to update the VM.
+ */
+ bool consider_update_vm;
+
struct VacuumCutoffs *cutoffs;
/*-------------------------------------------------------
@@ -108,8 +115,9 @@ typedef struct
*
* These fields are not used by pruning itself for the most part, but are
* used to collect information about what was pruned and what state the
- * page is in after pruning, for the benefit of the caller. They are
- * copied to the caller's PruneFreezeResult at the end.
+ * page is in after pruning to use when updating the visibility map and
+ * for the benefit of the caller. They are copied to the caller's
+ * PruneFreezeResult at the end.
* -------------------------------------------------------
*/
@@ -138,11 +146,10 @@ typedef struct
* bits. It is only valid if we froze some tuples, and all_frozen is
* true.
*
- * NOTE: all_visible and all_frozen don't include LP_DEAD items. That's
- * convenient for heap_page_prune_and_freeze(), to use them to decide
- * whether to freeze the page or not. The all_visible and all_frozen
- * values returned to the caller are adjusted to include LP_DEAD items at
- * the end.
+ * NOTE: all_visible and all_frozen don't include LP_DEAD items until
+ * directly before updating the VM. We ignore LP_DEAD items when deciding
+ * whether or not to opportunistically freeze and when determining the
+ * snapshot conflict horizon required when freezing tuples.
*/
bool all_visible;
bool all_frozen;
@@ -371,12 +378,15 @@ identify_and_fix_vm_corruption(Relation relation,
* If the HEAP_PRUNE_FREEZE option is set, we will also freeze tuples if it's
* required in order to advance relfrozenxid / relminmxid, or if it's
* considered advantageous for overall system performance to do so now. The
- * 'cutoffs', 'presult', 'new_relfrozen_xid' and 'new_relmin_mxid' arguments
- * are required when freezing. When HEAP_PRUNE_FREEZE option is set, we also
- * set presult->all_visible and presult->all_frozen on exit, to indicate if
- * the VM bits can be set. They are always set to false when the
- * HEAP_PRUNE_FREEZE option is not set, because at the moment only callers
- * that also freeze need that information.
+ * 'cutoffs', 'new_relfrozen_xid' and 'new_relmin_mxid' arguments are required
+ * when freezing.
+ *
+ * If HEAP_PAGE_PRUNE_UPDATE_VM is set and the visibility status of the page
+ * has changed, we will update the VM at the same time as pruning and freezing
+ * the heap page. We will also update presult->old_vmbits and
+ * presult->new_vmbits with the state of the VM before and after updating it
+ * for the caller to use in bookkeeping. Note that new and old_vmbits will be
+ * 0 if HEAP_PAGE_PRUNE_UPDATE_VM is not set.
*
* blk_known_av is the visibility status of the heap block as of the last call
* to find_next_unskippable_block(). vmbuffer is the buffer that may already
@@ -392,6 +402,8 @@ identify_and_fix_vm_corruption(Relation relation,
* FREEZE indicates that we will also freeze tuples, and will return
* 'all_visible', 'all_frozen' flags to the caller.
*
+ * UPDATE_VM indicates that we will set the page's status in the VM.
+ *
* cutoffs contains the freeze cutoffs, established by VACUUM at the beginning
* of vacuuming the relation. Required if HEAP_PRUNE_FREEZE option is set.
* cutoffs->OldestXmin is also used to determine if dead tuples are
@@ -436,18 +448,24 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
HeapTupleData tup;
bool do_freeze;
bool do_prune;
- bool do_hint;
+ bool do_hint_full_or_prunable;
+ bool do_set_vm;
uint8 vmflags = 0;
uint8 old_vmbits = 0;
bool hint_bit_fpi;
int64 fpi_before = pgWalUsage.wal_fpi;
+ bool all_frozen_except_lp_dead = false;
+ bool set_pd_all_visible = false;
/* Copy parameters to prstate */
prstate.vistest = vistest;
prstate.mark_unused_now = (options & HEAP_PAGE_PRUNE_MARK_UNUSED_NOW) != 0;
prstate.freeze = (options & HEAP_PAGE_PRUNE_FREEZE) != 0;
+ prstate.consider_update_vm = (options & HEAP_PAGE_PRUNE_UPDATE_VM) != 0;
prstate.cutoffs = cutoffs;
+ Assert(!prstate.consider_update_vm || vmbuffer);
+
/*
* Our strategy is to scan the page and make lists of items to change,
* then apply the changes within a critical section. This keeps as much
@@ -492,50 +510,57 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.deadoffsets = presult->deadoffsets;
/*
- * Caller may update the VM after we're done. We can keep track of
- * whether the page will be all-visible and all-frozen after pruning and
- * freezing to help the caller to do that.
+ * Keep track of whether or not the page will be all-visible and
+ * all-frozen for use in opportunistic freezing and to update the VM if
+ * the caller requests it.
+ *
+ * Currently, only VACUUM attempts freezing and setting the VM bits. But
+ * other callers could do either one. The visibility bookkeeping is
+ * required for opportunistic freezing (in addition to setting the VM
+ * bits) because we only consider opportunistically freezing tuples if the
+ * whole page would become all-frozen or if the whole page will be frozen
+ * except for dead tuples that will be removed by vacuum.
*
- * Currently, only VACUUM sets the VM bits. To save the effort, only do
- * the bookkeeping if the caller needs it. Currently, that's tied to
- * HEAP_PAGE_PRUNE_FREEZE, but it could be a separate flag if you wanted
- * to update the VM bits without also freezing or freeze without also
- * setting the VM bits.
+ * If only updating the VM, we must initialize all_frozen to false, as
+ * heap_prepare_freeze_tuple() will not be called for each tuple on the
+ * page and we will not end up correctly setting it to false later.
*
- * In addition to telling the caller whether it can set the VM bit, we
- * also use 'all_visible' and 'all_frozen' for our own decision-making. If
- * the whole page would become frozen, we consider opportunistically
- * freezing tuples. We will not be able to freeze the whole page if there
- * are tuples present that are not visible to everyone or if there are
- * dead tuples which are not yet removable. However, dead tuples which
- * will be removed by the end of vacuuming should not preclude us from
- * opportunistically freezing. Because of that, we do not clear
- * all_visible when we see LP_DEAD items. We fix that at the end of the
- * function, when we return the value to the caller, so that the caller
- * doesn't set the VM bit incorrectly.
+ * Dead tuples which will be removed by the end of vacuuming should not
+ * preclude us from opportunistically freezing, so we do not clear
+ * all_visible when we see LP_DEAD items. We fix that after determining
+ * whether or not to freeze but before deciding whether or not to update
+ * the VM so that we don't set the VM bit incorrectly.
+ *
+ * If not freezing or updating the VM, we otherwise avoid the extra
+ * bookkeeping. Initializing all_visible to false allows skipping the work
+ * to update them in heap_prune_record_unchanged_lp_normal().
*/
if (prstate.freeze)
{
prstate.all_visible = true;
prstate.all_frozen = true;
}
+ else if (prstate.consider_update_vm)
+ {
+ prstate.all_visible = true;
+ prstate.all_frozen = false;
+ }
else
{
- /*
- * Initializing to false allows skipping the work to update them in
- * heap_prune_record_unchanged_lp_normal().
- */
prstate.all_visible = false;
prstate.all_frozen = false;
}
/*
- * The visibility cutoff xid is the newest xmin of live tuples on the
- * page. In the common case, this will be set as the conflict horizon the
- * caller can use for updating the VM. If, at the end of freezing and
- * pruning, the page is all-frozen, there is no possibility that any
- * running transaction on the standby does not see tuples on the page as
- * all-visible, so the conflict horizon remains InvalidTransactionId.
+ * The visibility cutoff xid is the newest xmin of live, committed tuples
+ * older than OldestXmin on the page. This field is only kept up-to-date
+ * if the page is all-visible. As soon as a tuple is encountered that is
+ * not visible to all, this field is unmaintained. As long as it is
+ * maintained, it can be used to calculate the snapshot conflict horizon.
+ * This is most likely to happen when updating the VM and/or freezing all
+ * live tuples on the page. It is updated before returning to the caller
+ * because vacuum does assert-build only validation on the page using this
+ * field.
*/
prstate.visibility_cutoff_xid = InvalidTransactionId;
@@ -733,10 +758,11 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/*
* Even if we don't prune anything, if we found a new value for the
- * pd_prune_xid field or the page was marked full, we will update the hint
- * bit.
+ * pd_prune_xid field or the page was marked full, we will update those
+ * hint bits.
*/
- do_hint = ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
+ do_hint_full_or_prunable =
+ ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
PageIsFull(page);
/*
@@ -784,7 +810,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (XLogCheckBufferNeedsBackup(buffer))
do_freeze = true;
}
- else if (do_hint)
+ else if (do_hint_full_or_prunable)
{
if (XLogHintBitIsNeeded() && XLogCheckBufferNeedsBackup(buffer))
do_freeze = true;
@@ -823,11 +849,84 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
}
+ /*
+ * It was convenient to ignore LP_DEAD items in all_visible earlier on to
+ * make the choice of whether or not to freeze the page unaffected by the
+ * short-term presence of LP_DEAD items. These LP_DEAD items were
+ * effectively assumed to be LP_UNUSED items in the making. It doesn't
+ * matter which vacuum heap pass (initial pass or final pass) ends up
+ * setting the page all-frozen, as long as the ongoing VACUUM does it.
+ *
+ * Now that freezing has been finalized, unset all_visible if there are
+ * any LP_DEAD items on the page. It needs to reflect the present state of
+ * the page when using it to determine whether or not to update the VM.
+ *
+ * Keep track of whether or not the page was all-frozen except LP_DEAD
+ * items for the purposes of calculating the snapshot conflict horizon,
+ * though.
+ */
+ all_frozen_except_lp_dead = prstate.all_frozen;
+ if (prstate.lpdead_items > 0)
+ {
+ prstate.all_visible = false;
+ prstate.all_frozen = false;
+ }
+
Assert(!prstate.all_frozen || prstate.all_visible);
+
+ /*
+ * Handle setting visibility map bit based on information from the VM (as
+ * of last heap_vac_scan_next_block() call), and from all_visible and
+ * all_frozen variables.
+ */
+ if (prstate.consider_update_vm)
+ {
+ /*
+ * Clear any VM corruption. This does not need to be in a critical
+ * section, so we do it first. If PD_ALL_VISIBLE is incorrectly set,
+ * we may mark the heap page buffer dirty here and could end up doing
+ * so again later. This is not a correctness issue and is in the path
+ * of VM corruption, so we don't have to worry about the extra
+ * performance overhead.
+ */
+ if (identify_and_fix_vm_corruption(relation,
+ blockno, buffer, page,
+ blk_known_av, prstate.lpdead_items, vmbuffer))
+ {
+ /* If we fix corruption, don't update the VM further */
+ }
+
+ /* Determine if we actually need to set the VM and which bits to set. */
+ else if (prstate.all_visible &&
+ (!blk_known_av ||
+ (prstate.all_frozen && !VM_ALL_FROZEN(relation, blockno, &vmbuffer))))
+ {
+ vmflags |= VISIBILITYMAP_ALL_VISIBLE;
+ if (prstate.all_frozen)
+ vmflags |= VISIBILITYMAP_ALL_FROZEN;
+ }
+ }
+
+ do_set_vm = vmflags & VISIBILITYMAP_VALID_BITS;
+
+ /*
+ * Don't set PD_ALL_VISIBLE unless we also plan to set the VM. While it is
+ * correct for a heap page to have PD_ALL_VISIBLE even if the VM is not
+ * set, we strongly prefer to keep them in sync.
+ *
+ * Prior to Postgres 19, it was possible for the page-level bit to be set
+ * and the VM bit to be clear. This could happen if we crashed after
+ * setting PD_ALL_VISIBLE but before setting bits in the VM.
+ */
+ set_pd_all_visible = do_set_vm && !PageIsAllVisible(page);
+
+ /* Save these for the caller in case we later zero out vmflags */
+ presult->new_vmbits = vmflags;
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
- if (do_hint)
+ if (do_hint_full_or_prunable)
{
/*
* Update the page's pd_prune_xid field to either zero, or the lowest
@@ -843,15 +942,16 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
PageClearFull(page);
/*
- * If that's all we had to do to the page, this is a non-WAL-logged
- * hint. If we are going to freeze or prune the page, we will mark
- * the buffer dirty below.
+ * If we are _only_ setting the prune_xid or PD_PAGE_FULL hint, then
+ * this is a non-WAL-logged hint. If we are going to freeze or prune
+ * tuples on the page or set PD_ALL_VISIBLE, we will mark the buffer
+ * dirty and emit WAL below.
*/
- if (!do_freeze && !do_prune)
+ if (!do_prune && !do_freeze && !set_pd_all_visible)
MarkBufferDirtyHint(buffer, true);
}
- if (do_prune || do_freeze)
+ if (do_prune || do_freeze || do_set_vm)
{
/* Apply the planned item changes and repair page fragmentation. */
if (do_prune)
@@ -865,12 +965,48 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
heap_freeze_prepared_tuples(buffer, prstate.frozen, prstate.nfrozen);
- MarkBufferDirty(buffer);
+ if (set_pd_all_visible)
+ PageSetAllVisible(page);
+
+ /*
+ * We only set PD_ALL_VISIBLE if we also set the VM, and since setting
+ * the VM requires emitting WAL, MarkBufferDirtyHint() isn't
+ * appropriate here.
+ */
+ if (do_prune || do_freeze || set_pd_all_visible)
+ MarkBufferDirty(buffer);
+
+ if (do_set_vm)
+ {
+ Assert(PageIsAllVisible(page));
+ LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE);
+ old_vmbits = visibilitymap_set_vmbits(relation, blockno,
+ vmbuffer, vmflags);
+
+ if (old_vmbits == vmflags)
+ {
+ LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK);
+ do_set_vm = false;
+ /* 0 out vmflags so we don't emit WAL to update the VM */
+ vmflags = 0;
+ }
+ }
+
+ /*
+ * It should never be the case that PD_ALL_VISIBLE is not set and the
+ * VM is set. Or, if it were, we should have caught it earlier when
+ * finding and fixing VM corruption. So, if we found out the VM was
+ * already set above, we should have found PD_ALL_VISIBLE set earlier.
+ */
+ Assert(!set_pd_all_visible || do_set_vm);
/*
- * Emit a WAL XLOG_HEAP2_PRUNE_FREEZE record showing what we did
+ * Emit a WAL XLOG_HEAP2_PRUNE_FREEZE record showing what we did. If
+ * we were only updating the VM and it turns out it was already set,
+ * we will have unset do_set_vm earlier. As such, check it again
+ * before emitting the record.
*/
- if (RelationNeedsWAL(relation))
+ if (RelationNeedsWAL(relation) && (do_set_vm || do_prune || do_freeze))
{
/*
* The snapshotConflictHorizon for the whole record should be the
@@ -882,35 +1018,56 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* on the standby with xids older than the youngest tuple this
* record will freeze will conflict.
*/
- TransactionId frz_conflict_horizon = InvalidTransactionId;
- TransactionId conflict_xid;
+ TransactionId conflict_xid = InvalidTransactionId;
+
+ /*
+ * If we are updating the VM, the conflict horizon is almost
+ * always the visibility cutoff XID.
+ *
+ * Separately, if we are freezing any tuples, as an optimization,
+ * we can use the visibility_cutoff_xid as the conflict horizon if
+ * the page will be all-frozen. This is true even if there are
+ * LP_DEAD line pointers because we ignored those when maintaining
+ * the visibility_cutoff_xid.
+ */
+ if (do_set_vm || (do_freeze && all_frozen_except_lp_dead))
+ conflict_xid = prstate.visibility_cutoff_xid;
/*
- * We can use the visibility_cutoff_xid as our cutoff for
- * conflicts when the whole page is eligible to become all-frozen
- * in the VM once we're done with it. Otherwise we generate a
- * conservative cutoff by stepping back from OldestXmin.
+ * Otherwise, if we are freezing but the page would not be
+ * all-frozen, we have to use the more pessimistic horizon of
+ * OldestXmin, which may be newer than the newest tuple we froze.
+ * We currently don't track the newest tuple we froze.
*/
- if (do_freeze)
+ else if (do_freeze)
{
- if (prstate.all_visible && prstate.all_frozen)
- frz_conflict_horizon = prstate.visibility_cutoff_xid;
- else
- {
- /* Avoids false conflicts when hot_standby_feedback in use */
- frz_conflict_horizon = prstate.cutoffs->OldestXmin;
- TransactionIdRetreat(frz_conflict_horizon);
- }
+ conflict_xid = prstate.cutoffs->OldestXmin;
+ TransactionIdRetreat(conflict_xid);
}
- if (TransactionIdFollows(frz_conflict_horizon, prstate.latest_xid_removed))
- conflict_xid = frz_conflict_horizon;
- else
+ /*
+ * If we are removing tuples with a younger xmax than our so far
+ * calculated conflict_xid, we must use this as our horizon.
+ */
+ if (TransactionIdFollows(prstate.latest_xid_removed, conflict_xid))
conflict_xid = prstate.latest_xid_removed;
+ /*
+ * We can omit the snapshot conflict horizon if we are not pruning
+ * or freezing any tuples and are setting an already all-visible
+ * page all-frozen in the VM. In this case, all of the tuples on
+ * the page must already be visible to all MVCC snapshots on the
+ * standby.
+ */
+ if (!do_prune && !do_freeze && do_set_vm &&
+ blk_known_av && (vmflags & VISIBILITYMAP_ALL_FROZEN))
+ conflict_xid = InvalidTransactionId;
+
log_heap_prune_and_freeze(relation, buffer,
false,
- InvalidBuffer, 0, false,
+ vmbuffer,
+ vmflags,
+ set_pd_all_visible,
conflict_xid,
true, reason,
prstate.frozen, prstate.nfrozen,
@@ -922,124 +1079,55 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
END_CRIT_SECTION();
- /* Copy information back for caller */
- presult->ndeleted = prstate.ndeleted;
- presult->nnewlpdead = prstate.ndead;
- presult->nfrozen = prstate.nfrozen;
- presult->live_tuples = prstate.live_tuples;
- presult->recently_dead_tuples = prstate.recently_dead_tuples;
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on to
- * make the choice of whether or not to freeze the page unaffected by the
- * short-term presence of LP_DEAD items. These LP_DEAD items were
- * effectively assumed to be LP_UNUSED items in the making. It doesn't
- * matter which vacuum heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible if there are
- * any LP_DEAD items on the page. It needs to reflect the present state
- * of the page, as expected for updating the visibility map.
- */
- if (prstate.all_visible && prstate.lpdead_items == 0)
- {
- presult->all_visible = prstate.all_visible;
- presult->all_frozen = prstate.all_frozen;
- }
- else
- {
- presult->all_visible = false;
- presult->all_frozen = false;
- }
-
- presult->hastup = prstate.hastup;
+ if (do_set_vm)
+ LockBuffer(vmbuffer, BUFFER_LOCK_UNLOCK);
- /*
- * If updating the visibility map, the conflict horizon for that record
- * must be the newest xmin on the page. However, if the page is
- * completely frozen, there can be no conflict and the vm_conflict_horizon
- * should remain InvalidTransactionId. This includes the case that we
- * just froze all the tuples; the prune-freeze record included the
- * conflict XID already so the VM update record doesn't need it.
- */
- if (presult->all_frozen)
- presult->vm_conflict_horizon = InvalidTransactionId;
- else
- presult->vm_conflict_horizon = prstate.visibility_cutoff_xid;
+ Assert(!prstate.all_visible || (prstate.lpdead_items == 0));
/*
- * Handle setting visibility map bit based on information from the VM (as
- * of last heap_vac_scan_next_block() call), and from all_visible and
- * all_frozen variables.
+ * VACUUM will call heap_page_is_all_visible() during the second pass over
+ * the heap to determine all_visible and all_frozen for the page -- this
+ * is a specialized version of the logic from this function. Now that
+ * we've finished pruning and freezing, make sure that we're in total
+ * agreement with heap_page_is_all_visible() using an assertion. We will
+ * have already set the page in the VM, so this assertion will only let
+ * you know that you've already done something wrong.
*/
- if (options & HEAP_PAGE_PRUNE_UPDATE_VM)
+#ifdef USE_ASSERT_CHECKING
+ if (prstate.all_visible)
{
- if (identify_and_fix_vm_corruption(relation,
- blockno, buffer, page,
- blk_known_av,
- prstate.lpdead_items, vmbuffer))
- {
- /* If we fix corruption, don't update the VM further */
- }
+ TransactionId debug_cutoff;
+ bool debug_all_frozen;
- /*
- * If the page isn't yet marked all-visible in the VM or it is and
- * needs to me marked all-frozen, update the VM Note that all_frozen
- * is only valid if all_visible is true, so we must check both
- * all_visible and all_frozen.
- */
- else if (presult->all_visible &&
- (!blk_known_av ||
- (presult->all_frozen && !VM_ALL_FROZEN(relation, blockno, &vmbuffer))))
- {
- Assert(prstate.lpdead_items == 0);
- vmflags = VISIBILITYMAP_ALL_VISIBLE;
+ Assert(cutoffs);
- /*
- * If the page is all-frozen, we can pass InvalidTransactionId as
- * our cutoff_xid, since a snapshotConflictHorizon sufficient to
- * make everything safe for REDO was logged when the page's tuples
- * were frozen.
- */
- if (presult->all_frozen)
- {
- Assert(!TransactionIdIsValid(presult->vm_conflict_horizon));
- vmflags |= VISIBILITYMAP_ALL_FROZEN;
- }
+ Assert(prstate.lpdead_items == 0);
- /*
- * It's possible for the VM bit to be clear and the page-level bit
- * to be set if checksums are not enabled.
- *
- * And even if we are just planning to update the frozen bit in
- * the VM, we shouldn't rely 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.
- *
- * 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.
- */
- if (!PageIsAllVisible(page) || XLogHintBitIsNeeded())
- {
- PageSetAllVisible(page);
- MarkBufferDirty(buffer);
- }
+ if (!heap_page_is_all_visible(relation, buffer,
+ cutoffs->OldestXmin,
+ &debug_all_frozen,
+ &debug_cutoff, off_loc))
+ Assert(false);
- old_vmbits = visibilitymap_set(relation, blockno, buffer, InvalidXLogRecPtr,
- vmbuffer, presult->vm_conflict_horizon,
- vmflags);
- }
+ Assert(prstate.all_frozen == debug_all_frozen);
+
+ Assert(!TransactionIdIsValid(debug_cutoff) ||
+ debug_cutoff == prstate.visibility_cutoff_xid);
}
+#endif
+ /* Copy information back for caller */
+ presult->ndeleted = prstate.ndeleted;
+ presult->nnewlpdead = prstate.ndead;
+ presult->nfrozen = prstate.nfrozen;
+ presult->live_tuples = prstate.live_tuples;
+ presult->recently_dead_tuples = prstate.recently_dead_tuples;
+ presult->old_vmbits = old_vmbits;
+ /* new_vmbits was set above */
+ presult->hastup = prstate.hastup;
presult->lpdead_items = prstate.lpdead_items;
/* the presult->deadoffsets array was already filled in */
- presult->old_vmbits = old_vmbits;
- presult->new_vmbits = vmflags;
-
if (prstate.freeze)
{
if (presult->nfrozen > 0)
@@ -1621,7 +1709,12 @@ heap_prune_record_unchanged_lp_normal(Page page, PruneState *prstate, OffsetNumb
break;
}
- /* Consider freezing any normal tuples which will not be removed */
+ /*
+ * Consider freezing any normal tuples which will not be removed.
+ * Regardless of whether or not we want to freeze the tuples, if we want
+ * to update the VM, we have to call heap_prepare_freeze_tuple() on every
+ * tuple to know whether or not the page will be totally frozen.
+ */
if (prstate->freeze)
{
bool totally_frozen;
@@ -2184,7 +2277,7 @@ heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
* - Reaping: During vacuum phase III, items that are already LP_DEAD are
* marked as unused.
*
- * - VM updates: After vacuum phase III, the heap page may be marked
+ * - VM updates: After vacuum phases I and III, the heap page may be marked
* all-visible and all-frozen.
*
* These changes all happen together, so we use a singel WAL record for them
@@ -2238,6 +2331,7 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
OffsetNumber frz_offsets[MaxHeapTuplesPerPage];
bool do_prune = nredirected > 0 || ndead > 0 || nunused > 0;
+ Assert(do_prune || nfrozen > 0 || vmflags & VISIBILITYMAP_VALID_BITS);
Assert((vmflags & VISIBILITYMAP_VALID_BITS) == vmflags);
xlrec.flags = vmflags;
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 05d3d2a3267..75c10ba20c6 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2013,34 +2013,6 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->new_frozen_tuple_pages++;
}
- /*
- * VACUUM will call heap_page_is_all_visible() during the second pass over
- * the heap to determine all_visible and all_frozen for the page -- this
- * is a specialized version of the logic from this function. Now that
- * we've finished pruning and freezing, make sure that we're in total
- * agreement with heap_page_is_all_visible() using an assertion.
- */
-#ifdef USE_ASSERT_CHECKING
- /* Note that all_frozen value does not matter when !all_visible */
- if (presult.all_visible)
- {
- TransactionId debug_cutoff;
- bool debug_all_frozen;
-
- Assert(presult.lpdead_items == 0);
-
- if (!heap_page_is_all_visible(vacrel->rel, buf,
- vacrel->cutoffs.OldestXmin, &debug_all_frozen,
- &debug_cutoff, &vacrel->offnum))
- Assert(false);
-
- Assert(presult.all_frozen == debug_all_frozen);
-
- Assert(!TransactionIdIsValid(debug_cutoff) ||
- debug_cutoff == presult.vm_conflict_horizon);
- }
-#endif
-
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
@@ -2074,8 +2046,6 @@ lazy_scan_prune(LVRelState *vacrel,
/* Did we find LP_DEAD items? */
*has_lpdead_items = (presult.lpdead_items > 0);
- Assert(!presult.all_visible || !(*has_lpdead_items));
-
/*
* For the purposes of logging, count whether or not the page was newly
* set all-visible and, potentially, all-frozen.
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index b85648456e9..0b9bb1c9b13 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -235,19 +235,12 @@ typedef struct PruneFreezeResult
int recently_dead_tuples;
/*
- * all_visible and all_frozen indicate the status of the page as reflected
- * in the visibility map after pruning, freezing, and setting any pages
- * all-visible in the visibility map.
+ * old_vmbits are the state of the all-visible and all-frozen bits in the
+ * visibility map before updating it during phase I of vacuuming.
+ * new_vmbits are the state of those bits after phase I of vacuuming.
*
- * vm_conflict_horizon is the newest xmin of live tuples on the page
- * (older than OldestXmin). It will only be valid if we did not set the
- * page all-frozen in the VM.
- *
- * These are only set if the HEAP_PRUNE_FREEZE option is set.
+ * These are only set if the HEAP_PAGE_PRUNE_UPDATE_VM option is set.
*/
- bool all_visible;
- bool all_frozen;
- TransactionId vm_conflict_horizon;
uint8 old_vmbits;
uint8 new_vmbits;
--
2.43.0
[text/x-patch] v9-0017-Inline-TransactionIdFollows-Precedes.patch (4.9K, 18-v9-0017-Inline-TransactionIdFollows-Precedes.patch)
download | inline diff:
From 5be056f48478db42dc0ad09d480e091cd8c53ebe Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 29 Jul 2025 09:57:13 -0400
Subject: [PATCH v9 17/22] Inline TransactionIdFollows/Precedes()
Calling these from on-access pruning code had noticeable overhead in a
profile. There does not seem to be a reason not to inline them.
---
src/backend/access/transam/transam.c | 64 -------------------------
src/include/access/transam.h | 70 ++++++++++++++++++++++++++--
2 files changed, 66 insertions(+), 68 deletions(-)
diff --git a/src/backend/access/transam/transam.c b/src/backend/access/transam/transam.c
index 9a39451a29a..553d6756cb3 100644
--- a/src/backend/access/transam/transam.c
+++ b/src/backend/access/transam/transam.c
@@ -273,70 +273,6 @@ TransactionIdAbortTree(TransactionId xid, int nxids, TransactionId *xids)
TRANSACTION_STATUS_ABORTED, InvalidXLogRecPtr);
}
-/*
- * TransactionIdPrecedes --- is id1 logically < id2?
- */
-bool
-TransactionIdPrecedes(TransactionId id1, TransactionId id2)
-{
- /*
- * If either ID is a permanent XID then we can just do unsigned
- * comparison. If both are normal, do a modulo-2^32 comparison.
- */
- int32 diff;
-
- if (!TransactionIdIsNormal(id1) || !TransactionIdIsNormal(id2))
- return (id1 < id2);
-
- diff = (int32) (id1 - id2);
- return (diff < 0);
-}
-
-/*
- * TransactionIdPrecedesOrEquals --- is id1 logically <= id2?
- */
-bool
-TransactionIdPrecedesOrEquals(TransactionId id1, TransactionId id2)
-{
- int32 diff;
-
- if (!TransactionIdIsNormal(id1) || !TransactionIdIsNormal(id2))
- return (id1 <= id2);
-
- diff = (int32) (id1 - id2);
- return (diff <= 0);
-}
-
-/*
- * TransactionIdFollows --- is id1 logically > id2?
- */
-bool
-TransactionIdFollows(TransactionId id1, TransactionId id2)
-{
- int32 diff;
-
- if (!TransactionIdIsNormal(id1) || !TransactionIdIsNormal(id2))
- return (id1 > id2);
-
- diff = (int32) (id1 - id2);
- return (diff > 0);
-}
-
-/*
- * TransactionIdFollowsOrEquals --- is id1 logically >= id2?
- */
-bool
-TransactionIdFollowsOrEquals(TransactionId id1, TransactionId id2)
-{
- int32 diff;
-
- if (!TransactionIdIsNormal(id1) || !TransactionIdIsNormal(id2))
- return (id1 >= id2);
-
- diff = (int32) (id1 - id2);
- return (diff >= 0);
-}
-
/*
* TransactionIdLatest --- get latest XID among a main xact and its children
diff --git a/src/include/access/transam.h b/src/include/access/transam.h
index 7d82cd2eb56..c9e20418275 100644
--- a/src/include/access/transam.h
+++ b/src/include/access/transam.h
@@ -255,6 +255,72 @@ typedef struct TransamVariablesData
} TransamVariablesData;
+
+/*
+ * TransactionIdPrecedes --- is id1 logically < id2?
+ */
+static inline bool
+TransactionIdPrecedes(TransactionId id1, TransactionId id2)
+{
+ /*
+ * If either ID is a permanent XID then we can just do unsigned
+ * comparison. If both are normal, do a modulo-2^32 comparison.
+ */
+ int32 diff;
+
+ if (!TransactionIdIsNormal(id1) || !TransactionIdIsNormal(id2))
+ return (id1 < id2);
+
+ diff = (int32) (id1 - id2);
+ return (diff < 0);
+}
+
+/*
+ * TransactionIdPrecedesOrEquals --- is id1 logically <= id2?
+ */
+static inline bool
+TransactionIdPrecedesOrEquals(TransactionId id1, TransactionId id2)
+{
+ int32 diff;
+
+ if (!TransactionIdIsNormal(id1) || !TransactionIdIsNormal(id2))
+ return (id1 <= id2);
+
+ diff = (int32) (id1 - id2);
+ return (diff <= 0);
+}
+
+/*
+ * TransactionIdFollows --- is id1 logically > id2?
+ */
+static inline bool
+TransactionIdFollows(TransactionId id1, TransactionId id2)
+{
+ int32 diff;
+
+ if (!TransactionIdIsNormal(id1) || !TransactionIdIsNormal(id2))
+ return (id1 > id2);
+
+ diff = (int32) (id1 - id2);
+ return (diff > 0);
+}
+
+/*
+ * TransactionIdFollowsOrEquals --- is id1 logically >= id2?
+ */
+static inline bool
+TransactionIdFollowsOrEquals(TransactionId id1, TransactionId id2)
+{
+ int32 diff;
+
+ if (!TransactionIdIsNormal(id1) || !TransactionIdIsNormal(id2))
+ return (id1 >= id2);
+
+ diff = (int32) (id1 - id2);
+ return (diff >= 0);
+}
+
+
/* ----------------
* extern declarations
* ----------------
@@ -274,10 +340,6 @@ extern bool TransactionIdDidAbort(TransactionId transactionId);
extern void TransactionIdCommitTree(TransactionId xid, int nxids, TransactionId *xids);
extern void TransactionIdAsyncCommitTree(TransactionId xid, int nxids, TransactionId *xids, XLogRecPtr lsn);
extern void TransactionIdAbortTree(TransactionId xid, int nxids, TransactionId *xids);
-extern bool TransactionIdPrecedes(TransactionId id1, TransactionId id2);
-extern bool TransactionIdPrecedesOrEquals(TransactionId id1, TransactionId id2);
-extern bool TransactionIdFollows(TransactionId id1, TransactionId id2);
-extern bool TransactionIdFollowsOrEquals(TransactionId id1, TransactionId id2);
extern TransactionId TransactionIdLatest(TransactionId mainxid,
int nxids, const TransactionId *xids);
extern XLogRecPtr TransactionIdGetCommitLSN(TransactionId xid);
--
2.43.0
[text/x-patch] v9-0019-Allow-on-access-pruning-to-set-pages-all-visible.patch (27.3K, 19-v9-0019-Allow-on-access-pruning-to-set-pages-all-visible.patch)
download | inline diff:
From a83906def96db35ce75f93b3488ad64fc81b067f Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 29 Jul 2025 14:34:30 -0400
Subject: [PATCH v9 19/22] Allow on-access pruning to set pages all-visible
Many queries do not modify the underlying relation. For such queries, if
on-access pruning occurs during the scan, we can check whether the page
has become all-visible and update the visibility map accordingly.
Previously, only vacuum marked pages as all-visible or all-frozen.
Supporting this requires passing information about whether the relation
is modified from the executor down to the scan descriptor.
This commit implements on-access VM setting for sequential scans as well
as for the underlying heap relation in index scans and bitmap heap
scans.
---
src/backend/access/heap/heapam.c | 15 ++++-
src/backend/access/heap/heapam_handler.c | 15 ++++-
src/backend/access/heap/pruneheap.c | 67 ++++++++++++++-----
src/backend/access/index/indexam.c | 46 +++++++++++++
src/backend/access/table/tableam.c | 39 +++++++++--
src/backend/executor/execMain.c | 4 ++
src/backend/executor/execUtils.c | 2 +
src/backend/executor/nodeBitmapHeapscan.c | 7 +-
src/backend/executor/nodeIndexscan.c | 18 +++--
src/backend/executor/nodeSeqscan.c | 24 +++++--
src/include/access/genam.h | 11 +++
src/include/access/heapam.h | 24 ++++++-
src/include/access/relscan.h | 6 ++
src/include/access/tableam.h | 30 ++++++++-
src/include/nodes/execnodes.h | 6 ++
.../t/035_standby_logical_decoding.pl | 4 +-
16 files changed, 278 insertions(+), 40 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index cb16bb0cbbd..d07693b7075 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -556,6 +556,7 @@ heap_prepare_pagescan(TableScanDesc sscan)
Buffer buffer = scan->rs_cbuf;
BlockNumber block = scan->rs_cblock;
Snapshot snapshot;
+ Buffer *vmbuffer = NULL;
Page page;
int lines;
bool all_visible;
@@ -570,7 +571,9 @@ heap_prepare_pagescan(TableScanDesc sscan)
/*
* Prune and repair fragmentation for the whole page, if possible.
*/
- heap_page_prune_opt(scan->rs_base.rs_rd, buffer);
+ if (sscan->rs_flags & SO_ALLOW_VM_SET)
+ vmbuffer = &scan->rs_vmbuffer;
+ heap_page_prune_opt(scan->rs_base.rs_rd, buffer, vmbuffer);
/*
* We must hold share lock on the buffer content while examining tuple
@@ -1247,6 +1250,7 @@ heap_beginscan(Relation relation, Snapshot snapshot,
sizeof(TBMIterateResult));
}
+ scan->rs_vmbuffer = InvalidBuffer;
return (TableScanDesc) scan;
}
@@ -1285,6 +1289,12 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
scan->rs_cbuf = InvalidBuffer;
}
+ if (BufferIsValid(scan->rs_vmbuffer))
+ {
+ ReleaseBuffer(scan->rs_vmbuffer);
+ scan->rs_vmbuffer = InvalidBuffer;
+ }
+
/*
* SO_TYPE_BITMAPSCAN would be cleaned up here, but it does not hold any
* additional data vs a normal HeapScan
@@ -1317,6 +1327,9 @@ heap_endscan(TableScanDesc sscan)
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
+ if (BufferIsValid(scan->rs_vmbuffer))
+ ReleaseBuffer(scan->rs_vmbuffer);
+
/*
* Must free the read stream before freeing the BufferAccessStrategy.
*/
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index bcbac844bb6..f05b9e4968d 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -85,6 +85,7 @@ heapam_index_fetch_begin(Relation rel)
hscan->xs_base.rel = rel;
hscan->xs_cbuf = InvalidBuffer;
+ hscan->xs_vmbuffer = InvalidBuffer;
return &hscan->xs_base;
}
@@ -99,6 +100,12 @@ heapam_index_fetch_reset(IndexFetchTableData *scan)
ReleaseBuffer(hscan->xs_cbuf);
hscan->xs_cbuf = InvalidBuffer;
}
+
+ if (BufferIsValid(hscan->xs_vmbuffer))
+ {
+ ReleaseBuffer(hscan->xs_vmbuffer);
+ hscan->xs_vmbuffer = InvalidBuffer;
+ }
}
static void
@@ -138,7 +145,8 @@ heapam_index_fetch_tuple(struct IndexFetchTableData *scan,
* Prune page, but only if we weren't already on this page
*/
if (prev_buf != hscan->xs_cbuf)
- heap_page_prune_opt(hscan->xs_base.rel, hscan->xs_cbuf);
+ heap_page_prune_opt(hscan->xs_base.rel, hscan->xs_cbuf,
+ scan->modifies_base_rel ? NULL : &hscan->xs_vmbuffer);
}
/* Obtain share-lock on the buffer so we can examine visibility */
@@ -2471,6 +2479,7 @@ BitmapHeapScanNextBlock(TableScanDesc scan,
TBMIterateResult *tbmres;
OffsetNumber offsets[TBM_MAX_TUPLES_PER_PAGE];
int noffsets = -1;
+ Buffer *vmbuffer = NULL;
Assert(scan->rs_flags & SO_TYPE_BITMAPSCAN);
Assert(hscan->rs_read_stream);
@@ -2517,7 +2526,9 @@ BitmapHeapScanNextBlock(TableScanDesc scan,
/*
* Prune and repair fragmentation for the whole page, if possible.
*/
- heap_page_prune_opt(scan->rs_rd, buffer);
+ if (scan->rs_flags & SO_ALLOW_VM_SET)
+ vmbuffer = &hscan->rs_vmbuffer;
+ heap_page_prune_opt(scan->rs_rd, buffer, vmbuffer);
/*
* We must hold share lock on the buffer content while examining tuple
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ba3faab91fd..4400bf583dd 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -198,9 +198,13 @@ static bool identify_and_fix_vm_corruption(Relation relation,
* if there's not any use in pruning.
*
* Caller must have pin on the buffer, and must *not* have a lock on it.
+ *
+ * If vmbuffer is not NULL, it is okay for pruning to set the visibility map if
+ * the page is all visible. We will take care of pinning and, if needed,
+ * reading in the page of the visibility map.
*/
void
-heap_page_prune_opt(Relation relation, Buffer buffer)
+heap_page_prune_opt(Relation relation, Buffer buffer, Buffer *vmbuffer)
{
Page page = BufferGetPage(buffer);
TransactionId prune_xid;
@@ -264,6 +268,13 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
{
OffsetNumber dummy_off_loc;
PruneFreezeResult presult;
+ int options = 0;
+
+ if (vmbuffer)
+ {
+ visibilitymap_pin(relation, BufferGetBlockNumber(buffer), vmbuffer);
+ options = HEAP_PAGE_PRUNE_UPDATE_VM;
+ }
/*
* For now, pass mark_unused_now as false regardless of whether or
@@ -271,9 +282,10 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
* that during on-access pruning with the current implementation.
*/
heap_page_prune_and_freeze(relation, buffer, false,
- InvalidBuffer,
- vistest, 0,
- NULL, &presult, PRUNE_ON_ACCESS, &dummy_off_loc, NULL, NULL);
+ vmbuffer ? *vmbuffer : InvalidBuffer,
+ vistest, options,
+ NULL, &presult, PRUNE_ON_ACCESS,
+ &dummy_off_loc, NULL, NULL);
/*
* Report the number of tuples reclaimed to pgstats. This is
@@ -513,12 +525,17 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* all-frozen for use in opportunistic freezing and to update the VM if
* the caller requests it.
*
- * Currently, only VACUUM attempts freezing and setting the VM bits. But
- * other callers could do either one. The visibility bookkeeping is
- * required for opportunistic freezing (in addition to setting the VM
- * bits) because we only consider opportunistically freezing tuples if the
- * whole page would become all-frozen or if the whole page will be frozen
- * except for dead tuples that will be removed by vacuum.
+ * Currently, only VACUUM attempts freezing. But other callers could. The
+ * visibility bookkeeping is required for opportunistic freezing (in
+ * addition to setting the VM bits) because we only consider
+ * opportunistically freezing tuples if the whole page would become
+ * all-frozen or if the whole page will be frozen except for dead tuples
+ * that will be removed by vacuum. But if consider_update_vm is false,
+ * we'll not set the VM even if the page is discovered to be all-visible.
+ *
+ * If only HEAP_PAGE_PRUNE_UPDATE_VM is passed and not
+ * HEAP_PAGE_PRUNE_FREEZE, prstate.all_frozen must be initialized to false
+ * because we will not call heap_prepare_freeze_tuple() on each tuple.
*
* If only updating the VM, we must initialize all_frozen to false, as
* heap_prepare_freeze_tuple() will not be called for each tuple on the
@@ -530,7 +547,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* whether or not to freeze but before deciding whether or not to update
* the VM so that we don't set the VM bit incorrectly.
*
- * If not freezing or updating the VM, we otherwise avoid the extra
+ * If not freezing and not updating the VM, we avoid the extra
* bookkeeping. Initializing all_visible to false allows skipping the work
* to update them in heap_prune_record_unchanged_lp_normal().
*/
@@ -879,12 +896,30 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.all_frozen = false;
}
+ /*
+ * If this is an on-access call and we're not actually pruning, avoid
+ * setting the visibility map if it would newly dirty the heap page or, if
+ * the page is already dirty, if doing so would require including a
+ * full-page image (FPI) of the heap page in the WAL. This situation
+ * should be rare, as on-access pruning is only attempted when
+ * pd_prune_xid is valid.
+ */
+ if (reason == PRUNE_ON_ACCESS &&
+ prstate.consider_update_vm &&
+ prstate.all_visible &&
+ !do_prune && !do_freeze &&
+ (!BufferIsDirty(buffer) || XLogCheckBufferNeedsBackup(buffer)))
+ {
+ prstate.consider_update_vm = false;
+ prstate.all_visible = prstate.all_frozen = false;
+ }
+
Assert(!prstate.all_frozen || prstate.all_visible);
/*
- * Handle setting visibility map bit based on information from the VM (as
- * of last heap_vac_scan_next_block() call), and from all_visible and
- * all_frozen variables.
+ * Handle setting visibility map bit based on information from the VM (if
+ * provided, e.g. by vacuum from the last heap_vac_scan_next_block()
+ * call), and from all_visible and all_frozen variables.
*/
if (prstate.consider_update_vm)
{
@@ -2275,8 +2310,8 @@ heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
* - Reaping: During vacuum phase III, items that are already LP_DEAD are
* marked as unused.
*
- * - VM updates: After vacuum phases I and III, the heap page may be marked
- * all-visible and all-frozen.
+ * - VM updates: After vacuum phases I and III and on-access, the heap page
+ * may be marked all-visible and all-frozen.
*
* These changes all happen together, so we use a singel WAL record for them
* all.
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 86d11f4ec79..4603ece09bd 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -289,6 +289,32 @@ index_beginscan(Relation heapRelation,
return scan;
}
+/*
+ * Similar to index_beginscan(), but allows the caller to indicate whether the
+ * query modifies the underlying base relation. This is used when the caller
+ * wants to attempt marking pages in the base relation as all-visible in the
+ * visibility map during on-access pruning.
+ */
+IndexScanDesc
+index_beginscan_vmset(Relation heapRelation,
+ Relation indexRelation,
+ Snapshot snapshot,
+ IndexScanInstrumentation *instrument,
+ int nkeys, int norderbys, bool modifies_base_rel)
+{
+ IndexScanDesc scan;
+
+ scan = index_beginscan(heapRelation,
+ indexRelation,
+ snapshot,
+ instrument,
+ nkeys, norderbys);
+
+ scan->xs_heapfetch->modifies_base_rel = modifies_base_rel;
+
+ return scan;
+}
+
/*
* index_beginscan_bitmap - start a scan of an index with amgetbitmap
*
@@ -620,6 +646,26 @@ index_beginscan_parallel(Relation heaprel, Relation indexrel,
return scan;
}
+/*
+ * Parallel version of index_beginscan_vmset()
+ */
+IndexScanDesc
+index_beginscan_parallel_vmset(Relation heaprel, Relation indexrel,
+ IndexScanInstrumentation *instrument,
+ int nkeys, int norderbys,
+ ParallelIndexScanDesc pscan,
+ bool modifies_base_rel)
+{
+ IndexScanDesc scan;
+
+ scan = index_beginscan_parallel(heaprel, indexrel,
+ instrument,
+ nkeys, norderbys,
+ pscan);
+ scan->xs_heapfetch->modifies_base_rel = modifies_base_rel;
+ return scan;
+}
+
/* ----------------
* index_getnext_tid - get the next TID from a scan
*
diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index a56c5eceb14..67dbf99f5b5 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -49,6 +49,10 @@
char *default_table_access_method = DEFAULT_TABLE_ACCESS_METHOD;
bool synchronize_seqscans = true;
+/* Helper for table_beginscan_parallel() and table_beginscan_parallel_vmset() */
+static TableScanDesc table_beginscan_parallel_common(Relation relation, ParallelTableScanDesc pscan,
+ uint32 flags);
+
/* ----------------------------------------------------------------------------
* Slot functions.
@@ -162,12 +166,14 @@ table_parallelscan_initialize(Relation rel, ParallelTableScanDesc pscan,
}
}
-TableScanDesc
-table_beginscan_parallel(Relation relation, ParallelTableScanDesc pscan)
+/*
+ * Common helper for table_beginscan_parallel() and table_beginscan_parallel_vmset()
+ */
+static TableScanDesc
+table_beginscan_parallel_common(Relation relation, ParallelTableScanDesc pscan,
+ uint32 flags)
{
Snapshot snapshot;
- uint32 flags = SO_TYPE_SEQSCAN |
- SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE;
Assert(RelFileLocatorEquals(relation->rd_locator, pscan->phs_locator));
@@ -188,6 +194,31 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc pscan)
pscan, flags);
}
+TableScanDesc
+table_beginscan_parallel(Relation relation, ParallelTableScanDesc pscan)
+{
+ uint32 flags = SO_TYPE_SEQSCAN |
+ SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE;
+
+ return table_beginscan_parallel_common(relation, pscan, flags);
+}
+
+/*
+ * Parallel version of table_beginscan_vmset()
+ */
+TableScanDesc
+table_beginscan_parallel_vmset(Relation relation, ParallelTableScanDesc pscan,
+ bool modifies_rel)
+{
+ uint32 flags = SO_TYPE_SEQSCAN |
+ SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE;
+
+ if (!modifies_rel)
+ flags |= SO_ALLOW_VM_SET;
+
+ return table_beginscan_parallel_common(relation, pscan, flags);
+}
+
/* ----------------------------------------------------------------------------
* Index scan related functions.
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index ff12e2e1364..2e0474c948a 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -916,6 +916,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
break;
}
+ /* If it has a rowmark, the relation is modified */
+ estate->es_modified_relids = bms_add_member(estate->es_modified_relids,
+ rc->rti);
+
/* Check that relation is a legal target for marking */
if (relation)
CheckValidRowMarkRel(relation, rc->markType);
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index fdc65c2b42b..28a06dcd244 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -893,6 +893,8 @@ ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
estate->es_result_relations = (ResultRelInfo **)
palloc0(estate->es_range_table_size * sizeof(ResultRelInfo *));
estate->es_result_relations[rti - 1] = resultRelInfo;
+ estate->es_modified_relids = bms_add_member(estate->es_modified_relids,
+ rti);
/*
* Saving in the list allows to avoid needlessly traversing the whole
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index bf24f3d7fe0..af6db9f7919 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -105,11 +105,16 @@ BitmapTableScanSetup(BitmapHeapScanState *node)
*/
if (!node->ss.ss_currentScanDesc)
{
+ bool modifies_rel =
+ bms_is_member(((Scan *) node->ss.ps.plan)->scanrelid,
+ node->ss.ps.state->es_modified_relids);
+
node->ss.ss_currentScanDesc =
table_beginscan_bm(node->ss.ss_currentRelation,
node->ss.ps.state->es_snapshot,
0,
- NULL);
+ NULL,
+ modifies_rel);
}
node->ss.ss_currentScanDesc->st.rs_tbmiterator = tbmiterator;
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 7fcaa37fe62..c2ffbd3b08e 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -102,16 +102,22 @@ IndexNext(IndexScanState *node)
if (scandesc == NULL)
{
+
+ bool modifies_base_rel =
+ bms_is_member(((Scan *) node->ss.ps.plan)->scanrelid,
+ estate->es_modified_relids);
+
/*
* We reach here if the index scan is not parallel, or if we're
* serially executing an index scan that was planned to be parallel.
*/
- scandesc = index_beginscan(node->ss.ss_currentRelation,
- node->iss_RelationDesc,
- estate->es_snapshot,
- &node->iss_Instrument,
- node->iss_NumScanKeys,
- node->iss_NumOrderByKeys);
+ scandesc = index_beginscan_vmset(node->ss.ss_currentRelation,
+ node->iss_RelationDesc,
+ estate->es_snapshot,
+ &node->iss_Instrument,
+ node->iss_NumScanKeys,
+ node->iss_NumOrderByKeys,
+ modifies_base_rel);
node->iss_ScanDesc = scandesc;
diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index 94047d29430..fd69275c181 100644
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -65,13 +65,18 @@ SeqNext(SeqScanState *node)
if (scandesc == NULL)
{
+ bool modifies_rel =
+ bms_is_member(((Scan *) node->ss.ps.plan)->scanrelid,
+ estate->es_modified_relids);
+
/*
* We reach here if the scan is not parallel, or if we're serially
* executing a scan that was planned to be parallel.
*/
- scandesc = table_beginscan(node->ss.ss_currentRelation,
- estate->es_snapshot,
- 0, NULL);
+ scandesc = table_beginscan_vmset(node->ss.ss_currentRelation,
+ estate->es_snapshot,
+ 0, NULL, modifies_rel);
+
node->ss.ss_currentScanDesc = scandesc;
}
@@ -366,6 +371,7 @@ ExecSeqScanInitializeDSM(SeqScanState *node,
ParallelContext *pcxt)
{
EState *estate = node->ss.ps.state;
+ bool modifies_rel;
ParallelTableScanDesc pscan;
pscan = shm_toc_allocate(pcxt->toc, node->pscan_len);
@@ -373,8 +379,11 @@ ExecSeqScanInitializeDSM(SeqScanState *node,
pscan,
estate->es_snapshot);
shm_toc_insert(pcxt->toc, node->ss.ps.plan->plan_node_id, pscan);
+ modifies_rel = bms_is_member(((Scan *) node->ss.ps.plan)->scanrelid,
+ estate->es_modified_relids);
node->ss.ss_currentScanDesc =
- table_beginscan_parallel(node->ss.ss_currentRelation, pscan);
+ table_beginscan_parallel_vmset(node->ss.ss_currentRelation, pscan,
+ modifies_rel);
}
/* ----------------------------------------------------------------
@@ -404,8 +413,13 @@ ExecSeqScanInitializeWorker(SeqScanState *node,
ParallelWorkerContext *pwcxt)
{
ParallelTableScanDesc pscan;
+ bool modifies_rel =
+ bms_is_member(((Scan *) node->ss.ps.plan)->scanrelid,
+ node->ss.ps.state->es_modified_relids);
pscan = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false);
node->ss.ss_currentScanDesc =
- table_beginscan_parallel(node->ss.ss_currentRelation, pscan);
+ table_beginscan_parallel_vmset(node->ss.ss_currentRelation,
+ pscan,
+ modifies_rel);
}
diff --git a/src/include/access/genam.h b/src/include/access/genam.h
index 5b2ab181b5f..bf272c2c37f 100644
--- a/src/include/access/genam.h
+++ b/src/include/access/genam.h
@@ -180,6 +180,11 @@ extern IndexScanDesc index_beginscan(Relation heapRelation,
Snapshot snapshot,
IndexScanInstrumentation *instrument,
int nkeys, int norderbys);
+extern IndexScanDesc index_beginscan_vmset(Relation heapRelation,
+ Relation indexRelation,
+ Snapshot snapshot,
+ IndexScanInstrumentation *instrument,
+ int nkeys, int norderbys, bool modifies_heap_rel);
extern IndexScanDesc index_beginscan_bitmap(Relation indexRelation,
Snapshot snapshot,
IndexScanInstrumentation *instrument,
@@ -206,6 +211,12 @@ extern IndexScanDesc index_beginscan_parallel(Relation heaprel,
IndexScanInstrumentation *instrument,
int nkeys, int norderbys,
ParallelIndexScanDesc pscan);
+
+extern IndexScanDesc index_beginscan_parallel_vmset(Relation heaprel, Relation indexrel,
+ IndexScanInstrumentation *instrument,
+ int nkeys, int norderbys,
+ ParallelIndexScanDesc pscan,
+ bool modifies_rel);
extern ItemPointer index_getnext_tid(IndexScanDesc scan,
ScanDirection direction);
struct TupleTableSlot;
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 4278f351bdf..16f7904a21e 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -94,6 +94,13 @@ typedef struct HeapScanDescData
*/
ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
+ /*
+ * For sequential scans and bitmap heap scans. If the relation is not
+ * being modified, on-access pruning may read in the current heap page's
+ * corresponding VM block to this buffer.
+ */
+ Buffer rs_vmbuffer;
+
/* these fields only used in page-at-a-time mode and for bitmap scans */
uint32 rs_cindex; /* current tuple's index in vistuples */
uint32 rs_ntuples; /* number of visible tuples on page */
@@ -116,8 +123,18 @@ typedef struct IndexFetchHeapData
{
IndexFetchTableData xs_base; /* AM independent part of the descriptor */
- Buffer xs_cbuf; /* current heap buffer in scan, if any */
- /* NB: if xs_cbuf is not InvalidBuffer, we hold a pin on that buffer */
+ /*
+ * Current heap buffer in scan, if any. NB: if xs_cbuf is not
+ * InvalidBuffer, we hold a pin on that buffer.
+ */
+ Buffer xs_cbuf;
+
+ /*
+ * For index scans that do not modify the underlying heap table, on-access
+ * pruning may read in the current heap page's corresponding VM block to
+ * this buffer.
+ */
+ Buffer xs_vmbuffer;
} IndexFetchHeapData;
/* Result codes for HeapTupleSatisfiesVacuum */
@@ -374,7 +391,8 @@ extern TransactionId heap_index_delete_tuples(Relation rel,
/* in heap/pruneheap.c */
struct GlobalVisState;
-extern void heap_page_prune_opt(Relation relation, Buffer buffer);
+extern void heap_page_prune_opt(Relation relation, Buffer buffer,
+ Buffer *vmbuffer);
extern void heap_page_prune_and_freeze(Relation relation, Buffer buffer,
bool blk_known_av,
Buffer vmbuffer,
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h
index b5e0fb386c0..f496e0b4939 100644
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -121,6 +121,12 @@ typedef struct ParallelBlockTableScanWorkerData *ParallelBlockTableScanWorker;
typedef struct IndexFetchTableData
{
Relation rel;
+
+ /*
+ * Some optimizations can only be performed if the query does not modify
+ * the underlying relation. Track that here.
+ */
+ bool modifies_base_rel;
} IndexFetchTableData;
struct IndexScanInstrumentation;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index b2ce35e2a34..e31c21cf8eb 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -62,6 +62,8 @@ typedef enum ScanOptions
/* unregister snapshot at scan end? */
SO_TEMP_SNAPSHOT = 1 << 9,
+ /* whether or not scan should attempt to set the VM */
+ SO_ALLOW_VM_SET = 1 << 10,
} ScanOptions;
/*
@@ -881,6 +883,25 @@ table_beginscan(Relation rel, Snapshot snapshot,
return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
}
+/*
+ * Similar to table_beginscan(), but allows the caller to indicate whether the
+ * query modifies the relation. This is used when the caller wants to attempt
+ * marking pages in the relation as all-visible in the visibility map during
+ * on-access pruning.
+ */
+static inline TableScanDesc
+table_beginscan_vmset(Relation rel, Snapshot snapshot,
+ int nkeys, struct ScanKeyData *key, bool modifies_rel)
+{
+ uint32 flags = SO_TYPE_SEQSCAN |
+ SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE;
+
+ if (!modifies_rel)
+ flags |= SO_ALLOW_VM_SET;
+
+ return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
+}
+
/*
* Like table_beginscan(), but for scanning catalog. It'll automatically use a
* snapshot appropriate for scanning catalog relations.
@@ -918,10 +939,13 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
*/
static inline TableScanDesc
table_beginscan_bm(Relation rel, Snapshot snapshot,
- int nkeys, struct ScanKeyData *key)
+ int nkeys, struct ScanKeyData *key, bool modifies_rel)
{
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE;
+ if (!modifies_rel)
+ flags |= SO_ALLOW_VM_SET;
+
return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key,
NULL, flags);
}
@@ -1130,6 +1154,10 @@ extern void table_parallelscan_initialize(Relation rel,
extern TableScanDesc table_beginscan_parallel(Relation relation,
ParallelTableScanDesc pscan);
+extern TableScanDesc table_beginscan_parallel_vmset(Relation relation,
+ ParallelTableScanDesc pscan,
+ bool modifies_rel);
+
/*
* Restart a parallel scan. Call this in the leader process. Caller is
* responsible for making sure that all workers have finished the scan
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index de782014b2d..839c1be1d7c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -678,6 +678,12 @@ typedef struct EState
* ExecDoInitialPruning() */
const char *es_sourceText; /* Source text from QueryDesc */
+ /*
+ * RT indexes of relations modified by the query either through
+ * UPDATE/DELETE/INSERT/MERGE or SELECT FOR UPDATE
+ */
+ Bitmapset *es_modified_relids;
+
JunkFilter *es_junkFilter; /* top-level junk filter, if any */
/* If query can insert/delete tuples, the command ID to mark them with */
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index c9c182892cf..870f03bdd79 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -10,6 +10,7 @@ use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Time::HiRes qw(usleep);
use Test::More;
+use Time::HiRes qw(usleep);
if ($ENV{enable_injection_points} ne 'yes')
{
@@ -296,6 +297,7 @@ wal_level = 'logical'
max_replication_slots = 4
max_wal_senders = 4
autovacuum = off
+hot_standby_feedback = on
});
$node_primary->dump_info;
$node_primary->start;
@@ -745,7 +747,7 @@ check_pg_recvlogical_stderr($handle,
$logstart = -s $node_standby->logfile;
reactive_slots_change_hfs_and_wait_for_xmins('shared_row_removal_',
- 'no_conflict_', 0, 1);
+ 'no_conflict_', 1, 0);
# This should not trigger a conflict
wait_until_vacuum_can_remove(
--
2.43.0
[text/x-patch] v9-0018-Unset-all-visible-sooner-if-not-freezing.patch (2.5K, 20-v9-0018-Unset-all-visible-sooner-if-not-freezing.patch)
download | inline diff:
From 2dc6f7ada64352284c96e5f0d069913a6f1f6eef Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 29 Jul 2025 14:35:13 -0400
Subject: [PATCH v9 18/22] Unset all-visible sooner if not freezing
In prune/freeze code, we delay unsetting all-visible/all-frozen in the
presence of dead items to allow opportunistically freezing tuples if the
whole page would be frozen except for those dead items -- which are
removed later in vacuum's third phase.
Future commits will allow on-access pruning to set the VM, which means
all-visible will be initialized to true instead of false and we will do
extra bookkeeping in heap_prune_unchanged_lp_normal() to keep track of
whether or not the page is all-visible.
Because on-access pruning will not freeze tuples, it makes sense to
unset all-visible as soon as we encounter an LP_DEAD item and
avoid continued bookkeeping since we know the page is not all-visible
and we won't be able to remove those dead items.
---
src/backend/access/heap/pruneheap.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 273e9412a01..ba3faab91fd 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -1494,8 +1494,11 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
/*
* Deliberately delay unsetting all_visible until later during pruning.
- * Removable dead tuples shouldn't preclude freezing the page.
+ * Removable dead tuples shouldn't preclude freezing the page. If we won't
+ * attempt freezing, just unset all-visible now, though.
*/
+ if (!prstate->attempt_freeze)
+ prstate->all_visible = prstate->all_frozen = false;
/* Record the dead offset for vacuum */
prstate->deadoffsets[prstate->lpdead_items++] = offnum;
@@ -1753,8 +1756,11 @@ heap_prune_record_unchanged_lp_dead(Page page, PruneState *prstate, OffsetNumber
* Similarly, don't unset all_visible until later, at the end of
* heap_page_prune_and_freeze(). This will allow us to attempt to freeze
* the page after pruning. As long as we unset it before updating the
- * visibility map, this will be correct.
+ * visibility map, this will be correct. If we won't attempt freezing,
+ * though, just unset all-visible now.
*/
+ if (!prstate->attempt_freeze)
+ prstate->all_visible = prstate->all_frozen = false;
/* Record the dead offset for vacuum */
prstate->deadoffsets[prstate->lpdead_items++] = offnum;
--
2.43.0
[text/x-patch] v9-0021-Reorder-heap_page_prune_and_freeze-parameters.patch (5.8K, 21-v9-0021-Reorder-heap_page_prune_and_freeze-parameters.patch)
download | inline diff:
From fd56683e500e528ee9da99a7326368aca8cb8bac Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 31 Jul 2025 12:08:18 -0400
Subject: [PATCH v9 21/22] Reorder heap_page_prune_and_freeze parameters
Reorder parameters so that all of the output parameters are together at
the end of the parameter list.
---
src/backend/access/heap/pruneheap.c | 38 ++++++++++++++--------------
src/backend/access/heap/vacuumlazy.c | 6 ++---
src/include/access/heapam.h | 4 +--
3 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index b9f85d1452e..53cb81d2510 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -297,10 +297,10 @@ heap_page_prune_opt(Relation relation, Buffer buffer, Buffer *vmbuffer)
* not the relation has indexes, since we cannot safely determine
* that during on-access pruning with the current implementation.
*/
- heap_page_prune_and_freeze(relation, buffer, false,
+ heap_page_prune_and_freeze(relation, buffer, options, false,
vmbuffer ? *vmbuffer : InvalidBuffer,
- vistest, options,
- NULL, &presult, PRUNE_ON_ACCESS,
+ vistest,
+ NULL, PRUNE_ON_ACCESS, &presult,
&dummy_off_loc, NULL, NULL);
/*
@@ -645,6 +645,15 @@ heap_page_will_freeze(Relation relation, Buffer buffer,
* also need to account for a reduction in the length of the line pointer
* array following array truncation by us.
*
+ * options:
+ * MARK_UNUSED_NOW indicates that dead items can be set LP_UNUSED during
+ * pruning.
+ *
+ * FREEZE indicates that we will also freeze tuples, and will return
+ * 'all_visible', 'all_frozen' flags to the caller.
+ *
+ * UPDATE_VM indicates that we will set the page's status in the VM.
+ *
* If the HEAP_PRUNE_FREEZE option is set, we will also freeze tuples if it's
* required in order to advance relfrozenxid / relminmxid, or if it's
* considered advantageous for overall system performance to do so now. The
@@ -663,30 +672,21 @@ heap_page_will_freeze(Relation relation, Buffer buffer,
* contain the required block of the visibility map.
*
* vistest is used to distinguish whether tuples are DEAD or RECENTLY_DEAD
- * (see heap_prune_satisfies_vacuum).
- *
- * options:
- * MARK_UNUSED_NOW indicates that dead items can be set LP_UNUSED during
- * pruning.
- *
- * FREEZE indicates that we will also freeze tuples, and will return
- * 'all_visible', 'all_frozen' flags to the caller.
- *
- * UPDATE_VM indicates that we will set the page's status in the VM.
+ * (see heap_prune_satisfies_vacuum). It is an input parameter.
*
* cutoffs contains the freeze cutoffs, established by VACUUM at the beginning
* of vacuuming the relation. Required if HEAP_PRUNE_FREEZE option is set.
* cutoffs->OldestXmin is also used to determine if dead tuples are
- * HEAPTUPLE_RECENTLY_DEAD or HEAPTUPLE_DEAD.
+ * HEAPTUPLE_RECENTLY_DEAD or HEAPTUPLE_DEAD. It is an input parameter.
+ *
+ * reason indicates why the pruning is performed. It is included in the WAL
+ * record for debugging and analysis purposes, but otherwise has no effect.
*
* presult contains output parameters needed by callers, such as the number of
* tuples removed and the offsets of dead items on the page after pruning.
* heap_page_prune_and_freeze() is responsible for initializing it. Required
* by all callers.
*
- * reason indicates why the pruning is performed. It is included in the WAL
- * record for debugging and analysis purposes, but otherwise has no effect.
- *
* off_loc is the offset location required by the caller to use in error
* callback.
*
@@ -699,13 +699,13 @@ heap_page_will_freeze(Relation relation, Buffer buffer,
*/
void
heap_page_prune_and_freeze(Relation relation, Buffer buffer,
+ int options,
bool blk_known_av,
Buffer vmbuffer,
GlobalVisState *vistest,
- int options,
struct VacuumCutoffs *cutoffs,
- PruneFreezeResult *presult,
PruneReason reason,
+ PruneFreezeResult *presult,
OffsetNumber *off_loc,
TransactionId *new_relfrozen_xid,
MultiXactId *new_relmin_mxid)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 7558ac697f1..99b9cab0974 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1991,11 +1991,11 @@ lazy_scan_prune(LVRelState *vacrel,
if (vacrel->nindexes == 0)
prune_options |= HEAP_PAGE_PRUNE_MARK_UNUSED_NOW;
- heap_page_prune_and_freeze(rel, buf,
+ heap_page_prune_and_freeze(rel, buf, prune_options,
all_visible_according_to_vm,
vmbuffer,
- vacrel->vistest, prune_options,
- &vacrel->cutoffs, &presult, PRUNE_VACUUM_SCAN,
+ vacrel->vistest,
+ &vacrel->cutoffs, PRUNE_VACUUM_SCAN, &presult,
&vacrel->offnum,
&vacrel->NewRelfrozenXid, &vacrel->NewRelminMxid);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 16f7904a21e..0c4e5607627 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -394,13 +394,13 @@ struct GlobalVisState;
extern void heap_page_prune_opt(Relation relation, Buffer buffer,
Buffer *vmbuffer);
extern void heap_page_prune_and_freeze(Relation relation, Buffer buffer,
+ int options,
bool blk_known_av,
Buffer vmbuffer,
struct GlobalVisState *vistest,
- int options,
struct VacuumCutoffs *cutoffs,
- PruneFreezeResult *presult,
PruneReason reason,
+ PruneFreezeResult *presult,
OffsetNumber *off_loc,
TransactionId *new_relfrozen_xid,
MultiXactId *new_relmin_mxid);
--
2.43.0
[text/x-patch] v9-0020-Add-helper-functions-to-heap_page_prune_and_freez.patch (18.9K, 22-v9-0020-Add-helper-functions-to-heap_page_prune_and_freez.patch)
download | inline diff:
From b4a28cf0ab6cd86be2abc4ff20ecf7e99ed13cf4 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 30 Jul 2025 18:51:43 -0400
Subject: [PATCH v9 20/22] Add helper functions to heap_page_prune_and_freeze
heap_page_prune_and_freeze() has gotten rather long. It has several
stages:
1) setup - where the PruneStateis set up
2) tuple examination -- where tuples and line pointers are examined to
determine what needs to be pruned and what could be frozen
3) evaluation -- where we determine based on caller provided options,
heuristics, and state gathered during stage 2 whether or not to
freeze tuples and set the page in the VM
4) execution - where the page changes are actually made and logged
This commit refactors the evaluation stage into helpers which return
whether or not to freeze and set the VM.
For the purposes of committing, this likely shouldn't be a separate
commit. But I'm not sure yet whether it makes more sense to do this
refactoring earlier in the set for clarity for the reviewer.
---
src/backend/access/heap/pruneheap.c | 471 +++++++++++++++++-----------
1 file changed, 295 insertions(+), 176 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 4400bf583dd..b9f85d1452e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -179,6 +179,22 @@ static void heap_prune_record_unchanged_lp_redirect(PruneState *prstate, OffsetN
static void page_verify_redirects(Page page);
+static bool heap_page_will_freeze(Relation relation, Buffer buffer,
+ bool do_prune,
+ bool do_hint_full_or_prunable,
+ bool did_tuple_hint_fpi,
+ PruneState *prstate,
+ bool *all_frozen_except_lp_dead);
+
+static bool heap_page_will_update_vm(Relation relation,
+ Buffer buffer, BlockNumber blockno, Page page,
+ PruneReason reason,
+ bool do_prune, bool do_freeze,
+ bool blk_known_av,
+ PruneState *prstate,
+ Buffer *vmbuffer, uint8 *vmflags,
+ bool *set_pd_all_visible);
+
static bool identify_and_fix_vm_corruption(Relation relation,
BlockNumber heap_blk,
Buffer heap_buffer, Page heap_page,
@@ -376,6 +392,249 @@ identify_and_fix_vm_corruption(Relation relation,
return false;
}
+
+/*
+ * Determine whether to set the visibility map bits based on information from
+ * the PruneState and blk_known_av, which some callers will provide after
+ * previously examining this heap page's VM bits (e.g. vacuum from the last
+ * heap_vac_scan_next_block() call).
+ *
+ * We pass in blockno and page even those can be derived from buffer to avoid
+ * extra BufferGetBlock() and BufferGetBlockNumber() calls.
+ *
+ * This should be called only after do_freeze has been decided (and do_prune
+ * has been set), as these factor into our heuristic-based decision.
+ *
+ * prstate and vmbuffer are input/output fields. vmflags and and
+ * set_pd_all_visible are output fields.
+ *
+ * Returns true if the caller should set one or both of the VM bits and false
+ * otherwise.
+ */
+static bool
+heap_page_will_update_vm(Relation relation,
+ Buffer buffer, BlockNumber blockno, Page page,
+ PruneReason reason,
+ bool do_prune, bool do_freeze,
+ bool blk_known_av,
+ PruneState *prstate,
+ Buffer *vmbuffer, uint8 *vmflags,
+ bool *set_pd_all_visible)
+{
+ bool do_set_vm = false;
+
+ /*
+ * If the caller specified not to update the VM, validate everything is in
+ * the right state and exit.
+ */
+ if (!prstate->consider_update_vm)
+ {
+ Assert(!prstate->all_visible && !prstate->all_frozen);
+ /* We don't set only the page level visibility hint */
+ Assert(!(*set_pd_all_visible));
+ Assert(*vmflags == 0);
+ return false;
+ }
+
+ /*
+ * If this is an on-access call and we're not actually pruning, avoid
+ * setting the visibility map if it would newly dirty the heap page or, if
+ * the page is already dirty, if doing so would require including a
+ * full-page image (FPI) of the heap page in the WAL. This situation
+ * should be rare, as on-access pruning is only attempted when
+ * pd_prune_xid is valid.
+ */
+ if (reason == PRUNE_ON_ACCESS &&
+ prstate->consider_update_vm &&
+ prstate->all_visible &&
+ !do_prune && !do_freeze &&
+ (!BufferIsDirty(buffer) || XLogCheckBufferNeedsBackup(buffer)))
+ {
+ prstate->consider_update_vm = false;
+ prstate->all_visible = prstate->all_frozen = false;
+ }
+
+ Assert(!prstate->all_frozen || prstate->all_visible);
+
+ /*
+ * Clear any VM corruption. This does not need to be in a critical
+ * section, so we do it first. If PD_ALL_VISIBLE is incorrectly set, we
+ * may mark the heap page buffer dirty here and could end up doing so
+ * again later. This is not a correctness issue and is in the path of VM
+ * corruption, so we don't have to worry about the extra performance
+ * overhead.
+ */
+ if (identify_and_fix_vm_corruption(relation,
+ blockno, buffer, page,
+ blk_known_av, prstate->lpdead_items,
+ *vmbuffer))
+ {
+ /* If we fix corruption, don't update the VM further */
+ }
+
+ /* Determine if we actually need to set the VM and which bits to set. */
+ else if (prstate->all_visible &&
+ (!blk_known_av ||
+ (prstate->all_frozen && !VM_ALL_FROZEN(relation, blockno, vmbuffer))))
+ {
+ *vmflags |= VISIBILITYMAP_ALL_VISIBLE;
+ if (prstate->all_frozen)
+ *vmflags |= VISIBILITYMAP_ALL_FROZEN;
+ }
+
+ do_set_vm = *vmflags & VISIBILITYMAP_VALID_BITS;
+
+ /*
+ * Don't set PD_ALL_VISIBLE unless we also plan to set the VM. While it is
+ * correct for a heap page to have PD_ALL_VISIBLE even if the VM is not
+ * set, we strongly prefer to keep them in sync.
+ *
+ * Prior to Postgres 19, it was possible for the page-level bit to be set
+ * and the VM bit to be clear. This could happen if we crashed after
+ * setting PD_ALL_VISIBLE but before setting bits in the VM.
+ */
+ *set_pd_all_visible = do_set_vm && !PageIsAllVisible(page);
+ return do_set_vm;
+}
+
+/*
+ * Decide if we want to go ahead with freezing according to the freeze plans we
+ * prepared for the given buffer or not. If the caller specified we should not
+ * freeze tuples, it exits early.
+ *
+ * do_prune, do_hint_full_or_prunable, and did_tuple_hint_fpi must all have
+ * been decided before calling this function.
+ *
+ * prstate is an input/output parameter. all_frozen_except_lp_dead is set and
+ * used later to determine the snapshot conflict horizon for the record.
+ *
+ * Returns true if we should use our freeze plans and freeze tuples on the page
+ * and false otherwise.
+ */
+static bool
+heap_page_will_freeze(Relation relation, Buffer buffer,
+ bool do_prune,
+ bool do_hint_full_or_prunable,
+ bool did_tuple_hint_fpi,
+ PruneState *prstate,
+ bool *all_frozen_except_lp_dead)
+{
+ bool do_freeze = false;
+
+ /*
+ * If the caller specified we should not attempt to freeze any tuples,
+ * validate that everything is in the right state and exit.
+ */
+ if (!prstate->attempt_freeze)
+ {
+ Assert(!prstate->all_frozen && prstate->nfrozen == 0);
+ Assert(prstate->lpdead_items == 0 || !prstate->all_visible);
+ Assert(!(*all_frozen_except_lp_dead));
+ return false;
+ }
+
+ if (prstate->pagefrz.freeze_required)
+ {
+ /*
+ * heap_prepare_freeze_tuple indicated that at least one XID/MXID from
+ * before FreezeLimit/MultiXactCutoff is present. Must freeze to
+ * advance relfrozenxid/relminmxid.
+ */
+ do_freeze = true;
+ }
+ else
+ {
+ /*
+ * Opportunistically freeze the page if we are generating an FPI
+ * anyway and if doing so means that we can set the page all-frozen
+ * afterwards (might not happen until VACUUM's final heap pass).
+ *
+ * XXX: Previously, we knew if pruning emitted an FPI by checking
+ * pgWalUsage.wal_fpi before and after pruning. Once the freeze and
+ * prune records were combined, this heuristic couldn't be used
+ * anymore. The opportunistic freeze heuristic must be improved;
+ * however, for now, try to approximate the old logic.
+ */
+ if (prstate->all_visible && prstate->all_frozen && prstate->nfrozen > 0)
+ {
+ /*
+ * Freezing would make the page all-frozen. Have already emitted
+ * an FPI or will do so anyway?
+ */
+ if (RelationNeedsWAL(relation))
+ {
+ if (did_tuple_hint_fpi)
+ do_freeze = true;
+ else if (do_prune)
+ {
+ if (XLogCheckBufferNeedsBackup(buffer))
+ do_freeze = true;
+ }
+ else if (do_hint_full_or_prunable)
+ {
+ if (XLogHintBitIsNeeded() && XLogCheckBufferNeedsBackup(buffer))
+ do_freeze = true;
+ }
+ }
+ }
+ }
+
+ if (do_freeze)
+ {
+ /*
+ * Validate the tuples we will be freezing before entering the
+ * critical section.
+ */
+ heap_pre_freeze_checks(buffer, prstate->frozen, prstate->nfrozen);
+ }
+ else if (prstate->nfrozen > 0)
+ {
+ /*
+ * The page contained some tuples that were not already frozen, and we
+ * chose not to freeze them now. The page won't be all-frozen then.
+ */
+ Assert(!prstate->pagefrz.freeze_required);
+
+ prstate->all_frozen = false;
+ prstate->nfrozen = 0; /* avoid miscounts in instrumentation */
+ }
+ else
+ {
+ /*
+ * We have no freeze plans to execute. The page might already be
+ * all-frozen (perhaps only following pruning), though. Such pages
+ * can be marked all-frozen in the VM by our caller, even though none
+ * of its tuples were newly frozen here.
+ */
+ }
+
+ /*
+ * It was convenient to ignore LP_DEAD items in all_visible earlier on to
+ * make the choice of whether or not to freeze the page unaffected by the
+ * short-term presence of LP_DEAD items. These LP_DEAD items were
+ * effectively assumed to be LP_UNUSED items in the making. It doesn't
+ * matter which vacuum heap pass (initial pass or final pass) ends up
+ * setting the page all-frozen, as long as the ongoing VACUUM does it.
+ *
+ * Now that freezing has been finalized, unset all_visible if there are
+ * any LP_DEAD items on the page. It needs to reflect the present state of
+ * the page when using it to determine whether or not to update the VM.
+ *
+ * Keep track of whether or not the page was all-frozen except LP_DEAD
+ * items for the purposes of calculating the snapshot conflict horizon,
+ * though.
+ */
+ *all_frozen_except_lp_dead = prstate->all_frozen;
+ if (prstate->lpdead_items > 0)
+ {
+ prstate->all_visible = false;
+ prstate->all_frozen = false;
+ }
+
+ return do_freeze;
+}
+
+
/*
* Prune and repair fragmentation and potentially freeze tuples on the
* specified page. If the page's visibility status has changed, update it in
@@ -766,20 +1025,30 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/* Clear the offset information once we have processed the given page. */
*off_loc = InvalidOffsetNumber;
- do_prune = prstate.nredirected > 0 ||
- prstate.ndead > 0 ||
- prstate.nunused > 0;
-
/*
* After processing all the live tuples on the page, if the newest xmin
* amongst them is not visible to everyone, the page cannot be
- * all-visible.
+ * all-visible. This must be done before we decide whether or not to
+ * opportunistically freeze below because we do not want to
+ * opportunistically freeze the page if there are live tuples not visible
+ * to everyone, which would prevent setting the page frozen in the VM.
*/
if (prstate.all_visible &&
TransactionIdIsNormal(prstate.visibility_cutoff_xid) &&
!GlobalVisXidVisibleToAll(prstate.vistest, prstate.visibility_cutoff_xid))
prstate.all_visible = prstate.all_frozen = false;
+ /*
+ * Now decide based on information collected while examining every tuple
+ * which actions to take. If there are any prunable tuples, we'll prune
+ * them. However, we will decide based on options specified by the caller
+ * and various heuristics whether or not to freeze any tuples and whether
+ * or not the page should be set all-visible/all-frozen in the VM.
+ */
+ do_prune = prstate.nredirected > 0 ||
+ prstate.ndead > 0 ||
+ prstate.nunused > 0;
+
/*
* Even if we don't prune anything, if we found a new value for the
* pd_prune_xid field or the page was marked full, we will update those
@@ -790,182 +1059,32 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
PageIsFull(page);
/*
- * Decide if we want to go ahead with freezing according to the freeze
- * plans we prepared, or not.
- */
- do_freeze = false;
- if (prstate.attempt_freeze)
- {
- if (prstate.pagefrz.freeze_required)
- {
- /*
- * heap_prepare_freeze_tuple indicated that at least one XID/MXID
- * from before FreezeLimit/MultiXactCutoff is present. Must
- * freeze to advance relfrozenxid/relminmxid.
- */
- do_freeze = true;
- }
- else
- {
- /*
- * Opportunistically freeze the page if we are generating an FPI
- * anyway and if doing so means that we can set the page
- * all-frozen afterwards (might not happen until VACUUM's final
- * heap pass).
- *
- * XXX: Previously, we knew if pruning emitted an FPI by checking
- * pgWalUsage.wal_fpi before and after pruning. Once the freeze
- * and prune records were combined, this heuristic couldn't be
- * used anymore. The opportunistic freeze heuristic must be
- * improved; however, for now, try to approximate the old logic.
- */
- if (prstate.all_visible && prstate.all_frozen && prstate.nfrozen > 0)
- {
- /*
- * Freezing would make the page all-frozen. Have already
- * emitted an FPI or will do so anyway?
- */
- if (RelationNeedsWAL(relation))
- {
- if (did_tuple_hint_fpi)
- do_freeze = true;
- else if (do_prune)
- {
- if (XLogCheckBufferNeedsBackup(buffer))
- do_freeze = true;
- }
- else if (do_hint_full_or_prunable)
- {
- if (XLogHintBitIsNeeded() && XLogCheckBufferNeedsBackup(buffer))
- do_freeze = true;
- }
- }
- }
- }
- }
-
- if (do_freeze)
- {
- /*
- * Validate the tuples we will be freezing before entering the
- * critical section.
- */
- heap_pre_freeze_checks(buffer, prstate.frozen, prstate.nfrozen);
- }
- else if (prstate.nfrozen > 0)
- {
- /*
- * The page contained some tuples that were not already frozen, and we
- * chose not to freeze them now. The page won't be all-frozen then.
- */
- Assert(!prstate.pagefrz.freeze_required);
-
- prstate.all_frozen = false;
- prstate.nfrozen = 0; /* avoid miscounts in instrumentation */
- }
- else
- {
- /*
- * We have no freeze plans to execute. The page might already be
- * all-frozen (perhaps only following pruning), though. Such pages
- * can be marked all-frozen in the VM by our caller, even though none
- * of its tuples were newly frozen here.
- */
- }
-
- /*
- * It was convenient to ignore LP_DEAD items in all_visible earlier on to
- * make the choice of whether or not to freeze the page unaffected by the
- * short-term presence of LP_DEAD items. These LP_DEAD items were
- * effectively assumed to be LP_UNUSED items in the making. It doesn't
- * matter which vacuum heap pass (initial pass or final pass) ends up
- * setting the page all-frozen, as long as the ongoing VACUUM does it.
- *
- * Now that freezing has been finalized, unset all_visible if there are
- * any LP_DEAD items on the page. It needs to reflect the present state of
- * the page when using it to determine whether or not to update the VM.
- *
- * Keep track of whether or not the page was all-frozen except LP_DEAD
- * items for the purposes of calculating the snapshot conflict horizon,
- * though.
+ * We must decide whether or not to freeze before deciding if and what to
+ * set in the VM.
*/
- all_frozen_except_lp_dead = prstate.all_frozen;
- if (prstate.lpdead_items > 0)
- {
- prstate.all_visible = false;
- prstate.all_frozen = false;
- }
-
- /*
- * If this is an on-access call and we're not actually pruning, avoid
- * setting the visibility map if it would newly dirty the heap page or, if
- * the page is already dirty, if doing so would require including a
- * full-page image (FPI) of the heap page in the WAL. This situation
- * should be rare, as on-access pruning is only attempted when
- * pd_prune_xid is valid.
- */
- if (reason == PRUNE_ON_ACCESS &&
- prstate.consider_update_vm &&
- prstate.all_visible &&
- !do_prune && !do_freeze &&
- (!BufferIsDirty(buffer) || XLogCheckBufferNeedsBackup(buffer)))
- {
- prstate.consider_update_vm = false;
- prstate.all_visible = prstate.all_frozen = false;
- }
-
- Assert(!prstate.all_frozen || prstate.all_visible);
-
- /*
- * Handle setting visibility map bit based on information from the VM (if
- * provided, e.g. by vacuum from the last heap_vac_scan_next_block()
- * call), and from all_visible and all_frozen variables.
- */
- if (prstate.consider_update_vm)
- {
- /*
- * Clear any VM corruption. This does not need to be in a critical
- * section, so we do it first. If PD_ALL_VISIBLE is incorrectly set,
- * we may mark the heap page buffer dirty here and could end up doing
- * so again later. This is not a correctness issue and is in the path
- * of VM corruption, so we don't have to worry about the extra
- * performance overhead.
- */
- if (identify_and_fix_vm_corruption(relation,
- blockno, buffer, page,
- blk_known_av, prstate.lpdead_items, vmbuffer))
- {
- /* If we fix corruption, don't update the VM further */
- }
-
- /* Determine if we actually need to set the VM and which bits to set. */
- else if (prstate.all_visible &&
- (!blk_known_av ||
- (prstate.all_frozen && !VM_ALL_FROZEN(relation, blockno, &vmbuffer))))
- {
- vmflags |= VISIBILITYMAP_ALL_VISIBLE;
- if (prstate.all_frozen)
- vmflags |= VISIBILITYMAP_ALL_FROZEN;
- }
- }
-
- do_set_vm = vmflags & VISIBILITYMAP_VALID_BITS;
-
- /*
- * Don't set PD_ALL_VISIBLE unless we also plan to set the VM. While it is
- * correct for a heap page to have PD_ALL_VISIBLE even if the VM is not
- * set, we strongly prefer to keep them in sync.
- *
- * Prior to Postgres 19, it was possible for the page-level bit to be set
- * and the VM bit to be clear. This could happen if we crashed after
- * setting PD_ALL_VISIBLE but before setting bits in the VM.
- */
- set_pd_all_visible = do_set_vm && !PageIsAllVisible(page);
+ do_freeze = heap_page_will_freeze(relation, buffer,
+ do_prune,
+ do_hint_full_or_prunable,
+ did_tuple_hint_fpi,
+ &prstate,
+ &all_frozen_except_lp_dead);
+
+ do_set_vm = heap_page_will_update_vm(relation,
+ buffer, blockno, page,
+ reason,
+ do_prune, do_freeze,
+ blk_known_av,
+ &prstate,
+ &vmbuffer,
+ &vmflags, &set_pd_all_visible);
/* Save these for the caller in case we later zero out vmflags */
presult->new_vmbits = vmflags;
- /* Any error while applying the changes is critical */
+ /*
+ * Time to actually make the changes to the page and log them. Any error
+ * while applying the changes is critical.
+ */
START_CRIT_SECTION();
if (do_hint_full_or_prunable)
--
2.43.0
[text/x-patch] v9-0022-Set-pd_prune_xid-on-insert.patch (6.5K, 23-v9-0022-Set-pd_prune_xid-on-insert.patch)
download | inline diff:
From 9b5273ec435a8025295c3cfbded611795b50f4d8 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 29 Jul 2025 16:12:56 -0400
Subject: [PATCH v9 22/22] Set pd_prune_xid on insert
Now that we can set the VM during read-only queries, it makes sense to
start setting the page prunable hint on insert. This will allow
heap_page_prune_and_freeze() to be called when the page is full or
mostly full.
For years there has been a note in heap_insert() and heap_multi_insert()
pointing out that setting pd_prune_xid would help clean up aborted
inserted tuples that would otherwise not be cleaned up until vacuum.
So, that's another benefit of setting it.
Setting pd_prune_xid on insert causes a page to be pruned and then
written out which then affects the reported number of hits in the
index-killtuples isolation test. This is a quirk of how hits are tracked
which sometimes leads them to be double counted. This should probably be
fixed or changed independently.
ci-os-only:
---
src/backend/access/heap/heapam.c | 25 +++++++++++++------
src/backend/access/heap/heapam_xlog.c | 15 ++++++++++-
.../isolation/expected/index-killtuples.out | 6 ++---
3 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index d07693b7075..02aa2383c50 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2105,6 +2105,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
TransactionId xid = GetCurrentTransactionId();
HeapTuple heaptup;
Buffer buffer;
+ Page page;
Buffer vmbuffer = InvalidBuffer;
bool all_visible_cleared = false;
@@ -2164,15 +2165,19 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
}
/*
- * XXX Should we set PageSetPrunable on this page ?
+ * Set pd_prune_xid to trigger heap_page_prune_and_freeze() once the page
+ * is full so that we can set the page all-visible in the VM.
*
- * The inserting transaction may eventually abort thus making this tuple
- * DEAD and hence available for pruning. Though we don't want to optimize
- * for aborts, if no other tuple in this page is UPDATEd/DELETEd, the
- * aborted tuple will never be pruned until next vacuum is triggered.
+ * Setting pd_prune_xid is also handy if the inserting transaction
+ * eventually aborts making this tuple DEAD and hence available for
+ * pruning. If no other tuple in this page is UPDATEd/DELETEd, the aborted
+ * tuple would never otherwise be pruned until next vacuum is triggered.
*
- * If you do add PageSetPrunable here, add it in heap_xlog_insert too.
+ * Don't set it if we are in bootstrap mode, though.
*/
+ page = BufferGetPage(buffer);
+ if (TransactionIdIsNormal(xid))
+ PageSetPrunable(page, xid);
MarkBufferDirty(buffer);
@@ -2182,7 +2187,6 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
xl_heap_insert xlrec;
xl_heap_header xlhdr;
XLogRecPtr recptr;
- Page page = BufferGetPage(buffer);
uint8 info = XLOG_HEAP_INSERT;
int bufflags = 0;
@@ -2545,8 +2549,13 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
}
/*
- * XXX Should we set PageSetPrunable on this page ? See heap_insert()
+ * Set pd_prune_xid. See heap_insert() for more on why we do this when
+ * inserting tuples. This only makes sense if we aren't already
+ * setting the page frozen in the VM. We also don't set it in
+ * bootstrap mode.
*/
+ if (!all_frozen_set && TransactionIdIsNormal(xid))
+ PageSetPrunable(page, xid);
MarkBufferDirty(buffer);
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index ff3ad8b4cd2..e7d7804871b 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -470,6 +470,12 @@ heap_xlog_insert(XLogReaderState *record)
freespace = PageGetHeapFreeSpace(page); /* needed to update FSM below */
+ /*
+ * Set the page prunable to trigger on-access pruning later which may
+ * set the page all-visible in the VM.
+ */
+ PageSetPrunable(page, XLogRecGetXid(record));
+
PageSetLSN(page, lsn);
if (xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED)
@@ -619,9 +625,16 @@ heap_xlog_multi_insert(XLogReaderState *record)
if (xlrec->flags & XLH_INSERT_ALL_VISIBLE_CLEARED)
PageClearAllVisible(page);
- /* XLH_INSERT_ALL_FROZEN_SET implies that all tuples are visible */
+ /*
+ * XLH_INSERT_ALL_FROZEN_SET implies that all tuples are visible. If
+ * we are not setting the page frozen, then set the page's prunable
+ * hint so that we trigger on-access pruning later which may set the
+ * page all-visible in the VM.
+ */
if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET)
PageSetAllVisible(page);
+ else
+ PageSetPrunable(page, XLogRecGetXid(record));
MarkBufferDirty(buffer);
}
diff --git a/src/test/isolation/expected/index-killtuples.out b/src/test/isolation/expected/index-killtuples.out
index be7ddd756ef..b29f2434b00 100644
--- a/src/test/isolation/expected/index-killtuples.out
+++ b/src/test/isolation/expected/index-killtuples.out
@@ -54,7 +54,7 @@ step flush: SELECT FROM pg_stat_force_next_flush();
step result: SELECT heap_blks_read + heap_blks_hit - counter.heap_accesses AS new_heap_accesses FROM counter, pg_statio_all_tables WHERE relname = 'kill_prior_tuple';
new_heap_accesses
-----------------
- 1
+ 2
(1 row)
step measure: UPDATE counter SET heap_accesses = (SELECT heap_blks_read + heap_blks_hit FROM pg_statio_all_tables WHERE relname = 'kill_prior_tuple');
@@ -130,7 +130,7 @@ step flush: SELECT FROM pg_stat_force_next_flush();
step result: SELECT heap_blks_read + heap_blks_hit - counter.heap_accesses AS new_heap_accesses FROM counter, pg_statio_all_tables WHERE relname = 'kill_prior_tuple';
new_heap_accesses
-----------------
- 1
+ 2
(1 row)
step measure: UPDATE counter SET heap_accesses = (SELECT heap_blks_read + heap_blks_hit FROM pg_statio_all_tables WHERE relname = 'kill_prior_tuple');
@@ -283,7 +283,7 @@ step flush: SELECT FROM pg_stat_force_next_flush();
step result: SELECT heap_blks_read + heap_blks_hit - counter.heap_accesses AS new_heap_accesses FROM counter, pg_statio_all_tables WHERE relname = 'kill_prior_tuple';
new_heap_accesses
-----------------
- 1
+ 2
(1 row)
step measure: UPDATE counter SET heap_accesses = (SELECT heap_blks_read + heap_blks_hit FROM pg_statio_all_tables WHERE relname = 'kill_prior_tuple');
--
2.43.0
view thread (143+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
In-Reply-To: <CAAKRu_Yz9x0sejBa5ov_LJ5sMOSKM3AeKOFUg+fQpNqyMmxwRA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox