Date: Tue, 19 Mar 2024 21:36:02 -0400
From: Melanie Plageman <melanieplageman@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>,
	Andres Freund <andres@anarazel.de>,
	Robert Haas <robertmhaas@gmail.com>, Peter Geoghegan <pg@bowt.ie>
Subject: Re: Combine Prune and Freeze records emitted by vacuum
Message-ID: <20240320013602.6sypr4cx6sefpemg@liskov>
References: <CAAKRu_azf-zH=DgVbquZ3tFWjMY1w5pO8m-TXJaMdri8z3933g@mail.gmail.com>
 <390ec1c8-824a-4a53-8ab2-14f6173f1ae3@iki.fi>
 <CAAKRu_bEo54H+sKXiCb4zsG32vXU7QoO+kGYTAoFyvYZzXxGiA@mail.gmail.com>
 <ff8a5cd7-330f-40cf-8878-da09dc16ee41@iki.fi>
 <20240313232556.ohv3z7qzc5wtwqh5@liskov>
 <20240315005658.y7bvbzvnlciqmhd6@liskov>
 <bdc400b0-a506-4ef7-b909-8d9bbb729202@iki.fi>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="tez7m2a73jtztiij"
Content-Disposition: inline
In-Reply-To: <bdc400b0-a506-4ef7-b909-8d9bbb729202@iki.fi>
Archived-At: <https://www.postgresql.org/message-id/20240320013602.6sypr4cx6sefpemg%40liskov>
Precedence: bulk


--tez7m2a73jtztiij
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Mar 18, 2024 at 01:15:21AM +0200, Heikki Linnakangas wrote:
> On 15/03/2024 02:56, Melanie Plageman wrote:
> > Okay, so I was going to start using xl_heap_prune for vacuum here too,
> > but I realized it would be bigger because of the
> > snapshotConflictHorizon. Do you think there is a non-terrible way to
> > make the snapshotConflictHorizon optional? Like with a flag?
> 
> Yeah, another flag would do the trick.

Okay, I've done this in attached v4 (including removing
XLOG_HEAP2_VACUUM). I had to put the snapshot conflict horizon in the
"main chunk" of data available at replay regardless of whether or not
the record ended up including an FPI.

I made it its own sub-record (xlhp_conflict_horizon) less to help with
alignment (though we can use all the help we can get there) and more to
keep it from getting lost. When you look at heapam_xlog.h, you can see
what a XLOG_HEAP2_PRUNE record will contain starting with the
xl_heap_prune struct and then all the sub-record types.

> > > I introduced a few sub-record types similar to what you suggested --
> > > they help a bit with alignment, so I think they are worth keeping. There
> > > are comments around them, but perhaps a larger diagram of the layout of
> > > a the new XLOG_HEAP2_PRUNE record would be helpful.
> > 
> > I started doing this, but I can't find a way of laying out the diagram
> > that pgindent doesn't destroy. I thought I remember other diagrams in
> > the source code showing the layout of something (something with pages
> > somewhere?) that don't get messed up by pgindent, but I couldn't find
> > them.
> 
> See src/tools/pgindent/README, section "Cleaning up in case of failure or
> ugly output":
> 
>     /*----------
>      * Text here will not be touched by pgindent.
>      */
> 

Cool. This version doesn't include the spiffy drawing I promised yet.

> Note that we don't provide WAL compatibility across major versions. You can
> fully remove the old xl_heap_freeze_page format. (We should bump
> XLOG_PAGE_MAGIC when this is committed though)

I've removed the xl_heap_freeze (and xl_heap_prune). I didn't bump
XLOG_PAGE_MAGIC.

> > > On the point of removing the freeze-only code path from
> > > heap_page_prune() (now heap_page_prune_and_freeze()): while doing this,
> > > I realized that heap_pre_freeze_checks() was not being called in the
> > > case that we decide to freeze because we emitted an FPI while setting
> > > the hint bit. I've fixed that, however, I've done so by moving
> > > heap_pre_freeze_checks() into the critical section. I think that is not
> > > okay? I could move it earlier and not do call it when the hint bit FPI
> > > leads us to freeze tuples. But, I think that would lead to us doing a
> > > lot less validation of tuples being frozen when checksums are enabled.
> > > Or, I could make two critical sections?
> > 
> > I found another approach and just do the pre-freeze checks if we are
> > considering freezing except for the FPI criteria.
> 
> Hmm, I think you can make this simpler if you use
> XLogCheckBufferNeedsBackup() to check if the hint-bit update will generate a
> full-page-image before entering the critical section. Like you did to check
> if pruning will generate a full-page-image. I included that change in the
> attached patches.

I used your proposed structure. You had XLogCheckBufferNeedsBackup()
twice in your proposed version a few lines apart. I don't think there is
any point in checking it twice. If we are going to rely on
XLogCheckBufferNeedsBackup() to tell us whether or not setting the hint
bit is *likely* to emit an FPI, then we might as well just call
XLogCheckBufferNeedsBackup() once.

> I don't fully understand this:
> 
> > 	/*
> > 	 * If we will freeze tuples on the page or they were all already frozen
> > 	 * on the page, if we will set the page all-frozen in the visibility map,
> > 	 * we can advance relfrozenxid and relminmxid to the values in
> > 	 * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid.
> > 	 */
> > 	if (presult->all_frozen || presult->nfrozen > 0)
> > 	{
> > 		presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
> > 		presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
> > 	}
> > 	else
> > 	{
> > 		presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
> > 		presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
> > 	}
> 
> Firstly, the comment is outdated, because we have already done the freezing
> at this point. But more importantly, I don't understand the logic here.
> Could we check just presult->nfrozen > 0 here and get the same result?

I've updated the comment. The reason I had
if (presult->all_frozen ||  presult->nfrozen > 0) was because of this
comment in heapam.h in the HeapPageFreeze struct

	 * "Freeze" NewRelfrozenXid/NewRelminMxid trackers.
	 *
	 * Trackers used when heap_freeze_execute_prepared freezes, or when there
	 * are zero freeze plans for a page.  It is always valid for vacuumlazy.c
	 * to freeze any page, by definition.  This even includes pages that have
	 * no tuples with storage to consider in the first place.  That way the
	 * 'totally_frozen' results from heap_prepare_freeze_tuple can always be
	 * used in the same way, even when no freeze plans need to be executed to
	 * "freeze the page".  Only the "freeze" path needs to consider the need
	 * to set pages all-frozen in the visibility map under this scheme.
	 *
	 * When we freeze a page, we generally freeze all XIDs < OldestXmin, only
	 * leaving behind XIDs that are ineligible for freezing, if any.  And so
	 * you might wonder why these trackers are necessary at all; why should
	 * _any_ page that VACUUM freezes _ever_ be left with XIDs/MXIDs that
	 * ratchet back the top-level NewRelfrozenXid/NewRelminMxid trackers?
	 *
	 * It is useful to use a definition of "freeze the page" that does not
	 * overspecify how MultiXacts are affected.  heap_prepare_freeze_tuple
	 * generally prefers to remove Multis eagerly, but lazy processing is used
	 * in cases where laziness allows VACUUM to avoid allocating a new Multi.
	 * The "freeze the page" trackers enable this flexibility.
	 */

So, I don't really know if it is right to just check presult->nfrozen >
0 when updating relminmxid. I have changed it to the way you suggested.
But we can change it back.

> Here are some patches on top of your patches for some further refactorings.
> Some notable changes in heap_page_prune_and_freeze():
> 
> - I moved the heap_prepare_freeze_tuple() work from the 2nd loop to the 1st
> one. It seems more clear and efficient that way.

cool. I kept this.

> - I extracted the code to generate the WAL record to a separate function.

cool. kept this.

> These patches are in a very rough WIP state, but I wanted to share early. I
> haven't done much testing, and I'm not wedded to these changes, but I think
> they make it more readable. Please include / squash in the patch set if you
> agree with them.

I've squashed the changes into and across my nineteen patches :)

I cleaned up your sugestions a bit and made a few stylistic choices.

In this version, I also broke up the last couple commits which
streamlined the WAL record and eliminated XLOG_HEAP2_FREEZE/VACUUM and
redistributed those changes in a way that I thought made sense.

Now, the progression is that in one commit we merge the prune and freeze
record, eliminating the XLOG_HEAP2_FREEZE record. Then, in another
commit, we eliminate the XLOG_HEAP2_VACUUM record. Then a later commit
streamlines the new mega xl_heap_prune struct into the variable size
structure based on which modifications it includes.

> From 622620a7875ae8c1626e9cd118156e0c734d44ed Mon Sep 17 00:00:00 2001
> From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
> Date: Sun, 17 Mar 2024 22:52:28 +0200
> Subject: [PATCH v3heikki 1/4] Inline heap_frz_conflict_horizon() to the
>  caller.
> 
> FIXME: This frz_conflict_horizon business looks fishy to me. We have:
> - local frz_conflict_horizon variable,
> - presult->frz_conflict_horizon, and
> - prstate.snapshotConflictHorizon

Yea, this is a mistake I made when I was rebasing some changes in. The
local variable is gone now.


> should we really have all three, and what are the differences?

We do need both the prstate.snapshotConflictHorizon and the
presult->frz_conflict_horizon because the youngest freezable xmin will
often be different than the oldest removable xmax, so we have to track
both and take the most conservative one if we are both freezing and
pruning.

The third (local variable) one was an oops.

> From 0219842487931f899abcf183c863c43270c098f0 Mon Sep 17 00:00:00 2001
> From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
> Date: Sun, 17 Mar 2024 23:05:31 +0200
> Subject: [PATCH v3heikki 2/4] Misc cleanup
> ---
>  src/backend/access/heap/pruneheap.c  | 16 +++++++---------
>  src/backend/access/heap/vacuumlazy.c |  7 ++++++-
>  2 files changed, 13 insertions(+), 10 deletions(-)
> diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
> index 8d3723faf3a..d3df7029667 100644
> --- a/src/backend/access/heap/vacuumlazy.c
> +++ b/src/backend/access/heap/vacuumlazy.c
> @@ -433,7 +433,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
>  	 * heap_page_prune_and_freeze(). We expect vistest will always make
>  	 * heap_page_prune_and_freeze() remove any deleted tuple whose xmax is <
>  	 * OldestXmin.  lazy_scan_prune must never become confused about whether a
> -	 * tuple should be frozen or removed.  (In the future we might want to
> +	 * tuple should be frozen or removed.
> +	 * HEIKKI: Is such confusion possible anymore?
> +	 * (In the future we might want to
>  	 * teach lazy_scan_prune to recompute vistest from time to time, to
>  	 * increase the number of dead tuples it can prune away.)

TBH, I don't really know what this comment is even saying. But
lazy_scan_prune() doesn't do any freezing anymore, so I removed this
sentence.

>  	 */
> @@ -1387,6 +1389,9 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
>   * so we could deal with tuples that were DEAD to VACUUM, but nevertheless were
>   * left with storage after pruning.
>   *
> + * HEIKKI: does the above paragraph still make sense? We don't call
> + * HeapTupleSatisfiesVacuum() here anymore
> + *

Yea this whole comment definitely doesn't belong here anymore. Even
though we are calling HeapTupleSatisfiesVacuum() (from inside
heap_prune_satisifes_vacuum()) inside heap_page_prune_and_freeze(), the
comment really doesn't fit anywhere in there either. The comment is
describing a situation that is no longer possible. So describing a
situation that is no longer possible in a part of the code that it never
could have been possible does not make sense to me. I've removed the
comment.

>   * As of Postgres 17, we circumvent this problem altogether by reusing the
>   * result of heap_page_prune_and_freeze()'s visibility check. Without the
>   * second call to HeapTupleSatisfiesVacuum(), there is no new HTSV_Result and
> -- 
> 2.39.2
> 

> From d72cebf13f9866112309883f72a382fc2cb57e17 Mon Sep 17 00:00:00 2001
> From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
> Date: Sun, 17 Mar 2024 23:08:42 +0200
> Subject: [PATCH v3heikki 3/4] Move work to the first loop
>  src/backend/access/heap/pruneheap.c | 141 ++++++++++------------------
>  1 file changed, 52 insertions(+), 89 deletions(-)
> 
> diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
> index b3573bb628b..3541628799a 100644
> --- a/src/backend/access/heap/pruneheap.c
> +++ b/src/backend/access/heap/pruneheap.c
> @@ -78,9 +78,6 @@ static int	heap_prune_chain(Buffer buffer,
>  							 PruneState *prstate, PruneFreezeResult *presult);
>  
>  static inline HTSV_Result htsv_get_valid_status(int status);
> -static void prune_prepare_freeze_tuple(Page page, OffsetNumber offnum, PruneState *prstate,
> -									   HeapPageFreeze *pagefrz, HeapTupleFreeze *frozen,
> -									   PruneFreezeResult *presult);
>  static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
>  static void heap_prune_record_redirect(PruneState *prstate,
>  									   OffsetNumber offnum, OffsetNumber rdoffnum,
> @@ -322,6 +319,16 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
>  	/* for recovery conflicts */
>  	presult->frz_conflict_horizon = InvalidTransactionId;
>  
> +	/*
> +	 * We will update the VM after pruning, collecting LP_DEAD items, and
> +	 * freezing tuples. Keep track of whether or not the page is all_visible
> +	 * and all_frozen and use this information to update the VM. all_visible
> +	 * implies lpdead_items == 0, but don't trust all_frozen result unless
> +	 * all_visible is also set to true.
> +	 */
> +	/* HEIKKI: the caller updates the VM? not us */

I've updated this comment.

Other questions and notes on v4:

xl_heap_prune->flags is a uint8, but we are already using 7 of the bits.
Should we make it a unit16?

Eventually, I would like to avoid emitting a separate XLOG_HEAP2_VISIBLE
record for vacuum's first and second passes and just include the VM
update flags in the xl_heap_prune record. xl_heap_visible->flags is a
uint8. If we made xl_heap_prune->flags uint16, we could probably combine
them (though maybe we want other bits available). Also vacuum's second
pass doesn't set a snapshotConflictHorizon, so if we combined
xl_heap_visible and xl_heap_prune for vacuum we would end up saving even
more space (since vacuum sets xl_heap_visible->snapshotConflictHorizon
to InvalidXLogRecPtr).

A note on sub-record naming: I kept xl_heap_freeze_plan's name but
prefixed the other sub-records with xlhp. Do you think it is worth
renaming it (to xlhp_freeze_plan)? Also, should I change xlhp_freeze to
xlhp_freeze_page?

- Melanie

--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0001-Reorganize-heap_page_prune-function-comment.patch"

From d1b1ce71c88a04e3c5a2480827bd3af3ffbe1372 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 18 Mar 2024 19:01:17 -0400
Subject: [PATCH v4 01/19] Reorganize heap_page_prune() function comment

heap_page_prune()'s function header comment didn't explain the
parameters in the same order they appear in the function. Fix that.

While we are at it, move some parts of the initial function body
comments around so they are in more relevant locations.
---
 src/backend/access/heap/pruneheap.c | 30 ++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 4f12413b8b1..b5895406ec2 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -193,22 +193,26 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
  *
  * 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
- * also need to account for a reduction in the length of the line pointer
- * array following array truncation by us.
+ * also need to account for a reduction in the length of the line pointer array
+ * following array truncation by us.
  *
- * vistest is used to distinguish whether tuples are DEAD or RECENTLY_DEAD
- * (see heap_prune_satisfies_vacuum and
- * HeapTupleSatisfiesVacuum).
+ * 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 logic as
+ * possible out of the critical section, and also ensures that WAL replay will
+ * work the same as the normal case.
  *
- * mark_unused_now indicates whether or not dead items can be set LP_UNUSED during
- * pruning.
+ * vistest is used to distinguish whether tuples are DEAD or RECENTLY_DEAD (see
+ * heap_prune_satisfies_vacuum and HeapTupleSatisfiesVacuum).
  *
- * off_loc is the offset location required by the caller to use in error
- * callback.
+ * mark_unused_now indicates whether or not dead items can be set LP_UNUSED
+ * during pruning.
  *
  * presult contains output parameters needed by callers such as the number of
  * tuples removed and the number of line pointers newly marked LP_DEAD.
  * heap_page_prune() is responsible for initializing it.
+ *
+ * off_loc is the offset location required by the caller to use in error
+ * callback.
  */
 void
 heap_page_prune(Relation relation, Buffer buffer,
@@ -225,11 +229,6 @@ heap_page_prune(Relation relation, Buffer buffer,
 	HeapTupleData tup;
 
 	/*
-	 * 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
-	 * logic as possible out of the critical section, and also ensures that
-	 * WAL replay will work the same as the normal case.
-	 *
 	 * First, initialize the new pd_prune_xid value to zero (indicating no
 	 * prunable tuples).  If we find any tuples which may soon become
 	 * prunable, we will save the lowest relevant XID in new_prune_xid. Also
@@ -241,12 +240,13 @@ heap_page_prune(Relation relation, Buffer buffer,
 	prstate.mark_unused_now = mark_unused_now;
 	prstate.snapshotConflictHorizon = InvalidTransactionId;
 	prstate.nredirected = prstate.ndead = prstate.nunused = 0;
-	memset(prstate.marked, 0, sizeof(prstate.marked));
 
 	/*
 	 * presult->htsv is not initialized here because all ntuple spots in the
 	 * array will be set either to a valid HTSV_Result value or -1.
 	 */
+	memset(prstate.marked, 0, sizeof(prstate.marked));
+
 	presult->ndeleted = 0;
 	presult->nnewlpdead = 0;
 
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0002-Remove-unused-PruneState-member-rel.patch"

From f7c3a7680e144e55f8c1c3eccaa5a7a1af47a47e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 18 Mar 2024 18:59:09 -0400
Subject: [PATCH v4 02/19] Remove unused PruneState member rel

PruneState->rel is no longer being used, so just remove it.
---
 src/backend/access/heap/pruneheap.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index b5895406ec2..08cb2a6f533 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -29,8 +29,6 @@
 /* Working data for heap_page_prune and subroutines */
 typedef struct
 {
-	Relation	rel;
-
 	/* tuple visibility test, initialized for the relation */
 	GlobalVisState *vistest;
 	/* whether or not dead items can be set LP_UNUSED during pruning */
@@ -235,7 +233,6 @@ heap_page_prune(Relation relation, Buffer buffer,
 	 * initialize the rest of our working state.
 	 */
 	prstate.new_prune_xid = InvalidTransactionId;
-	prstate.rel = relation;
 	prstate.vistest = vistest;
 	prstate.mark_unused_now = mark_unused_now;
 	prstate.snapshotConflictHorizon = InvalidTransactionId;
@@ -251,7 +248,7 @@ heap_page_prune(Relation relation, Buffer buffer,
 	presult->nnewlpdead = 0;
 
 	maxoff = PageGetMaxOffsetNumber(page);
-	tup.t_tableOid = RelationGetRelid(prstate.rel);
+	tup.t_tableOid = RelationGetRelid(relation);
 
 	/*
 	 * Determine HTSV for all tuples.
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0003-lazy_scan_prune-tests-tuple-vis-with-GlobalVisTes.patch"

From 416650477aa7a2da4db57070fcb2c3524093bf09 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sat, 6 Jan 2024 13:14:47 -0500
Subject: [PATCH v4 03/19] lazy_scan_prune tests tuple vis with GlobalVisTest

One requirement for eventually combining the prune and freeze records,
is that we must check during pruning if live tuples on the page are
visible to everyone and thus, whether or not the page is all visible. We
only consider opportunistically freezing tuples if the whole page is all
visible and could be set all frozen.

During pruning (in heap_page_prune()), we do not have access to
VacuumCutoffs -- as on access pruning also calls heap_page_prune(). We
do, however, have access to a GlobalVisState. This can be used to
determine whether or not the tuple is visible to everyone. It also has
the potential of being more up-to-date than VacuumCutoffs->OldestXmin.

This commit simply modifies lazy_scan_prune() to use GlobalVisState
instead of OldestXmin. Future commits will move the
all_visible/all_frozen calculation into heap_page_prune().
---
 src/backend/access/heap/vacuumlazy.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 18004907750..3a991f0ea71 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1579,11 +1579,15 @@ lazy_scan_prune(LVRelState *vacrel,
 
 					/*
 					 * The inserter definitely committed. But is it old enough
-					 * that everyone sees it as committed?
+					 * that everyone sees it as committed? A
+					 * FrozenTransactionId is seen as committed to everyone.
+					 * Otherwise, we check if there is a snapshot that
+					 * considers this xid to still be running, and if so, we
+					 * don't consider the page all-visible.
 					 */
 					xmin = HeapTupleHeaderGetXmin(htup);
-					if (!TransactionIdPrecedes(xmin,
-											   vacrel->cutoffs.OldestXmin))
+					if (xmin != FrozenTransactionId &&
+						!GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
 					{
 						all_visible = false;
 						break;
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0004-Pass-heap_prune_chain-PruneResult-output-paramete.patch"

From 94c0256a9c7e11001fcf4dec8998943e0b326f50 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sat, 6 Jan 2024 13:39:59 -0500
Subject: [PATCH v4 04/19] Pass heap_prune_chain() PruneResult output parameter

Future commits will set other members of PruneResult in
heap_prune_chain(), so start passing it as an output parameter now. This
eliminates the output parameter htsv -- the array of HTSV_Results --
since that is a member of the PruneResult.
---
 src/backend/access/heap/pruneheap.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 08cb2a6f533..7eb21b603ba 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -59,8 +59,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
 											   Buffer buffer);
 static int	heap_prune_chain(Buffer buffer,
 							 OffsetNumber rootoffnum,
-							 int8 *htsv,
-							 PruneState *prstate);
+							 PruneState *prstate, PruneResult *presult);
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
 									   OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -322,7 +321,7 @@ heap_page_prune(Relation relation, Buffer buffer,
 
 		/* Process this item or chain of items */
 		presult->ndeleted += heap_prune_chain(buffer, offnum,
-											  presult->htsv, &prstate);
+											  &prstate, presult);
 	}
 
 	/* Clear the offset information once we have processed the given page. */
@@ -451,7 +450,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
 /*
  * Prune specified line pointer or a HOT chain originating at line pointer.
  *
- * Tuple visibility information is provided in htsv.
+ * Tuple visibility information is provided in presult->htsv.
  *
  * If the item is an index-referenced tuple (i.e. not a heap-only tuple),
  * the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -481,7 +480,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
  */
 static int
 heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
-				 int8 *htsv, PruneState *prstate)
+				 PruneState *prstate, PruneResult *presult)
 {
 	int			ndeleted = 0;
 	Page		dp = (Page) BufferGetPage(buffer);
@@ -502,7 +501,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 	 */
 	if (ItemIdIsNormal(rootlp))
 	{
-		Assert(htsv[rootoffnum] != -1);
+		Assert(presult->htsv[rootoffnum] != -1);
 		htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
 
 		if (HeapTupleHeaderIsHeapOnly(htup))
@@ -525,7 +524,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 			 * either here or while following a chain below.  Whichever path
 			 * gets there first will mark the tuple unused.
 			 */
-			if (htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+			if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
 				!HeapTupleHeaderIsHotUpdated(htup))
 			{
 				heap_prune_record_unused(prstate, rootoffnum);
@@ -622,7 +621,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 		 */
 		tupdead = recent_dead = false;
 
-		switch (htsv_get_valid_status(htsv[offnum]))
+		switch (htsv_get_valid_status(presult->htsv[offnum]))
 		{
 			case HEAPTUPLE_DEAD:
 				tupdead = true;
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0005-heap_page_prune-sets-all_visible-and-frz_conflict.patch"

From caecf591376392a5518cb42923542f380e1c327c Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sat, 6 Jan 2024 14:01:37 -0500
Subject: [PATCH v4 05/19] heap_page_prune sets all_visible and
 frz_conflict_horizon

In order to combine the prune and freeze records, we must know if the
page is eligible to be opportunistically frozen before finishing
pruning. Save all_visible in the PruneResult and set it to false when we
see non-removable tuples which are not visible to everyone.

We will also need to ensure that the snapshotConflictHorizon for the combined
prune + freeze record is the more conservative of that calculated for each of
pruning and freezing. Calculate the visibility_cutoff_xid for the purposes of
freezing -- the newest xmin on the page -- in heap_page_prune() and save it in
PruneResult.frz_conflict_horizon.
---
 src/backend/access/heap/pruneheap.c  | 127 +++++++++++++++++++++++++--
 src/backend/access/heap/vacuumlazy.c | 121 ++++++-------------------
 src/include/access/heapam.h          |   3 +
 3 files changed, 151 insertions(+), 100 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7eb21b603ba..bd30296ef1a 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -63,8 +63,10 @@ static int	heap_prune_chain(Buffer buffer,
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
 									   OffsetNumber offnum, OffsetNumber rdoffnum);
-static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum);
-static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+								   PruneResult *presult);
+static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+											 PruneResult *presult);
 static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
 static void page_verify_redirects(Page page);
 
@@ -246,6 +248,14 @@ heap_page_prune(Relation relation, Buffer buffer,
 	presult->ndeleted = 0;
 	presult->nnewlpdead = 0;
 
+	/*
+	 * Keep track of whether or not the page is all_visible in case the caller
+	 * wants to use this information to update the VM.
+	 */
+	presult->all_visible = true;
+	/* for recovery conflicts */
+	presult->frz_conflict_horizon = InvalidTransactionId;
+
 	maxoff = PageGetMaxOffsetNumber(page);
 	tup.t_tableOid = RelationGetRelid(relation);
 
@@ -297,8 +307,97 @@ heap_page_prune(Relation relation, Buffer buffer,
 
 		presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
 															buffer);
+		switch (presult->htsv[offnum])
+		{
+			case HEAPTUPLE_DEAD:
+
+				/*
+				 * Deliberately delay unsetting all_visible until later during
+				 * pruning. Removable dead tuples shouldn't preclude freezing
+				 * the page. After finishing this first pass of tuple
+				 * visibility checks, initialize all_visible_except_removable
+				 * with the current value of all_visible to indicate whether
+				 * or not the page is all visible except for dead tuples. This
+				 * will allow us to attempt to freeze the page after pruning.
+				 * Later during pruning, if we encounter an LP_DEAD item or
+				 * are setting an item LP_DEAD, we will unset all_visible. As
+				 * long as we unset it before updating the visibility map,
+				 * this will be correct.
+				 */
+				break;
+			case HEAPTUPLE_LIVE:
+
+				/*
+				 * Is the tuple definitely visible to all transactions?
+				 *
+				 * NB: Like with per-tuple hint bits, we can't set the
+				 * PD_ALL_VISIBLE flag if the inserter committed
+				 * asynchronously. See SetHintBits for more info. Check that
+				 * the tuple is hinted xmin-committed because of that.
+				 */
+				if (presult->all_visible)
+				{
+					TransactionId xmin;
+
+					if (!HeapTupleHeaderXminCommitted(htup))
+					{
+						presult->all_visible = false;
+						break;
+					}
+
+					/*
+					 * The inserter definitely committed. But is it old enough
+					 * that everyone sees it as committed? A
+					 * FrozenTransactionId is seen as committed to everyone.
+					 * Otherwise, we check if there is a snapshot that
+					 * considers this xid to still be running, and if so, we
+					 * don't consider the page all-visible.
+					 */
+					xmin = HeapTupleHeaderGetXmin(htup);
+					if (xmin != FrozenTransactionId &&
+						!GlobalVisTestIsRemovableXid(vistest, xmin))
+					{
+						presult->all_visible = false;
+						break;
+					}
+
+					/* Track newest xmin on page. */
+					if (TransactionIdFollows(xmin, presult->frz_conflict_horizon) &&
+						TransactionIdIsNormal(xmin))
+						presult->frz_conflict_horizon = xmin;
+				}
+				break;
+			case HEAPTUPLE_RECENTLY_DEAD:
+				presult->all_visible = false;
+				break;
+			case HEAPTUPLE_INSERT_IN_PROGRESS:
+				presult->all_visible = false;
+				break;
+			case HEAPTUPLE_DELETE_IN_PROGRESS:
+				/* This is an expected case during concurrent vacuum */
+				presult->all_visible = false;
+				break;
+			default:
+				elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
+				break;
+		}
 	}
 
+	/*
+	 * For vacuum, if the whole page will become frozen, we consider
+	 * opportunistically freezing tuples. Dead tuples which will be removed by
+	 * the end of vacuuming should not preclude us from opportunistically
+	 * freezing. We will not be able to freeze the whole page if there are
+	 * tuples present which are not visible to everyone or if there are dead
+	 * tuples which are not yet removable. We need all_visible to be false if
+	 * LP_DEAD tuples remain after pruning so that we do not incorrectly
+	 * update the visibility map or page hint bit. So, we will update
+	 * presult->all_visible to reflect the presence of LP_DEAD items while
+	 * pruning and keep all_visible_except_removable to permit freezing if the
+	 * whole page will eventually become all visible after removing tuples.
+	 */
+	presult->all_visible_except_removable = presult->all_visible;
+
 	/* Scan the page */
 	for (offnum = FirstOffsetNumber;
 		 offnum <= maxoff;
@@ -593,10 +692,14 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 			/*
 			 * If the caller set mark_unused_now true, we can set dead line
 			 * pointers LP_UNUSED now. We don't increment ndeleted here since
-			 * the LP was already marked dead.
+			 * the LP was already marked dead. If it will not be marked
+			 * LP_UNUSED, it will remain LP_DEAD, making the page not
+			 * all_visible.
 			 */
 			if (unlikely(prstate->mark_unused_now))
 				heap_prune_record_unused(prstate, offnum);
+			else
+				presult->all_visible = false;
 
 			break;
 		}
@@ -733,7 +836,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 		 * redirect the root to the correct chain member.
 		 */
 		if (i >= nchain)
-			heap_prune_record_dead_or_unused(prstate, rootoffnum);
+			heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
 		else
 			heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
 	}
@@ -746,7 +849,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 		 * redirect item.  We can clean up by setting the redirect item to
 		 * DEAD state or LP_UNUSED if the caller indicated.
 		 */
-		heap_prune_record_dead_or_unused(prstate, rootoffnum);
+		heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
 	}
 
 	return ndeleted;
@@ -783,13 +886,20 @@ heap_prune_record_redirect(PruneState *prstate,
 
 /* Record line pointer to be marked dead */
 static void
-heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
+					   PruneResult *presult)
 {
 	Assert(prstate->ndead < MaxHeapTuplesPerPage);
 	prstate->nowdead[prstate->ndead] = offnum;
 	prstate->ndead++;
 	Assert(!prstate->marked[offnum]);
 	prstate->marked[offnum] = true;
+
+	/*
+	 * Setting the line pointer LP_DEAD means the page will definitely not be
+	 * all_visible.
+	 */
+	presult->all_visible = false;
 }
 
 /*
@@ -799,7 +909,8 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum)
  * pointers LP_DEAD if mark_unused_now is true.
  */
 static void
-heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
+								 PruneResult *presult)
 {
 	/*
 	 * If the caller set mark_unused_now to true, we can remove dead tuples
@@ -810,7 +921,7 @@ heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum)
 	if (unlikely(prstate->mark_unused_now))
 		heap_prune_record_unused(prstate, offnum);
 	else
-		heap_prune_record_dead(prstate, offnum);
+		heap_prune_record_dead(prstate, offnum, presult);
 }
 
 /* Record line pointer to be marked unused */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 3a991f0ea71..f9892f4cd08 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1422,9 +1422,7 @@ lazy_scan_prune(LVRelState *vacrel,
 				recently_dead_tuples;
 	HeapPageFreeze pagefrz;
 	bool		hastup = false;
-	bool		all_visible,
-				all_frozen;
-	TransactionId visibility_cutoff_xid;
+	bool		all_frozen;
 	int64		fpi_before = pgWalUsage.wal_fpi;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
 	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
@@ -1465,17 +1463,16 @@ lazy_scan_prune(LVRelState *vacrel,
 					&presult, &vacrel->offnum);
 
 	/*
-	 * We will update the VM after collecting LP_DEAD items and freezing
-	 * tuples. Keep track of whether or not the page is all_visible and
-	 * all_frozen and use this information to update the VM. all_visible
-	 * implies 0 lpdead_items, but don't trust all_frozen result unless
-	 * all_visible is also set to true.
+	 * Now scan the page to collect LP_DEAD items and check for tuples
+	 * requiring freezing among remaining tuples with storage. 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. Keep track of
+	 * whether or not the page is all_frozen and use this information to
+	 * update the VM. all_visible implies lpdead_items == 0, but don't trust
+	 * all_frozen result unless all_visible is also set to true.
 	 *
-	 * Also keep track of the visibility cutoff xid for recovery conflicts.
 	 */
-	all_visible = true;
 	all_frozen = true;
-	visibility_cutoff_xid = InvalidTransactionId;
 
 	/*
 	 * Now scan the page to collect LP_DEAD items and update the variables set
@@ -1516,11 +1513,6 @@ lazy_scan_prune(LVRelState *vacrel,
 			 * will only happen every other VACUUM, at most.  Besides, VACUUM
 			 * must treat hastup/nonempty_pages as provisional no matter how
 			 * LP_DEAD items are handled (handled here, or handled later on).
-			 *
-			 * Also deliberately delay unsetting all_visible until just before
-			 * we return to lazy_scan_heap caller, as explained in full below.
-			 * (This is another case where it's useful to anticipate that any
-			 * LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
 			 */
 			deadoffsets[lpdead_items++] = offnum;
 			continue;
@@ -1558,46 +1550,6 @@ lazy_scan_prune(LVRelState *vacrel,
 				 * what acquire_sample_rows() does.
 				 */
 				live_tuples++;
-
-				/*
-				 * Is the tuple definitely visible to all transactions?
-				 *
-				 * NB: Like with per-tuple hint bits, we can't set the
-				 * PD_ALL_VISIBLE flag if the inserter committed
-				 * asynchronously. See SetHintBits for more info. Check that
-				 * the tuple is hinted xmin-committed because of that.
-				 */
-				if (all_visible)
-				{
-					TransactionId xmin;
-
-					if (!HeapTupleHeaderXminCommitted(htup))
-					{
-						all_visible = false;
-						break;
-					}
-
-					/*
-					 * The inserter definitely committed. But is it old enough
-					 * that everyone sees it as committed? A
-					 * FrozenTransactionId is seen as committed to everyone.
-					 * Otherwise, we check if there is a snapshot that
-					 * considers this xid to still be running, and if so, we
-					 * don't consider the page all-visible.
-					 */
-					xmin = HeapTupleHeaderGetXmin(htup);
-					if (xmin != FrozenTransactionId &&
-						!GlobalVisTestIsRemovableXid(vacrel->vistest, xmin))
-					{
-						all_visible = false;
-						break;
-					}
-
-					/* Track newest xmin on page. */
-					if (TransactionIdFollows(xmin, visibility_cutoff_xid) &&
-						TransactionIdIsNormal(xmin))
-						visibility_cutoff_xid = xmin;
-				}
 				break;
 			case HEAPTUPLE_RECENTLY_DEAD:
 
@@ -1607,7 +1559,6 @@ lazy_scan_prune(LVRelState *vacrel,
 				 * pruning.)
 				 */
 				recently_dead_tuples++;
-				all_visible = false;
 				break;
 			case HEAPTUPLE_INSERT_IN_PROGRESS:
 
@@ -1618,16 +1569,13 @@ lazy_scan_prune(LVRelState *vacrel,
 				 * results.  This assumption is a bit shaky, but it is what
 				 * acquire_sample_rows() does, so be consistent.
 				 */
-				all_visible = false;
 				break;
 			case HEAPTUPLE_DELETE_IN_PROGRESS:
-				/* This is an expected case during concurrent vacuum */
-				all_visible = false;
 
 				/*
-				 * Count such rows as live.  As above, we assume the deleting
-				 * transaction will commit and update the counters after we
-				 * report.
+				 * This an expected case during concurrent vacuum. Count such
+				 * rows as live.  As above, we assume the deleting transaction
+				 * will commit and update the counters after we report.
 				 */
 				live_tuples++;
 				break;
@@ -1670,7 +1618,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * page all-frozen afterwards (might not happen until final heap pass).
 	 */
 	if (pagefrz.freeze_required || tuples_frozen == 0 ||
-		(all_visible && all_frozen &&
+		(presult.all_visible_except_removable && all_frozen &&
 		 fpi_before != pgWalUsage.wal_fpi))
 	{
 		/*
@@ -1703,16 +1651,16 @@ lazy_scan_prune(LVRelState *vacrel,
 			vacrel->frozen_pages++;
 
 			/*
-			 * We can use visibility_cutoff_xid as our cutoff for conflicts
+			 * We can use frz_conflict_horizon 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.
 			 */
-			if (all_visible && all_frozen)
+			if (presult.all_visible_except_removable && all_frozen)
 			{
 				/* Using same cutoff when setting VM is now unnecessary */
-				snapshotConflictHorizon = visibility_cutoff_xid;
-				visibility_cutoff_xid = InvalidTransactionId;
+				snapshotConflictHorizon = presult.frz_conflict_horizon;
+				presult.frz_conflict_horizon = InvalidTransactionId;
 			}
 			else
 			{
@@ -1748,17 +1696,19 @@ lazy_scan_prune(LVRelState *vacrel,
 	 */
 #ifdef USE_ASSERT_CHECKING
 	/* Note that all_frozen value does not matter when !all_visible */
-	if (all_visible && lpdead_items == 0)
+	if (presult.all_visible)
 	{
 		TransactionId debug_cutoff;
 		bool		debug_all_frozen;
 
+		Assert(lpdead_items == 0);
+
 		if (!heap_page_is_all_visible(vacrel, buf,
 									  &debug_cutoff, &debug_all_frozen))
 			Assert(false);
 
 		Assert(!TransactionIdIsValid(debug_cutoff) ||
-			   debug_cutoff == visibility_cutoff_xid);
+			   debug_cutoff == presult.frz_conflict_horizon);
 	}
 #endif
 
@@ -1783,19 +1733,6 @@ lazy_scan_prune(LVRelState *vacrel,
 		Assert(dead_items->num_items <= dead_items->max_items);
 		pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
 									 dead_items->num_items);
-
-		/*
-		 * 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 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.  It needs
-		 * to reflect the present state of things, as expected by our caller.
-		 */
-		all_visible = false;
 	}
 
 	/* Finally, add page-local counts to whole-VACUUM counts */
@@ -1812,20 +1749,20 @@ lazy_scan_prune(LVRelState *vacrel,
 	/* Did we find LP_DEAD items? */
 	*has_lpdead_items = (lpdead_items > 0);
 
-	Assert(!all_visible || !(*has_lpdead_items));
+	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
 	 */
-	if (!all_visible_according_to_vm && all_visible)
+	if (!all_visible_according_to_vm && presult.all_visible)
 	{
 		uint8		flags = VISIBILITYMAP_ALL_VISIBLE;
 
 		if (all_frozen)
 		{
-			Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+			Assert(!TransactionIdIsValid(presult.frz_conflict_horizon));
 			flags |= VISIBILITYMAP_ALL_FROZEN;
 		}
 
@@ -1845,7 +1782,7 @@ lazy_scan_prune(LVRelState *vacrel,
 		PageSetAllVisible(page);
 		MarkBufferDirty(buf);
 		visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
-						  vmbuffer, visibility_cutoff_xid,
+						  vmbuffer, presult.frz_conflict_horizon,
 						  flags);
 	}
 
@@ -1893,7 +1830,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * 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 && all_visible &&
+	else if (all_visible_according_to_vm && presult.all_visible &&
 			 all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
 	{
 		/*
@@ -1910,11 +1847,11 @@ lazy_scan_prune(LVRelState *vacrel,
 		/*
 		 * Set the page all-frozen (and all-visible) in the VM.
 		 *
-		 * We can pass InvalidTransactionId as our visibility_cutoff_xid,
-		 * since a snapshotConflictHorizon sufficient to make everything safe
-		 * for REDO was logged when the page's tuples were frozen.
+		 * We can pass InvalidTransactionId as our frz_conflict_horizon, since
+		 * a snapshotConflictHorizon sufficient to make everything safe for
+		 * REDO was logged when the page's tuples were frozen.
 		 */
-		Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+		Assert(!TransactionIdIsValid(presult.frz_conflict_horizon));
 		visibilitymap_set(vacrel->rel, blkno, buf, InvalidXLogRecPtr,
 						  vmbuffer, InvalidTransactionId,
 						  VISIBILITYMAP_ALL_VISIBLE |
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 4b133f68593..d8e65ae7e35 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -198,6 +198,9 @@ typedef struct PruneResult
 {
 	int			ndeleted;		/* Number of tuples deleted from the page */
 	int			nnewlpdead;		/* Number of newly LP_DEAD items */
+	bool		all_visible;	/* Whether or not the page is all visible */
+	bool		all_visible_except_removable;
+	TransactionId frz_conflict_horizon; /* Newest xmin on the page */
 
 	/*
 	 * Tuple visibility is only computed once for each tuple, for correctness
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0006-Add-reference-to-VacuumCutoffs-in-HeapPageFreeze.patch"

From 23036ff6bad6a50574dac7f398fcdf9f171f7120 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sat, 6 Jan 2024 16:22:17 -0500
Subject: [PATCH v4 06/19] Add reference to VacuumCutoffs in HeapPageFreeze

Future commits will move opportunistic freezing into the main path of
pruning in heap_page_prune(). Because on-access pruning will not do
opportunistic freezing, it is cleaner to keep the visibility information
required for calling heap_prepare_freeze_tuple() inside of the
HeapPageFreeze structure itself by saving a reference to VacuumCutoffs.
---
 src/backend/access/heap/heapam.c     | 67 ++++++++++++++--------------
 src/backend/access/heap/vacuumlazy.c |  3 +-
 src/include/access/heapam.h          |  2 +-
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 34bc60f625f..7261c4988d7 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6023,7 +6023,7 @@ heap_inplace_update(Relation relation, HeapTuple tuple)
  */
 static TransactionId
 FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
-				  const struct VacuumCutoffs *cutoffs, uint16 *flags,
+				  uint16 *flags,
 				  HeapPageFreeze *pagefrz)
 {
 	TransactionId newxmax;
@@ -6049,12 +6049,12 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 		pagefrz->freeze_required = true;
 		return InvalidTransactionId;
 	}
-	else if (MultiXactIdPrecedes(multi, cutoffs->relminmxid))
+	else if (MultiXactIdPrecedes(multi, pagefrz->cutoffs->relminmxid))
 		ereport(ERROR,
 				(errcode(ERRCODE_DATA_CORRUPTED),
 				 errmsg_internal("found multixact %u from before relminmxid %u",
-								 multi, cutoffs->relminmxid)));
-	else if (MultiXactIdPrecedes(multi, cutoffs->OldestMxact))
+								 multi, pagefrz->cutoffs->relminmxid)));
+	else if (MultiXactIdPrecedes(multi, pagefrz->cutoffs->OldestMxact))
 	{
 		TransactionId update_xact;
 
@@ -6069,7 +6069,7 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 			ereport(ERROR,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 errmsg_internal("multixact %u from before multi freeze cutoff %u found to be still running",
-									 multi, cutoffs->OldestMxact)));
+									 multi, pagefrz->cutoffs->OldestMxact)));
 
 		if (HEAP_XMAX_IS_LOCKED_ONLY(t_infomask))
 		{
@@ -6080,13 +6080,13 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 
 		/* replace multi with single XID for its updater? */
 		update_xact = MultiXactIdGetUpdateXid(multi, t_infomask);
-		if (TransactionIdPrecedes(update_xact, cutoffs->relfrozenxid))
+		if (TransactionIdPrecedes(update_xact, pagefrz->cutoffs->relfrozenxid))
 			ereport(ERROR,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 errmsg_internal("multixact %u contains update XID %u from before relfrozenxid %u",
 									 multi, update_xact,
-									 cutoffs->relfrozenxid)));
-		else if (TransactionIdPrecedes(update_xact, cutoffs->OldestXmin))
+									 pagefrz->cutoffs->relfrozenxid)));
+		else if (TransactionIdPrecedes(update_xact, pagefrz->cutoffs->OldestXmin))
 		{
 			/*
 			 * Updater XID has to have aborted (otherwise the tuple would have
@@ -6098,7 +6098,7 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 						(errcode(ERRCODE_DATA_CORRUPTED),
 						 errmsg_internal("multixact %u contains committed update XID %u from before removable cutoff %u",
 										 multi, update_xact,
-										 cutoffs->OldestXmin)));
+										 pagefrz->cutoffs->OldestXmin)));
 			*flags |= FRM_INVALIDATE_XMAX;
 			pagefrz->freeze_required = true;
 			return InvalidTransactionId;
@@ -6150,9 +6150,9 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 	{
 		TransactionId xid = members[i].xid;
 
-		Assert(!TransactionIdPrecedes(xid, cutoffs->relfrozenxid));
+		Assert(!TransactionIdPrecedes(xid, pagefrz->cutoffs->relfrozenxid));
 
-		if (TransactionIdPrecedes(xid, cutoffs->FreezeLimit))
+		if (TransactionIdPrecedes(xid, pagefrz->cutoffs->FreezeLimit))
 		{
 			/* Can't violate the FreezeLimit postcondition */
 			need_replace = true;
@@ -6164,7 +6164,7 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 
 	/* Can't violate the MultiXactCutoff postcondition, either */
 	if (!need_replace)
-		need_replace = MultiXactIdPrecedes(multi, cutoffs->MultiXactCutoff);
+		need_replace = MultiXactIdPrecedes(multi, pagefrz->cutoffs->MultiXactCutoff);
 
 	if (!need_replace)
 	{
@@ -6203,7 +6203,7 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 		TransactionId xid = members[i].xid;
 		MultiXactStatus mstatus = members[i].status;
 
-		Assert(!TransactionIdPrecedes(xid, cutoffs->relfrozenxid));
+		Assert(!TransactionIdPrecedes(xid, pagefrz->cutoffs->relfrozenxid));
 
 		if (!ISUPDATE_from_mxstatus(mstatus))
 		{
@@ -6214,12 +6214,12 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 			if (TransactionIdIsCurrentTransactionId(xid) ||
 				TransactionIdIsInProgress(xid))
 			{
-				if (TransactionIdPrecedes(xid, cutoffs->OldestXmin))
+				if (TransactionIdPrecedes(xid, pagefrz->cutoffs->OldestXmin))
 					ereport(ERROR,
 							(errcode(ERRCODE_DATA_CORRUPTED),
 							 errmsg_internal("multixact %u contains running locker XID %u from before removable cutoff %u",
 											 multi, xid,
-											 cutoffs->OldestXmin)));
+											 pagefrz->cutoffs->OldestXmin)));
 				newmembers[nnewmembers++] = members[i];
 				has_lockers = true;
 			}
@@ -6277,11 +6277,11 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
 		 * We determined that updater must be kept -- add it to pending new
 		 * members list
 		 */
-		if (TransactionIdPrecedes(xid, cutoffs->OldestXmin))
+		if (TransactionIdPrecedes(xid, pagefrz->cutoffs->OldestXmin))
 			ereport(ERROR,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 errmsg_internal("multixact %u contains committed update XID %u from before removable cutoff %u",
-									 multi, xid, cutoffs->OldestXmin)));
+									 multi, xid, pagefrz->cutoffs->OldestXmin)));
 		newmembers[nnewmembers++] = members[i];
 	}
 
@@ -6373,7 +6373,6 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
  */
 bool
 heap_prepare_freeze_tuple(HeapTupleHeader tuple,
-						  const struct VacuumCutoffs *cutoffs,
 						  HeapPageFreeze *pagefrz,
 						  HeapTupleFreeze *frz, bool *totally_frozen)
 {
@@ -6401,14 +6400,14 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 		xmin_already_frozen = true;
 	else
 	{
-		if (TransactionIdPrecedes(xid, cutoffs->relfrozenxid))
+		if (TransactionIdPrecedes(xid, pagefrz->cutoffs->relfrozenxid))
 			ereport(ERROR,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 errmsg_internal("found xmin %u from before relfrozenxid %u",
-									 xid, cutoffs->relfrozenxid)));
+									 xid, pagefrz->cutoffs->relfrozenxid)));
 
 		/* Will set freeze_xmin flags in freeze plan below */
-		freeze_xmin = TransactionIdPrecedes(xid, cutoffs->OldestXmin);
+		freeze_xmin = TransactionIdPrecedes(xid, pagefrz->cutoffs->OldestXmin);
 
 		/* Verify that xmin committed if and when freeze plan is executed */
 		if (freeze_xmin)
@@ -6422,8 +6421,8 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 	xid = HeapTupleHeaderGetXvac(tuple);
 	if (TransactionIdIsNormal(xid))
 	{
-		Assert(TransactionIdPrecedesOrEquals(cutoffs->relfrozenxid, xid));
-		Assert(TransactionIdPrecedes(xid, cutoffs->OldestXmin));
+		Assert(TransactionIdPrecedesOrEquals(pagefrz->cutoffs->relfrozenxid, xid));
+		Assert(TransactionIdPrecedes(xid, pagefrz->cutoffs->OldestXmin));
 
 		/*
 		 * For Xvac, we always freeze proactively.  This allows totally_frozen
@@ -6448,8 +6447,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 		 * perform no-op xmax processing.  The only constraint is that the
 		 * FreezeLimit/MultiXactCutoff postcondition must never be violated.
 		 */
-		newxmax = FreezeMultiXactId(xid, tuple->t_infomask, cutoffs,
-									&flags, pagefrz);
+		newxmax = FreezeMultiXactId(xid, tuple->t_infomask, &flags, pagefrz);
 
 		if (flags & FRM_NOOP)
 		{
@@ -6472,7 +6470,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 			 * (This repeats work from FreezeMultiXactId, but allows "no
 			 * freeze" tracker maintenance to happen in only one place.)
 			 */
-			Assert(!MultiXactIdPrecedes(newxmax, cutoffs->MultiXactCutoff));
+			Assert(!MultiXactIdPrecedes(newxmax, pagefrz->cutoffs->MultiXactCutoff));
 			Assert(MultiXactIdIsValid(newxmax) && xid == newxmax);
 		}
 		else if (flags & FRM_RETURN_IS_XID)
@@ -6481,7 +6479,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 			 * xmax will become an updater Xid (original MultiXact's updater
 			 * member Xid will be carried forward as a simple Xid in Xmax).
 			 */
-			Assert(!TransactionIdPrecedes(newxmax, cutoffs->OldestXmin));
+			Assert(!TransactionIdPrecedes(newxmax, pagefrz->cutoffs->OldestXmin));
 
 			/*
 			 * NB -- some of these transformations are only valid because we
@@ -6505,7 +6503,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 			 * xmax is an old MultiXactId that we have to replace with a new
 			 * MultiXactId, to carry forward two or more original member XIDs.
 			 */
-			Assert(!MultiXactIdPrecedes(newxmax, cutoffs->OldestMxact));
+			Assert(!MultiXactIdPrecedes(newxmax, pagefrz->cutoffs->OldestMxact));
 
 			/*
 			 * We can't use GetMultiXactIdHintBits directly on the new multi
@@ -6540,14 +6538,14 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 	else if (TransactionIdIsNormal(xid))
 	{
 		/* Raw xmax is normal XID */
-		if (TransactionIdPrecedes(xid, cutoffs->relfrozenxid))
+		if (TransactionIdPrecedes(xid, pagefrz->cutoffs->relfrozenxid))
 			ereport(ERROR,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 errmsg_internal("found xmax %u from before relfrozenxid %u",
-									 xid, cutoffs->relfrozenxid)));
+									 xid, pagefrz->cutoffs->relfrozenxid)));
 
 		/* Will set freeze_xmax flags in freeze plan below */
-		freeze_xmax = TransactionIdPrecedes(xid, cutoffs->OldestXmin);
+		freeze_xmax = TransactionIdPrecedes(xid, pagefrz->cutoffs->OldestXmin);
 
 		/*
 		 * Verify that xmax aborted if and when freeze plan is executed,
@@ -6627,7 +6625,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 		 * Does this tuple force caller to freeze the entire page?
 		 */
 		pagefrz->freeze_required =
-			heap_tuple_should_freeze(tuple, cutoffs,
+			heap_tuple_should_freeze(tuple, pagefrz->cutoffs,
 									 &pagefrz->NoFreezePageRelfrozenXid,
 									 &pagefrz->NoFreezePageRelminMxid);
 	}
@@ -6949,8 +6947,9 @@ heap_freeze_tuple(HeapTupleHeader tuple,
 	pagefrz.NoFreezePageRelfrozenXid = FreezeLimit;
 	pagefrz.NoFreezePageRelminMxid = MultiXactCutoff;
 
-	do_freeze = heap_prepare_freeze_tuple(tuple, &cutoffs,
-										  &pagefrz, &frz, &totally_frozen);
+	pagefrz.cutoffs = &cutoffs;
+
+	do_freeze = heap_prepare_freeze_tuple(tuple, &pagefrz, &frz, &totally_frozen);
 
 	/*
 	 * Note that because this is not a WAL-logged operation, we don't need to
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index f9892f4cd08..06e0e841582 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1442,6 +1442,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	pagefrz.FreezePageRelminMxid = vacrel->NewRelminMxid;
 	pagefrz.NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
 	pagefrz.NoFreezePageRelminMxid = vacrel->NewRelminMxid;
+	pagefrz.cutoffs = &vacrel->cutoffs;
 	tuples_frozen = 0;
 	lpdead_items = 0;
 	live_tuples = 0;
@@ -1587,7 +1588,7 @@ lazy_scan_prune(LVRelState *vacrel,
 		hastup = true;			/* page makes rel truncation unsafe */
 
 		/* Tuple with storage -- consider need to freeze */
-		if (heap_prepare_freeze_tuple(htup, &vacrel->cutoffs, &pagefrz,
+		if (heap_prepare_freeze_tuple(htup, &pagefrz,
 									  &frozen[tuples_frozen], &totally_frozen))
 		{
 			/* Save prepared freeze plan for later */
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index d8e65ae7e35..297ba03bf09 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -189,6 +189,7 @@ typedef struct HeapPageFreeze
 	TransactionId NoFreezePageRelfrozenXid;
 	MultiXactId NoFreezePageRelminMxid;
 
+	struct VacuumCutoffs *cutoffs;
 } HeapPageFreeze;
 
 /*
@@ -295,7 +296,6 @@ extern TM_Result heap_lock_tuple(Relation relation, HeapTuple tuple,
 
 extern void heap_inplace_update(Relation relation, HeapTuple tuple);
 extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
-									  const struct VacuumCutoffs *cutoffs,
 									  HeapPageFreeze *pagefrz,
 									  HeapTupleFreeze *frz, bool *totally_frozen);
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0007-Prepare-freeze-tuples-in-heap_page_prune.patch"

From 470e695363d881d48d1fe839ea6770938f20d078 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 18 Mar 2024 20:01:38 -0400
Subject: [PATCH v4 07/19] Prepare freeze tuples in heap_page_prune()

In order to combine the freeze and prune records, we must determine
which tuples are freezable before actually executing pruning. All of the
page modifications should be made in the same critical section along
with emitting the combined WAL. Determine whether or not tuples should
or must be frozen and whether or not the page will be all frozen as a
consequence during pruning.
---
 src/backend/access/heap/pruneheap.c  | 41 +++++++++++++++--
 src/backend/access/heap/vacuumlazy.c | 68 +++++++---------------------
 src/include/access/heapam.h          | 12 +++++
 3 files changed, 66 insertions(+), 55 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bd30296ef1a..afc5ea5e0e7 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -153,7 +153,7 @@ 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(relation, buffer, vistest, false,
+			heap_page_prune(relation, buffer, vistest, false, NULL,
 							&presult, NULL);
 
 			/*
@@ -206,6 +206,9 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
  * mark_unused_now indicates whether or not dead items can be set LP_UNUSED
  * during pruning.
  *
+ * pagefrz contains both input and output parameters used if the caller is
+ * interested in potentially freezing tuples on the page.
+ *
  * presult contains output parameters needed by callers such as the number of
  * tuples removed and the number of line pointers newly marked LP_DEAD.
  * heap_page_prune() is responsible for initializing it.
@@ -217,6 +220,7 @@ void
 heap_page_prune(Relation relation, Buffer buffer,
 				GlobalVisState *vistest,
 				bool mark_unused_now,
+				HeapPageFreeze *pagefrz,
 				PruneResult *presult,
 				OffsetNumber *off_loc)
 {
@@ -247,11 +251,16 @@ heap_page_prune(Relation relation, Buffer buffer,
 
 	presult->ndeleted = 0;
 	presult->nnewlpdead = 0;
+	presult->nfrozen = 0;
 
 	/*
-	 * Keep track of whether or not the page is all_visible in case the caller
-	 * wants to use this information to update the VM.
+	 * Caller will update the VM after pruning, collecting LP_DEAD items, and
+	 * freezing tuples. Keep track of whether or not the page is all_visible
+	 * and all_frozen and use this information to update the VM. all_visible
+	 * implies lpdead_items == 0, but don't trust all_frozen result unless
+	 * all_visible is also set to true.
 	 */
+	presult->all_frozen = true;
 	presult->all_visible = true;
 	/* for recovery conflicts */
 	presult->frz_conflict_horizon = InvalidTransactionId;
@@ -381,6 +390,32 @@ heap_page_prune(Relation relation, Buffer buffer,
 				elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
 				break;
 		}
+
+		/*
+		 * Consider freezing any normal tuples which will not be removed
+		 */
+		if (presult->htsv[offnum] != HEAPTUPLE_DEAD && pagefrz)
+		{
+			bool		totally_frozen;
+
+			/* Tuple with storage -- consider need to freeze */
+			if ((heap_prepare_freeze_tuple(htup, pagefrz,
+										   &presult->frozen[presult->nfrozen],
+										   &totally_frozen)))
+			{
+				/* Save prepared freeze plan for later */
+				presult->frozen[presult->nfrozen++].offset = offnum;
+			}
+
+			/*
+			 * If any tuple isn't either totally frozen already or eligible to
+			 * become totally frozen (according to its freeze plan), then the
+			 * page definitely cannot be set all-frozen in the visibility map
+			 * later on
+			 */
+			if (!totally_frozen)
+				presult->all_frozen = false;
+		}
 	}
 
 	/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 06e0e841582..4187c998d25 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1416,16 +1416,13 @@ lazy_scan_prune(LVRelState *vacrel,
 				maxoff;
 	ItemId		itemid;
 	PruneResult presult;
-	int			tuples_frozen,
-				lpdead_items,
+	int			lpdead_items,
 				live_tuples,
 				recently_dead_tuples;
 	HeapPageFreeze pagefrz;
 	bool		hastup = false;
-	bool		all_frozen;
 	int64		fpi_before = pgWalUsage.wal_fpi;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
-	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 
 	Assert(BufferGetBlockNumber(buf) == blkno);
 
@@ -1443,7 +1440,6 @@ lazy_scan_prune(LVRelState *vacrel,
 	pagefrz.NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
 	pagefrz.NoFreezePageRelminMxid = vacrel->NewRelminMxid;
 	pagefrz.cutoffs = &vacrel->cutoffs;
-	tuples_frozen = 0;
 	lpdead_items = 0;
 	live_tuples = 0;
 	recently_dead_tuples = 0;
@@ -1461,31 +1457,20 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * false otherwise.
 	 */
 	heap_page_prune(rel, buf, vacrel->vistest, vacrel->nindexes == 0,
-					&presult, &vacrel->offnum);
+					&pagefrz, &presult, &vacrel->offnum);
 
 	/*
 	 * Now scan the page to collect LP_DEAD items and check for tuples
 	 * requiring freezing among remaining tuples with storage. 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. Keep track of
-	 * whether or not the page is all_frozen and use this information to
-	 * update the VM. all_visible implies lpdead_items == 0, but don't trust
-	 * all_frozen result unless all_visible is also set to true.
+	 * have determined whether or not the page is all_visible and able to
+	 * become all_frozen.
 	 *
 	 */
-	all_frozen = true;
-
-	/*
-	 * Now scan the page to collect LP_DEAD items and update the variables set
-	 * just above.
-	 */
 	for (offnum = FirstOffsetNumber;
 		 offnum <= maxoff;
 		 offnum = OffsetNumberNext(offnum))
 	{
-		HeapTupleHeader htup;
-		bool		totally_frozen;
-
 		/*
 		 * Set the offset number so that we can display it along with any
 		 * error that occurred while processing this tuple.
@@ -1521,8 +1506,6 @@ lazy_scan_prune(LVRelState *vacrel,
 
 		Assert(ItemIdIsNormal(itemid));
 
-		htup = (HeapTupleHeader) PageGetItem(page, itemid);
-
 		/*
 		 * The criteria for counting a tuple as live in this block need to
 		 * match what analyze.c's acquire_sample_rows() does, otherwise VACUUM
@@ -1587,29 +1570,8 @@ lazy_scan_prune(LVRelState *vacrel,
 
 		hastup = true;			/* page makes rel truncation unsafe */
 
-		/* Tuple with storage -- consider need to freeze */
-		if (heap_prepare_freeze_tuple(htup, &pagefrz,
-									  &frozen[tuples_frozen], &totally_frozen))
-		{
-			/* Save prepared freeze plan for later */
-			frozen[tuples_frozen++].offset = offnum;
-		}
-
-		/*
-		 * If any tuple isn't either totally frozen already or eligible to
-		 * become totally frozen (according to its freeze plan), then the page
-		 * definitely cannot be set all-frozen in the visibility map later on
-		 */
-		if (!totally_frozen)
-			all_frozen = false;
 	}
 
-	/*
-	 * We have now divided every item on the page into either an LP_DEAD item
-	 * that will need to be vacuumed in indexes later, or a LP_NORMAL tuple
-	 * that remains and needs to be considered for freezing now (LP_UNUSED and
-	 * LP_REDIRECT items also remain, but are of no further interest to us).
-	 */
 	vacrel->offnum = InvalidOffsetNumber;
 
 	/*
@@ -1618,8 +1580,8 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * freeze when pruning generated an FPI, if doing so means that we set the
 	 * page all-frozen afterwards (might not happen until final heap pass).
 	 */
-	if (pagefrz.freeze_required || tuples_frozen == 0 ||
-		(presult.all_visible_except_removable && all_frozen &&
+	if (pagefrz.freeze_required || presult.nfrozen == 0 ||
+		(presult.all_visible_except_removable && presult.all_frozen &&
 		 fpi_before != pgWalUsage.wal_fpi))
 	{
 		/*
@@ -1629,7 +1591,7 @@ lazy_scan_prune(LVRelState *vacrel,
 		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
 		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
 
-		if (tuples_frozen == 0)
+		if (presult.nfrozen == 0)
 		{
 			/*
 			 * We have no freeze plans to execute, so there's no added cost
@@ -1657,7 +1619,7 @@ lazy_scan_prune(LVRelState *vacrel,
 			 * once we're done with it.  Otherwise we generate a conservative
 			 * cutoff by stepping back from OldestXmin.
 			 */
-			if (presult.all_visible_except_removable && all_frozen)
+			if (presult.all_visible_except_removable && presult.all_frozen)
 			{
 				/* Using same cutoff when setting VM is now unnecessary */
 				snapshotConflictHorizon = presult.frz_conflict_horizon;
@@ -1673,7 +1635,7 @@ lazy_scan_prune(LVRelState *vacrel,
 			/* Execute all freeze plans for page as a single atomic action */
 			heap_freeze_execute_prepared(vacrel->rel, buf,
 										 snapshotConflictHorizon,
-										 frozen, tuples_frozen);
+										 presult.frozen, presult.nfrozen);
 		}
 	}
 	else
@@ -1684,8 +1646,8 @@ lazy_scan_prune(LVRelState *vacrel,
 		 */
 		vacrel->NewRelfrozenXid = pagefrz.NoFreezePageRelfrozenXid;
 		vacrel->NewRelminMxid = pagefrz.NoFreezePageRelminMxid;
-		all_frozen = false;
-		tuples_frozen = 0;		/* avoid miscounts in instrumentation */
+		presult.all_frozen = false;
+		presult.nfrozen = 0;	/* avoid miscounts in instrumentation */
 	}
 
 	/*
@@ -1708,6 +1670,8 @@ lazy_scan_prune(LVRelState *vacrel,
 									  &debug_cutoff, &debug_all_frozen))
 			Assert(false);
 
+		Assert(presult.all_frozen == debug_all_frozen);
+
 		Assert(!TransactionIdIsValid(debug_cutoff) ||
 			   debug_cutoff == presult.frz_conflict_horizon);
 	}
@@ -1738,7 +1702,7 @@ lazy_scan_prune(LVRelState *vacrel,
 
 	/* Finally, add page-local counts to whole-VACUUM counts */
 	vacrel->tuples_deleted += presult.ndeleted;
-	vacrel->tuples_frozen += tuples_frozen;
+	vacrel->tuples_frozen += presult.nfrozen;
 	vacrel->lpdead_items += lpdead_items;
 	vacrel->live_tuples += live_tuples;
 	vacrel->recently_dead_tuples += recently_dead_tuples;
@@ -1761,7 +1725,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	{
 		uint8		flags = VISIBILITYMAP_ALL_VISIBLE;
 
-		if (all_frozen)
+		if (presult.all_frozen)
 		{
 			Assert(!TransactionIdIsValid(presult.frz_conflict_horizon));
 			flags |= VISIBILITYMAP_ALL_FROZEN;
@@ -1832,7 +1796,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * true, so we must check both all_visible and all_frozen.
 	 */
 	else if (all_visible_according_to_vm && presult.all_visible &&
-			 all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
+			 presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
 	{
 		/*
 		 * Avoid relying on all_visible_according_to_vm as a proxy for the
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 297ba03bf09..2339abfd28a 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -201,6 +201,11 @@ typedef struct PruneResult
 	int			nnewlpdead;		/* Number of newly LP_DEAD items */
 	bool		all_visible;	/* Whether or not the page is all visible */
 	bool		all_visible_except_removable;
+	/* Whether or not the page can be set all frozen in the VM */
+	bool		all_frozen;
+
+	/* Number of newly frozen tuples */
+	int			nfrozen;
 	TransactionId frz_conflict_horizon; /* Newest xmin on the page */
 
 	/*
@@ -213,6 +218,12 @@ typedef struct PruneResult
 	 * 1. Otherwise every access would need to subtract 1.
 	 */
 	int8		htsv[MaxHeapTuplesPerPage + 1];
+
+
+	/*
+	 * One entry for every tuple that we may freeze.
+	 */
+	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 } PruneResult;
 
 /*
@@ -324,6 +335,7 @@ extern void heap_page_prune_opt(Relation relation, Buffer buffer);
 extern void heap_page_prune(Relation relation, Buffer buffer,
 							struct GlobalVisState *vistest,
 							bool mark_unused_now,
+							HeapPageFreeze *pagefrz,
 							PruneResult *presult,
 							OffsetNumber *off_loc);
 extern void heap_page_prune_execute(Buffer buffer,
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0008-lazy_scan_prune-reorder-freeze-execution-logic.patch"

From 3c208039193ce94111e8ddc1b03828cf820e11e3 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 19 Mar 2024 19:30:16 -0400
Subject: [PATCH v4 08/19] lazy_scan_prune reorder freeze execution logic

To combine the prune and freeze records, freezing must be done before a
pruning WAL record is emitted. We will move the freeze execution into
heap_page_prune() in future commits. lazy_scan_prune() currently
executes freezing, updates vacrel->NewRelfrozenXid and
vacrel->NewRelminMxid, and resets the snapshotConflictHorizon that the
visibility map update record may use in the same block of if statements.

This commit starts reordering that logic so that the freeze execution
can be separated from the other updates which should not be done in
pruning.
---
 src/backend/access/heap/vacuumlazy.c | 92 +++++++++++++++-------------
 1 file changed, 49 insertions(+), 43 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 4187c998d25..74ebab25a95 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1421,6 +1421,7 @@ lazy_scan_prune(LVRelState *vacrel,
 				recently_dead_tuples;
 	HeapPageFreeze pagefrz;
 	bool		hastup = false;
+	bool		do_freeze;
 	int64		fpi_before = pgWalUsage.wal_fpi;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
 
@@ -1580,10 +1581,15 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * freeze when pruning generated an FPI, if doing so means that we set the
 	 * page all-frozen afterwards (might not happen until final heap pass).
 	 */
-	if (pagefrz.freeze_required || presult.nfrozen == 0 ||
+	do_freeze = pagefrz.freeze_required ||
 		(presult.all_visible_except_removable && presult.all_frozen &&
-		 fpi_before != pgWalUsage.wal_fpi))
+		 presult.nfrozen > 0 &&
+		 fpi_before != pgWalUsage.wal_fpi);
+
+	if (do_freeze)
 	{
+		TransactionId snapshotConflictHorizon;
+
 		/*
 		 * We're freezing the page.  Our final NewRelfrozenXid doesn't need to
 		 * be affected by the XIDs that are just about to be frozen anyway.
@@ -1591,52 +1597,52 @@ lazy_scan_prune(LVRelState *vacrel,
 		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
 		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
 
-		if (presult.nfrozen == 0)
-		{
-			/*
-			 * We have no freeze plans to execute, so there's no added cost
-			 * from following the freeze path.  That's why it was chosen. This
-			 * is important in the case where the page only contains totally
-			 * frozen tuples at this point (perhaps only following pruning).
-			 * Such pages can be marked all-frozen in the VM by our caller,
-			 * even though none of its tuples were newly frozen here (note
-			 * that the "no freeze" path never sets pages all-frozen).
-			 *
-			 * We never increment the frozen_pages instrumentation counter
-			 * here, since it only counts pages with newly frozen tuples
-			 * (don't confuse that with pages newly set all-frozen in VM).
-			 */
-		}
+		vacrel->frozen_pages++;
+
+		/*
+		 * We can use frz_conflict_horizon 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.
+		 */
+		if (presult.all_visible_except_removable && presult.all_frozen)
+			snapshotConflictHorizon = presult.frz_conflict_horizon;
 		else
 		{
-			TransactionId snapshotConflictHorizon;
+			/* Avoids false conflicts when hot_standby_feedback in use */
+			snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin;
+			TransactionIdRetreat(snapshotConflictHorizon);
+		}
 
-			vacrel->frozen_pages++;
+		/* Using same cutoff when setting VM is now unnecessary */
+		if (presult.all_visible_except_removable && presult.all_frozen)
+			presult.frz_conflict_horizon = InvalidTransactionId;
 
-			/*
-			 * We can use frz_conflict_horizon 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.
-			 */
-			if (presult.all_visible_except_removable && presult.all_frozen)
-			{
-				/* Using same cutoff when setting VM is now unnecessary */
-				snapshotConflictHorizon = presult.frz_conflict_horizon;
-				presult.frz_conflict_horizon = InvalidTransactionId;
-			}
-			else
-			{
-				/* Avoids false conflicts when hot_standby_feedback in use */
-				snapshotConflictHorizon = vacrel->cutoffs.OldestXmin;
-				TransactionIdRetreat(snapshotConflictHorizon);
-			}
+		/* Execute all freeze plans for page as a single atomic action */
+		heap_freeze_execute_prepared(vacrel->rel, buf,
+									 snapshotConflictHorizon,
+									 presult.frozen, presult.nfrozen);
+	}
+	else if (presult.all_frozen && presult.nfrozen == 0)
+	{
+		/* Page should be all visible except to-be-removed tuples */
+		Assert(presult.all_visible_except_removable);
 
-			/* Execute all freeze plans for page as a single atomic action */
-			heap_freeze_execute_prepared(vacrel->rel, buf,
-										 snapshotConflictHorizon,
-										 presult.frozen, presult.nfrozen);
-		}
+		/*
+		 * We have no freeze plans to execute, so there's no added cost from
+		 * following the freeze path.  That's why it was chosen. This is
+		 * important in the case where the page only contains totally frozen
+		 * tuples at this point (perhaps only following pruning). Such pages
+		 * can be marked all-frozen in the VM by our caller, even though none
+		 * of its tuples were newly frozen here (note that the "no freeze"
+		 * path never sets pages all-frozen).
+		 *
+		 * We never increment the frozen_pages instrumentation counter here,
+		 * since it only counts pages with newly frozen tuples (don't confuse
+		 * that with pages newly set all-frozen in VM).
+		 */
+		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
+		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
 	}
 	else
 	{
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0009-Execute-freezing-in-heap_page_prune.patch"

From aec400aa0289ecb8cab6dfaeb8fee050db6487c3 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 8 Mar 2024 16:45:57 -0500
Subject: [PATCH v4 09/19] Execute freezing in heap_page_prune()

As a step toward combining the prune and freeze WAL records, execute
freezing in heap_page_prune(). The logic to determine whether or not to
execute freeze plans was moved from lazy_scan_prune() over to
heap_page_prune() with little modification.
---
 src/backend/access/heap/heapam_handler.c |   2 +-
 src/backend/access/heap/pruneheap.c      | 155 ++++++++++++++++++-----
 src/backend/access/heap/vacuumlazy.c     | 134 +++++---------------
 src/backend/storage/ipc/procarray.c      |   6 +-
 src/include/access/heapam.h              |  39 +++---
 src/tools/pgindent/typedefs.list         |   2 +-
 6 files changed, 182 insertions(+), 156 deletions(-)

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 680a50bf8b1..5e522f5b0ba 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -1046,7 +1046,7 @@ heapam_scan_analyze_next_tuple(TableScanDesc scan, TransactionId OldestXmin,
 		 * We ignore unused and redirect line pointers.  DEAD line pointers
 		 * should be counted as dead, because we need vacuum to run to get rid
 		 * of them.  Note that this rule agrees with the way that
-		 * heap_page_prune() counts things.
+		 * heap_page_prune_and_freeze() counts things.
 		 */
 		if (!ItemIdIsNormal(itemid))
 		{
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index afc5ea5e0e7..20907ba5408 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -17,16 +17,19 @@
 #include "access/heapam.h"
 #include "access/heapam_xlog.h"
 #include "access/htup_details.h"
+#include "access/multixact.h"
 #include "access/transam.h"
 #include "access/xlog.h"
+#include "commands/vacuum.h"
 #include "access/xloginsert.h"
+#include "executor/instrument.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "storage/bufmgr.h"
 #include "utils/rel.h"
 #include "utils/snapmgr.h"
 
-/* Working data for heap_page_prune and subroutines */
+/* Working data for heap_page_prune_and_freeze() and subroutines */
 typedef struct
 {
 	/* tuple visibility test, initialized for the relation */
@@ -51,6 +54,11 @@ typedef struct
 	 * 1. Otherwise every access would need to subtract 1.
 	 */
 	bool		marked[MaxHeapTuplesPerPage + 1];
+
+	/*
+	 * One entry for every tuple that we may freeze.
+	 */
+	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 } PruneState;
 
 /* Local functions */
@@ -59,14 +67,15 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
 											   Buffer buffer);
 static int	heap_prune_chain(Buffer buffer,
 							 OffsetNumber rootoffnum,
-							 PruneState *prstate, PruneResult *presult);
+							 PruneState *prstate, PruneFreezeResult *presult);
+
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
 									   OffsetNumber offnum, OffsetNumber rdoffnum);
 static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
-								   PruneResult *presult);
+								   PruneFreezeResult *presult);
 static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
-											 PruneResult *presult);
+											 PruneFreezeResult *presult);
 static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
 static void page_verify_redirects(Page page);
 
@@ -146,15 +155,15 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 		 */
 		if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
 		{
-			PruneResult presult;
+			PruneFreezeResult presult;
 
 			/*
 			 * For now, pass mark_unused_now as false regardless of whether or
 			 * not the relation has indexes, since we cannot safely determine
 			 * that during on-access pruning with the current implementation.
 			 */
-			heap_page_prune(relation, buffer, vistest, false, NULL,
-							&presult, NULL);
+			heap_page_prune_and_freeze(relation, buffer, vistest, false, NULL,
+									   &presult, NULL);
 
 			/*
 			 * Report the number of tuples reclaimed to pgstats.  This is
@@ -188,7 +197,12 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 
 
 /*
- * Prune and repair fragmentation in the specified page.
+ * Prune and repair fragmentation and potentially freeze tuples on the
+ * specified page.
+ *
+ * If the page can be marked all-frozen in the visibility map, we may
+ * opportunistically freeze tuples on the page if either its tuples are old
+ * enough or freezing will be cheap enough.
  *
  * 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
@@ -206,23 +220,24 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
  * mark_unused_now indicates whether or not dead items can be set LP_UNUSED
  * during pruning.
  *
- * pagefrz contains both input and output parameters used if the caller is
- * interested in potentially freezing tuples on the page.
+ * pagefrz is an input parameter containing visibility cutoff information and
+ * the current relfrozenxid and relminmxids used if the caller is interested in
+ * freezing tuples on the page.
  *
  * presult contains output parameters needed by callers such as the number of
  * tuples removed and the number of line pointers newly marked LP_DEAD.
- * heap_page_prune() is responsible for initializing it.
+ * heap_page_prune_and_freeze() is responsible for initializing it.
  *
  * off_loc is the offset location required by the caller to use in error
  * callback.
  */
 void
-heap_page_prune(Relation relation, Buffer buffer,
-				GlobalVisState *vistest,
-				bool mark_unused_now,
-				HeapPageFreeze *pagefrz,
-				PruneResult *presult,
-				OffsetNumber *off_loc)
+heap_page_prune_and_freeze(Relation relation, Buffer buffer,
+						   GlobalVisState *vistest,
+						   bool mark_unused_now,
+						   HeapPageFreeze *pagefrz,
+						   PruneFreezeResult *presult,
+						   OffsetNumber *off_loc)
 {
 	Page		page = BufferGetPage(buffer);
 	BlockNumber blockno = BufferGetBlockNumber(buffer);
@@ -230,6 +245,8 @@ heap_page_prune(Relation relation, Buffer buffer,
 				maxoff;
 	PruneState	prstate;
 	HeapTupleData tup;
+	bool		do_freeze;
+	int64		fpi_before = pgWalUsage.wal_fpi;
 
 	/*
 	 * First, initialize the new pd_prune_xid value to zero (indicating no
@@ -265,6 +282,10 @@ heap_page_prune(Relation relation, Buffer buffer,
 	/* for recovery conflicts */
 	presult->frz_conflict_horizon = InvalidTransactionId;
 
+	/* For advancing relfrozenxid and relminmxid */
+	presult->new_relfrozenxid = InvalidTransactionId;
+	presult->new_relminmxid = InvalidMultiXactId;
+
 	maxoff = PageGetMaxOffsetNumber(page);
 	tup.t_tableOid = RelationGetRelid(relation);
 
@@ -400,11 +421,11 @@ heap_page_prune(Relation relation, Buffer buffer,
 
 			/* Tuple with storage -- consider need to freeze */
 			if ((heap_prepare_freeze_tuple(htup, pagefrz,
-										   &presult->frozen[presult->nfrozen],
+										   &prstate.frozen[presult->nfrozen],
 										   &totally_frozen)))
 			{
 				/* Save prepared freeze plan for later */
-				presult->frozen[presult->nfrozen++].offset = offnum;
+				prstate.frozen[presult->nfrozen++].offset = offnum;
 			}
 
 			/*
@@ -557,6 +578,72 @@ heap_page_prune(Relation relation, Buffer buffer,
 
 	/* Record number of newly-set-LP_DEAD items for caller */
 	presult->nnewlpdead = prstate.ndead;
+
+	/*
+	 * Freeze the page when heap_prepare_freeze_tuple indicates that at least
+	 * one XID/MXID from before FreezeLimit/MultiXactCutoff is present.  Also
+	 * freeze when pruning generated an FPI, if doing so means that we set the
+	 * page all-frozen afterwards (might not happen until final heap pass).
+	 */
+	if (pagefrz)
+		do_freeze = pagefrz->freeze_required ||
+			(presult->all_visible_except_removable && presult->all_frozen &&
+			 presult->nfrozen > 0 &&
+			 fpi_before != pgWalUsage.wal_fpi);
+	else
+		do_freeze = false;
+
+	if (do_freeze)
+	{
+		/*
+		 * We can use frz_conflict_horizon 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.
+		 */
+		if (!(presult->all_visible_except_removable && presult->all_frozen))
+		{
+			/* Avoids false conflicts when hot_standby_feedback in use */
+			presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+			TransactionIdRetreat(presult->frz_conflict_horizon);
+		}
+
+		/* Execute all freeze plans for page as a single atomic action */
+		heap_freeze_execute_prepared(relation, buffer,
+									 presult->frz_conflict_horizon,
+									 prstate.frozen, presult->nfrozen);
+	}
+	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
+	{
+		/*
+		 * If we will neither freeze tuples on the page nor set the page all
+		 * frozen in the visibility map, the page is not all frozen and there
+		 * will be no newly frozen tuples.
+		 */
+		presult->all_frozen = false;
+		presult->nfrozen = 0;	/* avoid miscounts in instrumentation */
+	}
+
+	/* Caller won't update new_relfrozenxid and new_relminmxid */
+	if (!pagefrz)
+		return;
+
+	/*
+	 * If we will freeze tuples on the page or, even if we don't freeze tuples
+	 * on the page, if we will set the page all-frozen in the visibility map,
+	 * we can advance relfrozenxid and relminmxid to the values in
+	 * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid.
+	 */
+	if (presult->all_frozen || presult->nfrozen > 0)
+	{
+		presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
+		presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
+	}
+	else
+	{
+		presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
+		presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
+	}
 }
 
 
@@ -614,7 +701,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
  */
 static int
 heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
-				 PruneState *prstate, PruneResult *presult)
+				 PruneState *prstate, PruneFreezeResult *presult)
 {
 	int			ndeleted = 0;
 	Page		dp = (Page) BufferGetPage(buffer);
@@ -879,10 +966,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 	{
 		/*
 		 * We found a redirect item that doesn't point to a valid follow-on
-		 * item.  This can happen if the loop in heap_page_prune caused us to
-		 * visit the dead successor of a redirect item before visiting the
-		 * redirect item.  We can clean up by setting the redirect item to
-		 * DEAD state or LP_UNUSED if the caller indicated.
+		 * item.  This can happen if the loop in heap_page_prune_and_freeze()
+		 * caused us to visit the dead successor of a redirect item before
+		 * visiting the redirect item.  We can clean up by setting the
+		 * redirect item to DEAD state or LP_UNUSED if the caller indicated.
 		 */
 		heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
 	}
@@ -922,7 +1009,7 @@ heap_prune_record_redirect(PruneState *prstate,
 /* Record line pointer to be marked dead */
 static void
 heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
-					   PruneResult *presult)
+					   PruneFreezeResult *presult)
 {
 	Assert(prstate->ndead < MaxHeapTuplesPerPage);
 	prstate->nowdead[prstate->ndead] = offnum;
@@ -945,7 +1032,7 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
  */
 static void
 heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
-								 PruneResult *presult)
+								 PruneFreezeResult *presult)
 {
 	/*
 	 * If the caller set mark_unused_now to true, we can remove dead tuples
@@ -972,9 +1059,9 @@ heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum)
 
 
 /*
- * Perform the actual page changes needed by heap_page_prune.
- * It is expected that the caller has a full cleanup lock on the
- * buffer.
+ * Perform the actual page pruning modifications needed by
+ * heap_page_prune_and_freeze(). It is expected that the caller has a full
+ * cleanup lock on the buffer.
  */
 void
 heap_page_prune_execute(Buffer buffer,
@@ -1088,11 +1175,11 @@ heap_page_prune_execute(Buffer buffer,
 #ifdef USE_ASSERT_CHECKING
 
 		/*
-		 * When heap_page_prune() was called, mark_unused_now may have been
-		 * passed as true, which allows would-be LP_DEAD items to be made
-		 * LP_UNUSED instead. This is only possible if the relation has no
-		 * indexes. If there are any dead items, then mark_unused_now was not
-		 * true and every item being marked LP_UNUSED must refer to a
+		 * When heap_page_prune_and_freeze() was called, mark_unused_now may
+		 * have been passed as true, which allows would-be LP_DEAD items to be
+		 * made LP_UNUSED instead. This is only possible if the relation has
+		 * no indexes. If there are any dead items, then mark_unused_now was
+		 * not true and every item being marked LP_UNUSED must refer to a
 		 * heap-only tuple.
 		 */
 		if (ndead > 0)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 74ebab25a95..c4553a4159c 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -430,12 +430,12 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
 	 * as an upper bound on the XIDs stored in the pages we'll actually scan
 	 * (NewRelfrozenXid tracking must never be allowed to miss unfrozen XIDs).
 	 *
-	 * Next acquire vistest, a related cutoff that's used in heap_page_prune.
-	 * We expect vistest will always make heap_page_prune remove any deleted
-	 * tuple whose xmax is < OldestXmin.  lazy_scan_prune must never become
-	 * confused about whether a tuple should be frozen or removed.  (In the
-	 * future we might want to teach lazy_scan_prune to recompute vistest from
-	 * time to time, to increase the number of dead tuples it can prune away.)
+	 * Next acquire vistest, a related cutoff that's used in
+	 * heap_page_prune_and_freeze(). We expect vistest will always make
+	 * heap_page_prune_and_freeze() remove any deleted tuple whose xmax is <
+	 * OldestXmin. (In the future we might want to teach lazy_scan_prune to
+	 * recompute vistest from time to time, to increase the number of dead
+	 * tuples it can prune away.)
 	 */
 	vacrel->aggressive = vacuum_get_cutoffs(rel, params, &vacrel->cutoffs);
 	vacrel->rel_pages = orig_rel_pages = RelationGetNumberOfBlocks(rel);
@@ -1378,21 +1378,21 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
  *
  * Caller must hold pin and buffer cleanup lock on the buffer.
  *
- * Prior to PostgreSQL 14 there were very rare cases where heap_page_prune()
- * was allowed to disagree with our HeapTupleSatisfiesVacuum() call about
- * whether or not a tuple should be considered DEAD.  This happened when an
- * inserting transaction concurrently aborted (after our heap_page_prune()
- * call, before our HeapTupleSatisfiesVacuum() call).  There was rather a lot
- * of complexity just so we could deal with tuples that were DEAD to VACUUM,
- * but nevertheless were left with storage after pruning.
+ * Prior to PostgreSQL 14 there were very rare cases where
+ * heap_page_prune_and_freeze() was allowed to disagree with our
+ * HeapTupleSatisfiesVacuum() call about whether or not a tuple should be
+ * considered DEAD.  This happened when an inserting transaction concurrently
+ * aborted (after our heap_page_prune_and_freeze() call, before our
+ * HeapTupleSatisfiesVacuum() call).  There was rather a lot of complexity just
+ * so we could deal with tuples that were DEAD to VACUUM, but nevertheless were
+ * left with storage after pruning.
  *
  * As of Postgres 17, we circumvent this problem altogether by reusing the
- * result of heap_page_prune()'s visibility check. Without the second call to
- * HeapTupleSatisfiesVacuum(), there is no new HTSV_Result and there can be no
- * disagreement. We'll just handle such tuples as if they had become fully dead
- * right after this operation completes instead of in the middle of it. Note that
- * any tuple that becomes dead after the call to heap_page_prune() can't need to
- * be frozen, because it was visible to another session when vacuum started.
+ * result of heap_page_prune_and_freeze()'s visibility check. Without the
+ * second call to HeapTupleSatisfiesVacuum(), there is no new HTSV_Result and
+ * there can be no disagreement. We'll just handle such tuples as if they had
+ * become fully dead right after this operation completes instead of in the
+ * middle of it.
  *
  * vmbuffer is the buffer containing the VM block with visibility information
  * for the heap block, blkno. all_visible_according_to_vm is the saved
@@ -1415,26 +1415,24 @@ lazy_scan_prune(LVRelState *vacrel,
 	OffsetNumber offnum,
 				maxoff;
 	ItemId		itemid;
-	PruneResult presult;
+	PruneFreezeResult presult;
 	int			lpdead_items,
 				live_tuples,
 				recently_dead_tuples;
 	HeapPageFreeze pagefrz;
 	bool		hastup = false;
-	bool		do_freeze;
-	int64		fpi_before = pgWalUsage.wal_fpi;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
 
 	Assert(BufferGetBlockNumber(buf) == blkno);
 
 	/*
 	 * maxoff might be reduced following line pointer array truncation in
-	 * heap_page_prune.  That's safe for us to ignore, since the reclaimed
-	 * space will continue to look like LP_UNUSED items below.
+	 * heap_page_prune_and_freeze().  That's safe for us to ignore, since the
+	 * reclaimed space will continue to look like LP_UNUSED items below.
 	 */
 	maxoff = PageGetMaxOffsetNumber(page);
 
-	/* Initialize (or reset) page-level state */
+	/* Initialize pagefrz */
 	pagefrz.freeze_required = false;
 	pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
 	pagefrz.FreezePageRelminMxid = vacrel->NewRelminMxid;
@@ -1446,7 +1444,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	recently_dead_tuples = 0;
 
 	/*
-	 * Prune all HOT-update chains in this page.
+	 * Prune all HOT-update chains and potentially freeze tuples on this page.
 	 *
 	 * We count the number of tuples removed from the page by the pruning step
 	 * in presult.ndeleted. It should not be confused with lpdead_items;
@@ -1457,8 +1455,8 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * items LP_UNUSED, so mark_unused_now should be true if no indexes and
 	 * false otherwise.
 	 */
-	heap_page_prune(rel, buf, vacrel->vistest, vacrel->nindexes == 0,
-					&pagefrz, &presult, &vacrel->offnum);
+	heap_page_prune_and_freeze(rel, buf, vacrel->vistest, vacrel->nindexes == 0,
+							   &pagefrz, &presult, &vacrel->offnum);
 
 	/*
 	 * Now scan the page to collect LP_DEAD items and check for tuples
@@ -1575,85 +1573,23 @@ lazy_scan_prune(LVRelState *vacrel,
 
 	vacrel->offnum = InvalidOffsetNumber;
 
-	/*
-	 * Freeze the page when heap_prepare_freeze_tuple indicates that at least
-	 * one XID/MXID from before FreezeLimit/MultiXactCutoff is present.  Also
-	 * freeze when pruning generated an FPI, if doing so means that we set the
-	 * page all-frozen afterwards (might not happen until final heap pass).
-	 */
-	do_freeze = pagefrz.freeze_required ||
-		(presult.all_visible_except_removable && presult.all_frozen &&
-		 presult.nfrozen > 0 &&
-		 fpi_before != pgWalUsage.wal_fpi);
+	Assert(MultiXactIdIsValid(presult.new_relminmxid));
+	vacrel->NewRelfrozenXid = presult.new_relfrozenxid;
+	Assert(TransactionIdIsValid(presult.new_relfrozenxid));
+	vacrel->NewRelminMxid = presult.new_relminmxid;
 
-	if (do_freeze)
+	if (presult.nfrozen > 0)
 	{
-		TransactionId snapshotConflictHorizon;
-
 		/*
-		 * We're freezing the page.  Our final NewRelfrozenXid doesn't need to
-		 * be affected by the XIDs that are just about to be frozen anyway.
+		 * We never increment the frozen_pages instrumentation counter when
+		 * nfrozen == 0, since it only counts pages with newly frozen tuples
+		 * (don't confuse that with pages newly set all-frozen in VM).
 		 */
-		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
-		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
-
 		vacrel->frozen_pages++;
 
-		/*
-		 * We can use frz_conflict_horizon 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.
-		 */
-		if (presult.all_visible_except_removable && presult.all_frozen)
-			snapshotConflictHorizon = presult.frz_conflict_horizon;
-		else
-		{
-			/* Avoids false conflicts when hot_standby_feedback in use */
-			snapshotConflictHorizon = pagefrz.cutoffs->OldestXmin;
-			TransactionIdRetreat(snapshotConflictHorizon);
-		}
-
 		/* Using same cutoff when setting VM is now unnecessary */
-		if (presult.all_visible_except_removable && presult.all_frozen)
+		if (presult.all_frozen)
 			presult.frz_conflict_horizon = InvalidTransactionId;
-
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(vacrel->rel, buf,
-									 snapshotConflictHorizon,
-									 presult.frozen, presult.nfrozen);
-	}
-	else if (presult.all_frozen && presult.nfrozen == 0)
-	{
-		/* Page should be all visible except to-be-removed tuples */
-		Assert(presult.all_visible_except_removable);
-
-		/*
-		 * We have no freeze plans to execute, so there's no added cost from
-		 * following the freeze path.  That's why it was chosen. This is
-		 * important in the case where the page only contains totally frozen
-		 * tuples at this point (perhaps only following pruning). Such pages
-		 * can be marked all-frozen in the VM by our caller, even though none
-		 * of its tuples were newly frozen here (note that the "no freeze"
-		 * path never sets pages all-frozen).
-		 *
-		 * We never increment the frozen_pages instrumentation counter here,
-		 * since it only counts pages with newly frozen tuples (don't confuse
-		 * that with pages newly set all-frozen in VM).
-		 */
-		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
-		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
-	}
-	else
-	{
-		/*
-		 * Page requires "no freeze" processing.  It might be set all-visible
-		 * in the visibility map, but it can never be set all-frozen.
-		 */
-		vacrel->NewRelfrozenXid = pagefrz.NoFreezePageRelfrozenXid;
-		vacrel->NewRelminMxid = pagefrz.NoFreezePageRelminMxid;
-		presult.all_frozen = false;
-		presult.nfrozen = 0;	/* avoid miscounts in instrumentation */
 	}
 
 	/*
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index b3cd248fb64..88a6d504dff 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -1715,9 +1715,9 @@ TransactionIdIsActive(TransactionId xid)
  * Note: the approximate horizons (see definition of GlobalVisState) are
  * updated by the computations done here. That's currently required for
  * correctness and a small optimization. Without doing so it's possible that
- * heap vacuum's call to heap_page_prune() uses a more conservative horizon
- * than later when deciding which tuples can be removed - which the code
- * doesn't expect (breaking HOT).
+ * heap vacuum's call to heap_page_prune_and_freeze() uses a more conservative
+ * horizon than later when deciding which tuples can be removed - which the
+ * code doesn't expect (breaking HOT).
  */
 static void
 ComputeXidHorizons(ComputeXidHorizonsResult *h)
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 2339abfd28a..b2a4caeb33a 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -195,7 +195,7 @@ typedef struct HeapPageFreeze
 /*
  * Per-page state returned from pruning
  */
-typedef struct PruneResult
+typedef struct PruneFreezeResult
 {
 	int			ndeleted;		/* Number of tuples deleted from the page */
 	int			nnewlpdead;		/* Number of newly LP_DEAD items */
@@ -210,9 +210,10 @@ typedef struct PruneResult
 
 	/*
 	 * Tuple visibility is only computed once for each tuple, for correctness
-	 * and efficiency reasons; see comment in heap_page_prune() for details.
-	 * This is of type int8[], instead of HTSV_Result[], so we can use -1 to
-	 * indicate no visibility has been computed, e.g. for LP_DEAD items.
+	 * and efficiency reasons; see comment in heap_page_prune_and_freeze() for
+	 * details. This is of type int8[], instead of HTSV_Result[], so we can
+	 * use -1 to indicate no visibility has been computed, e.g. for LP_DEAD
+	 * items.
 	 *
 	 * This needs to be MaxHeapTuplesPerPage + 1 long as FirstOffsetNumber is
 	 * 1. Otherwise every access would need to subtract 1.
@@ -220,17 +221,18 @@ typedef struct PruneResult
 	int8		htsv[MaxHeapTuplesPerPage + 1];
 
 
-	/*
-	 * One entry for every tuple that we may freeze.
-	 */
-	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
-} PruneResult;
+	/* New value of relfrozenxid found by heap_page_prune_and_freeze() */
+	TransactionId new_relfrozenxid;
+
+	/* New value of relminmxid found by heap_page_prune_and_freeze() */
+	MultiXactId new_relminmxid;
+} PruneFreezeResult;
 
 /*
  * Pruning calculates tuple visibility once and saves the results in an array
- * of int8. See PruneResult.htsv for details. This helper function is meant to
- * guard against examining visibility status array members which have not yet
- * been computed.
+ * of int8. See PruneFreezeResult.htsv for details. This helper function is
+ * meant to guard against examining visibility status array members which have
+ * not yet been computed.
  */
 static inline HTSV_Result
 htsv_get_valid_status(int status)
@@ -306,6 +308,7 @@ extern TM_Result heap_lock_tuple(Relation relation, HeapTuple tuple,
 								 Buffer *buffer, struct TM_FailureData *tmfd);
 
 extern void heap_inplace_update(Relation relation, HeapTuple tuple);
+
 extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 									  HeapPageFreeze *pagefrz,
 									  HeapTupleFreeze *frz, bool *totally_frozen);
@@ -332,12 +335,12 @@ 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(Relation relation, Buffer buffer,
-							struct GlobalVisState *vistest,
-							bool mark_unused_now,
-							HeapPageFreeze *pagefrz,
-							PruneResult *presult,
-							OffsetNumber *off_loc);
+extern void heap_page_prune_and_freeze(Relation relation, Buffer buffer,
+									   struct GlobalVisState *vistest,
+									   bool mark_unused_now,
+									   HeapPageFreeze *pagefrz,
+									   PruneFreezeResult *presult,
+									   OffsetNumber *off_loc);
 extern void heap_page_prune_execute(Buffer buffer,
 									OffsetNumber *redirected, int nredirected,
 									OffsetNumber *nowdead, int ndead,
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 042d04c8de2..b2ddc1e2549 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2179,7 +2179,7 @@ ProjectionPath
 PromptInterruptContext
 ProtocolVersion
 PrsStorage
-PruneResult
+PruneFreezeResult
 PruneState
 PruneStepResult
 PsqlScanCallbacks
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0010-Make-opp-freeze-heuristic-compatible-with-prune-f.patch"

From dd6bdad1253dc6c2e62cfe8144d62456ee7be2e8 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sun, 7 Jan 2024 16:11:35 -0500
Subject: [PATCH v4 10/19] Make opp freeze heuristic compatible with
 prune+freeze record

Once the prune and freeze records are combined, we will no longer be
able to use a test of whether or not pruning emitted an FPI to decide
whether or not to opportunistically freeze a freezable page.

While this heuristic should be improved, for now, approximate the
previous logic by keeping track of whether or not a hint bit FPI was
emitted during visibility checks (when checksums are on) and combine
that with checking XLogCheckBufferNeedsBackup(). If we just finished
deciding whether or not to prune and the current buffer seems to need an
FPI after modification, it is likely that pruning would have emitted an
FPI.
---
 src/backend/access/heap/pruneheap.c | 57 +++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 20907ba5408..9edf6bf72d7 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -246,6 +246,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	PruneState	prstate;
 	HeapTupleData tup;
 	bool		do_freeze;
+	bool		do_prune;
+	bool		whole_page_freezable;
+	bool		hint_bit_fpi;
+	bool		prune_fpi = false;
 	int64		fpi_before = pgWalUsage.wal_fpi;
 
 	/*
@@ -439,6 +443,13 @@ 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. Then reset fpi_before for no prune case.
+	 */
+	hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
+	fpi_before = pgWalUsage.wal_fpi;
+
 	/*
 	 * For vacuum, if the whole page will become frozen, we consider
 	 * opportunistically freezing tuples. Dead tuples which will be removed by
@@ -483,11 +494,41 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	if (off_loc)
 		*off_loc = InvalidOffsetNumber;
 
+	do_prune = prstate.nredirected > 0 ||
+		prstate.ndead > 0 ||
+		prstate.nunused > 0;
+
+	/*
+	 * Only incur overhead of checking if we will do an FPI if we might use
+	 * the information.
+	 */
+	if (do_prune && pagefrz)
+		prune_fpi = XLogCheckBufferNeedsBackup(buffer);
+
+	/* Is the whole page freezable? And is there something to freeze */
+	whole_page_freezable = presult->all_visible_except_removable &&
+		presult->all_frozen;
+
+	/*
+	 * Freeze the page when heap_prepare_freeze_tuple indicates that at least
+	 * one XID/MXID from before FreezeLimit/MultiXactCutoff is present.  Also
+	 * freeze when pruning generated an FPI, if doing so means that we set the
+	 * page all-frozen afterwards (might not happen until 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 are combined, this heuristic couldn't be used anymore. The
+	 * opportunistic freeze heuristic must be improved; however, for now, try
+	 * to approximate it.
+	 */
+	do_freeze = pagefrz &&
+		(pagefrz->freeze_required ||
+		 (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+
 	/* Any error while applying the changes is critical */
 	START_CRIT_SECTION();
 
 	/* Have we found any prunable items? */
-	if (prstate.nredirected > 0 || prstate.ndead > 0 || prstate.nunused > 0)
+	if (do_prune)
 	{
 		/*
 		 * Apply the planned item changes, then repair page fragmentation, and
@@ -579,20 +620,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	/* Record number of newly-set-LP_DEAD items for caller */
 	presult->nnewlpdead = prstate.ndead;
 
-	/*
-	 * Freeze the page when heap_prepare_freeze_tuple indicates that at least
-	 * one XID/MXID from before FreezeLimit/MultiXactCutoff is present.  Also
-	 * freeze when pruning generated an FPI, if doing so means that we set the
-	 * page all-frozen afterwards (might not happen until final heap pass).
-	 */
-	if (pagefrz)
-		do_freeze = pagefrz->freeze_required ||
-			(presult->all_visible_except_removable && presult->all_frozen &&
-			 presult->nfrozen > 0 &&
-			 fpi_before != pgWalUsage.wal_fpi);
-	else
-		do_freeze = false;
-
 	if (do_freeze)
 	{
 		/*
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0011-Separate-tuple-pre-freeze-checks-and-invoke-earli.patch"

From 2f9a262a6b85e3bfc25f2c066634ccb9958529e3 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sun, 7 Jan 2024 16:53:45 -0500
Subject: [PATCH v4 11/19] Separate tuple pre freeze checks and invoke earlier

When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.

Also move up the calculation of the freeze snapshot conflict horizon.
---
 src/backend/access/heap/heapam.c    | 58 ++++++++++++++++-------------
 src/backend/access/heap/pruneheap.c | 31 ++++++++-------
 src/include/access/heapam.h         |  3 ++
 3 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive.  They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+					   HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	/*
-	 * Perform xmin/xmax XID status sanity checks before critical section.
-	 *
-	 * heap_prepare_freeze_tuple doesn't perform these checks directly because
-	 * pg_xact lookups are relatively expensive.  They shouldn't be repeated
-	 * by successive VACUUMs that each decide against freezing the same page.
-	 */
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 										 xmax)));
 		}
 	}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us.  Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby).  We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+							 TransactionId snapshotConflictHorizon,
+							 HeapTupleFreeze *tuples, int ntuples)
+{
+	Page		page = BufferGetPage(buffer);
+
+	Assert(ntuples > 0);
 
 	START_CRIT_SECTION();
 
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 9edf6bf72d7..87f99497865 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -524,6 +524,24 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 		(pagefrz->freeze_required ||
 		 (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
 
+	if (do_freeze)
+	{
+		heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+
+		/*
+		 * We can use frz_conflict_horizon 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.
+		 */
+		if (!(presult->all_visible_except_removable && presult->all_frozen))
+		{
+			/* Avoids false conflicts when hot_standby_feedback in use */
+			presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+			TransactionIdRetreat(presult->frz_conflict_horizon);
+		}
+	}
+
 	/* Any error while applying the changes is critical */
 	START_CRIT_SECTION();
 
@@ -622,19 +640,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/*
-		 * We can use frz_conflict_horizon 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.
-		 */
-		if (!(presult->all_visible_except_removable && presult->all_frozen))
-		{
-			/* Avoids false conflicts when hot_standby_feedback in use */
-			presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
-			TransactionIdRetreat(presult->frz_conflict_horizon);
-		}
-
 		/* Execute all freeze plans for page as a single atomic action */
 		heap_freeze_execute_prepared(relation, buffer,
 									 presult->frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index b2a4caeb33a..02e33f213e1 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -312,6 +312,9 @@ extern void heap_inplace_update(Relation relation, HeapTuple tuple);
 extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 									  HeapPageFreeze *pagefrz,
 									  HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+								   HeapTupleFreeze *tuples, int ntuples);
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 										 TransactionId snapshotConflictHorizon,
 										 HeapTupleFreeze *tuples, int ntuples);
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0012-Remove-heap_freeze_execute_prepared.patch"

From 2a2e5407f8e6f8200dbda1b95cd3ec8d379282dd Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sun, 7 Jan 2024 17:03:17 -0500
Subject: [PATCH v4 12/19] Remove heap_freeze_execute_prepared()

In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune().
---
 src/backend/access/heap/heapam.c    | 79 +++++------------------------
 src/backend/access/heap/pruneheap.c | 51 +++++++++++++++++--
 src/include/access/heapam.h         | 31 ++++++-----
 3 files changed, 77 insertions(+), 84 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16e3f2520a4..e47b56e7856 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -91,9 +91,6 @@ static void compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
 static TM_Result heap_lock_updated_tuple(Relation rel, HeapTuple tuple,
 										 ItemPointer ctid, TransactionId xid,
 										 LockTupleMode mode);
-static int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
-								 xl_heap_freeze_plan *plans_out,
-								 OffsetNumber *offsets_out);
 static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 								   uint16 *new_infomask2);
 static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
@@ -6343,9 +6340,9 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
  * XIDs or MultiXactIds that will need to be processed by a future VACUUM.
  *
  * VACUUM caller must assemble HeapTupleFreeze freeze plan entries for every
- * tuple that we returned true for, and call heap_freeze_execute_prepared to
- * execute freezing.  Caller must initialize pagefrz fields for page as a
- * whole before first call here for each heap page.
+ * tuple that we returned true for, and then execute freezing.  Caller must
+ * initialize pagefrz fields for page as a whole before first call here for
+ * each heap page.
  *
  * VACUUM caller decides on whether or not to freeze the page as a whole.
  * We'll often prepare freeze plans for a page that caller just discards.
@@ -6659,8 +6656,8 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
 }
 
 /*
-* Perform xmin/xmax XID status sanity checks before calling
-* heap_freeze_execute_prepared().
+* Perform xmin/xmax XID status sanity checks before actually executing freeze
+* plans.
 *
 * heap_prepare_freeze_tuple doesn't perform these checks directly because
 * pg_xact lookups are relatively expensive.  They shouldn't be repeated
@@ -6713,30 +6710,17 @@ heap_pre_freeze_checks(Buffer buffer,
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
  */
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	START_CRIT_SECTION();
-
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6746,45 +6730,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		htup = (HeapTupleHeader) PageGetItem(page, itemid);
 		heap_execute_freeze_tuple(htup, frz);
 	}
-
-	MarkBufferDirty(buffer);
-
-	/* Now WAL-log freezing if necessary */
-	if (RelationNeedsWAL(rel))
-	{
-		xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
-		OffsetNumber offsets[MaxHeapTuplesPerPage];
-		int			nplans;
-		xl_heap_freeze_page xlrec;
-		XLogRecPtr	recptr;
-
-		/* Prepare deduplicated representation for use in WAL record */
-		nplans = heap_log_freeze_plan(tuples, ntuples, plans, offsets);
-
-		xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
-		xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(rel);
-		xlrec.nplans = nplans;
-
-		XLogBeginInsert();
-		XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
-
-		/*
-		 * The freeze plan array and offset array are not actually in the
-		 * buffer, but pretend that they are.  When XLogInsert stores the
-		 * whole buffer, the arrays need not be stored too.
-		 */
-		XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
-		XLogRegisterBufData(0, (char *) plans,
-							nplans * sizeof(xl_heap_freeze_plan));
-		XLogRegisterBufData(0, (char *) offsets,
-							ntuples * sizeof(OffsetNumber));
-
-		recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
-
-		PageSetLSN(page, recptr);
-	}
-
-	END_CRIT_SECTION();
 }
 
 /*
@@ -6874,7 +6819,7 @@ heap_log_freeze_new_plan(xl_heap_freeze_plan *plan, HeapTupleFreeze *frz)
  * (actually there is one array per freeze plan, but that's not of immediate
  * concern to our caller).
  */
-static int
+int
 heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
 					 xl_heap_freeze_plan *plans_out,
 					 OffsetNumber *offsets_out)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 87f99497865..7bd479cfd4e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -640,10 +640,53 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(relation, buffer,
-									 presult->frz_conflict_horizon,
-									 prstate.frozen, presult->nfrozen);
+		START_CRIT_SECTION();
+
+		Assert(presult->nfrozen > 0);
+
+		heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+
+		MarkBufferDirty(buffer);
+
+		/* Now WAL-log freezing if necessary */
+		if (RelationNeedsWAL(relation))
+		{
+			xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+			OffsetNumber offsets[MaxHeapTuplesPerPage];
+			int			nplans;
+			xl_heap_freeze_page xlrec;
+			XLogRecPtr	recptr;
+
+			/*
+			 * Prepare deduplicated representation for use in WAL record
+			 * Destructively sorts tuples array in-place.
+			 */
+			nplans = heap_log_freeze_plan(prstate.frozen, presult->nfrozen, plans, offsets);
+
+			xlrec.snapshotConflictHorizon = presult->frz_conflict_horizon;
+			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+			xlrec.nplans = nplans;
+
+			XLogBeginInsert();
+			XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+
+			/*
+			 * The freeze plan array and offset array are not actually in the
+			 * buffer, but pretend that they are.  When XLogInsert stores the
+			 * whole buffer, the arrays need not be stored too.
+			 */
+			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+			XLogRegisterBufData(0, (char *) plans,
+								nplans * sizeof(xl_heap_freeze_plan));
+			XLogRegisterBufData(0, (char *) offsets,
+								presult->nfrozen * sizeof(OffsetNumber));
+
+			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+
+			PageSetLSN(page, recptr);
+		}
+
+		END_CRIT_SECTION();
 	}
 	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
 	{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 02e33f213e1..321a46185e1 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
 #ifndef HEAPAM_H
 #define HEAPAM_H
 
+#include "access/heapam_xlog.h"
 #include "access/relation.h"	/* for backward compatibility */
 #include "access/relscan.h"
 #include "access/sdir.h"
@@ -101,8 +102,8 @@ typedef enum
 } HTSV_Result;
 
 /*
- * heap_prepare_freeze_tuple may request that heap_freeze_execute_prepared
- * check any tuple's to-be-frozen xmin and/or xmax status using pg_xact
+ * heap_prepare_freeze_tuple may request that any tuple's to-be-frozen xmin
+ * and/or xmax status is checked using pg_xact during freezing execution.
  */
 #define		HEAP_FREEZE_CHECK_XMIN_COMMITTED	0x01
 #define		HEAP_FREEZE_CHECK_XMAX_ABORTED		0x02
@@ -154,14 +155,14 @@ typedef struct HeapPageFreeze
 	/*
 	 * "Freeze" NewRelfrozenXid/NewRelminMxid trackers.
 	 *
-	 * Trackers used when heap_freeze_execute_prepared freezes, or when there
-	 * are zero freeze plans for a page.  It is always valid for vacuumlazy.c
-	 * to freeze any page, by definition.  This even includes pages that have
-	 * no tuples with storage to consider in the first place.  That way the
-	 * 'totally_frozen' results from heap_prepare_freeze_tuple can always be
-	 * used in the same way, even when no freeze plans need to be executed to
-	 * "freeze the page".  Only the "freeze" path needs to consider the need
-	 * to set pages all-frozen in the visibility map under this scheme.
+	 * Trackers used when tuples will be frozen, or when there are zero freeze
+	 * plans for a page.  It is always valid for vacuumlazy.c to freeze any
+	 * page, by definition.  This even includes pages that have no tuples with
+	 * storage to consider in the first place.  That way the 'totally_frozen'
+	 * results from heap_prepare_freeze_tuple can always be used in the same
+	 * way, even when no freeze plans need to be executed to "freeze the
+	 * page". Only the "freeze" path needs to consider the need to set pages
+	 * all-frozen in the visibility map under this scheme.
 	 *
 	 * When we freeze a page, we generally freeze all XIDs < OldestXmin, only
 	 * leaving behind XIDs that are ineligible for freezing, if any.  And so
@@ -315,12 +316,16 @@ extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 
 extern void heap_pre_freeze_checks(Buffer buffer,
 								   HeapTupleFreeze *tuples, int ntuples);
-extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-										 TransactionId snapshotConflictHorizon,
-										 HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+										HeapTupleFreeze *tuples, int ntuples);
 extern bool heap_freeze_tuple(HeapTupleHeader tuple,
 							  TransactionId relfrozenxid, TransactionId relminmxid,
 							  TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
+extern int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
+								 xl_heap_freeze_plan *plans_out,
+								 OffsetNumber *offsets_out);
 extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
 									 const struct VacuumCutoffs *cutoffs,
 									 TransactionId *NoFreezePageRelfrozenXid,
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0013-Merge-prune-and-freeze-records.patch"

From b13f4d1d5fb4e8fcb3f97fe1f0043fdfaf319b4c Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sun, 7 Jan 2024 17:55:31 -0500
Subject: [PATCH v4 13/19] Merge prune and freeze records

Eliminate xl_heap_freeze and XLOG_HEAP2_FREEZE record. When vacuum
freezes tuples, the information needed to replay those changes is
now recorded in the xl_heap_prune record.

When both pruning and freezing is done, this means a single, combined
WAL record is emitted for both operations. This will reduce the number
of WAL records emitted.

When there are only tuples to freeze present, we can avoid taking a full
cleanup lock when replaying the record.

The XLOG_HEAP2_PRUNE record is now bigger than it was previously and
bigger than the XLOG_HEAP2_FREEZE record. A future commit will
streamline the record.
---
 src/backend/access/heap/heapam.c         | 146 ++++------
 src/backend/access/heap/pruneheap.c      | 326 ++++++++++++-----------
 src/backend/access/rmgrdesc/heapdesc.c   |  95 ++++---
 src/backend/replication/logical/decode.c |   1 -
 src/include/access/heapam_xlog.h         |  97 ++++---
 5 files changed, 318 insertions(+), 347 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e47b56e7856..532868039d5 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8706,8 +8706,6 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required,
 
 /*
  * Handles XLOG_HEAP2_PRUNE record type.
- *
- * Acquires a full cleanup lock.
  */
 static void
 heap_xlog_prune(XLogReaderState *record)
@@ -8718,12 +8716,22 @@ heap_xlog_prune(XLogReaderState *record)
 	RelFileLocator rlocator;
 	BlockNumber blkno;
 	XLogRedoAction action;
+	bool		get_cleanup_lock;
 
 	XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno);
 
 	/*
-	 * We're about to remove tuples. In Hot Standby mode, ensure that there's
-	 * no queries running for which the removed tuples are still visible.
+	 * If there are dead, redirected, or unused items set unused by
+	 * heap_page_prune_and_freeze(), heap_page_prune_execute() will call
+	 * PageRepairFragementation() which expects a full cleanup lock.
+	 */
+	get_cleanup_lock = xlrec->nredirected > 0 ||
+		xlrec->ndead > 0 || xlrec->nunused > 0;
+
+	/*
+	 * We are either about to remove tuples or freeze them. In Hot Standby
+	 * mode, ensure that there's no queries running for which any removed
+	 * tuples are still visible or which consider the frozen xids as running.
 	 */
 	if (InHotStandby)
 		ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
@@ -8731,38 +8739,69 @@ heap_xlog_prune(XLogReaderState *record)
 											rlocator);
 
 	/*
-	 * If we have a full-page image, restore it (using a cleanup lock) and
-	 * we're done.
+	 * If we have a full-page image, restore it and we're done.
 	 */
-	action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL, true,
-										   &buffer);
+	action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL,
+										   get_cleanup_lock, &buffer);
+
 	if (action == BLK_NEEDS_REDO)
 	{
 		Page		page = (Page) BufferGetPage(buffer);
-		OffsetNumber *end;
 		OffsetNumber *redirected;
 		OffsetNumber *nowdead;
 		OffsetNumber *nowunused;
 		int			nredirected;
 		int			ndead;
 		int			nunused;
+		int			nplans;
 		Size		datalen;
+		xl_heap_freeze_plan *plans;
+		OffsetNumber *frz_offsets;
+		int			curoff = 0;
 
-		redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+		nplans = xlrec->nplans;
 		nredirected = xlrec->nredirected;
 		ndead = xlrec->ndead;
-		end = (OffsetNumber *) ((char *) redirected + datalen);
+		nunused = xlrec->nunused;
+
+		plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+		redirected = (OffsetNumber *) &plans[nplans];
 		nowdead = redirected + (nredirected * 2);
 		nowunused = nowdead + ndead;
-		nunused = (end - nowunused);
-		Assert(nunused >= 0);
+		frz_offsets = nowunused + nunused;
 
 		/* Update all line pointers per the record, and repair fragmentation */
-		heap_page_prune_execute(buffer,
-								redirected, nredirected,
-								nowdead, ndead,
-								nowunused, nunused);
+		if (nredirected > 0 || ndead > 0 || nunused > 0)
+			heap_page_prune_execute(buffer,
+									redirected, nredirected,
+									nowdead, ndead,
+									nowunused, nunused);
+
+		for (int p = 0; p < nplans; p++)
+		{
+			HeapTupleFreeze frz;
+
+			/*
+			 * Convert freeze plan representation from WAL record into
+			 * per-tuple format used by heap_execute_freeze_tuple
+			 */
+			frz.xmax = plans[p].xmax;
+			frz.t_infomask2 = plans[p].t_infomask2;
+			frz.t_infomask = plans[p].t_infomask;
+			frz.frzflags = plans[p].frzflags;
+			frz.offset = InvalidOffsetNumber;	/* unused, but be tidy */
+
+			for (int i = 0; i < plans[p].ntuples; i++)
+			{
+				OffsetNumber offset = frz_offsets[curoff++];
+				ItemId		lp;
+				HeapTupleHeader tuple;
+
+				lp = PageGetItemId(page, offset);
+				tuple = (HeapTupleHeader) PageGetItem(page, lp);
+				heap_execute_freeze_tuple(tuple, &frz);
+			}
+		}
 
 		/*
 		 * Note: we don't worry about updating the page's prunability hints.
@@ -9001,74 +9040,6 @@ heap_xlog_visible(XLogReaderState *record)
 		UnlockReleaseBuffer(vmbuffer);
 }
 
-/*
- * Replay XLOG_HEAP2_FREEZE_PAGE records
- */
-static void
-heap_xlog_freeze_page(XLogReaderState *record)
-{
-	XLogRecPtr	lsn = record->EndRecPtr;
-	xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) XLogRecGetData(record);
-	Buffer		buffer;
-
-	/*
-	 * In Hot Standby mode, ensure that there's no queries running which still
-	 * consider the frozen xids as running.
-	 */
-	if (InHotStandby)
-	{
-		RelFileLocator rlocator;
-
-		XLogRecGetBlockTag(record, 0, &rlocator, NULL, NULL);
-		ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
-											xlrec->isCatalogRel,
-											rlocator);
-	}
-
-	if (XLogReadBufferForRedo(record, 0, &buffer) == BLK_NEEDS_REDO)
-	{
-		Page		page = BufferGetPage(buffer);
-		xl_heap_freeze_plan *plans;
-		OffsetNumber *offsets;
-		int			curoff = 0;
-
-		plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
-		offsets = (OffsetNumber *) ((char *) plans +
-									(xlrec->nplans *
-									 sizeof(xl_heap_freeze_plan)));
-		for (int p = 0; p < xlrec->nplans; p++)
-		{
-			HeapTupleFreeze frz;
-
-			/*
-			 * Convert freeze plan representation from WAL record into
-			 * per-tuple format used by heap_execute_freeze_tuple
-			 */
-			frz.xmax = plans[p].xmax;
-			frz.t_infomask2 = plans[p].t_infomask2;
-			frz.t_infomask = plans[p].t_infomask;
-			frz.frzflags = plans[p].frzflags;
-			frz.offset = InvalidOffsetNumber;	/* unused, but be tidy */
-
-			for (int i = 0; i < plans[p].ntuples; i++)
-			{
-				OffsetNumber offset = offsets[curoff++];
-				ItemId		lp;
-				HeapTupleHeader tuple;
-
-				lp = PageGetItemId(page, offset);
-				tuple = (HeapTupleHeader) PageGetItem(page, lp);
-				heap_execute_freeze_tuple(tuple, &frz);
-			}
-		}
-
-		PageSetLSN(page, lsn);
-		MarkBufferDirty(buffer);
-	}
-	if (BufferIsValid(buffer))
-		UnlockReleaseBuffer(buffer);
-}
-
 /*
  * 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.
@@ -9975,9 +9946,6 @@ heap2_redo(XLogReaderState *record)
 		case XLOG_HEAP2_VACUUM:
 			heap_xlog_vacuum(record);
 			break;
-		case XLOG_HEAP2_FREEZE_PAGE:
-			heap_xlog_freeze_page(record);
-			break;
 		case XLOG_HEAP2_VISIBLE:
 			heap_xlog_visible(record);
 			break;
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7bd479cfd4e..19b50931b90 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -79,6 +79,9 @@ static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber o
 static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
 static void page_verify_redirects(Page page);
 
+static void log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+									  PruneState *prstate, PruneFreezeResult *presult);
+
 
 /*
  * Optionally prune and repair fragmentation in the specified page.
@@ -247,9 +250,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	HeapTupleData tup;
 	bool		do_freeze;
 	bool		do_prune;
+	bool		do_hint;
 	bool		whole_page_freezable;
 	bool		hint_bit_fpi;
-	bool		prune_fpi = false;
 	int64		fpi_before = pgWalUsage.wal_fpi;
 
 	/*
@@ -445,10 +448,9 @@ 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. Then reset fpi_before for no prune case.
+	 * an FPI to be emitted.
 	 */
 	hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
-	fpi_before = pgWalUsage.wal_fpi;
 
 	/*
 	 * For vacuum, if the whole page will become frozen, we consider
@@ -498,14 +500,18 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 		prstate.ndead > 0 ||
 		prstate.nunused > 0;
 
+	/* Record number of newly-set-LP_DEAD items for caller */
+	presult->nnewlpdead = prstate.ndead;
+
 	/*
-	 * Only incur overhead of checking if we will do an FPI if we might use
-	 * the information.
+	 * 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.
 	 */
-	if (do_prune && pagefrz)
-		prune_fpi = XLogCheckBufferNeedsBackup(buffer);
+	do_hint = ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
+		PageIsFull(page);
 
-	/* Is the whole page freezable? And is there something to freeze */
+	/* Is the whole page freezable? And is there something to freeze? */
 	whole_page_freezable = presult->all_visible_except_removable &&
 		presult->all_frozen;
 
@@ -520,43 +526,51 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	 * opportunistic freeze heuristic must be improved; however, for now, try
 	 * to approximate it.
 	 */
-	do_freeze = pagefrz &&
-		(pagefrz->freeze_required ||
-		 (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+	do_freeze = false;
+	if (pagefrz)
+	{
+		if (pagefrz->freeze_required)
+			do_freeze = true;
+		else if (whole_page_freezable && presult->nfrozen > 0)
+		{
+			/*
+			 * Freezing would make the page all-frozen. In this case, we will
+			 * freeze if we have already emitted an FPI or will do so anyway.
+			 * Be sure only to incur the overhead of checking if we will do an
+			 * FPI if we may use that information.
+			 */
+			if (hint_bit_fpi ||
+				((do_prune || do_hint) && XLogCheckBufferNeedsBackup(buffer)))
+			{
+				do_freeze = true;
+			}
+		}
+	}
 
+	/*
+	 * Validate the tuples we are considering freezing. We do this even if
+	 * pruning and hint bit setting have not emitted an FPI so far because we
+	 * still may emit an FPI while setting the page hint bit later. But we
+	 * want to avoid doing the pre-freeze checks in a critical section.
+	 */
 	if (do_freeze)
-	{
 		heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
 
+	if (!do_freeze && (!pagefrz || !presult->all_frozen || presult->nfrozen > 0))
+	{
 		/*
-		 * We can use frz_conflict_horizon 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.
+		 * If we will neither freeze tuples on the page nor set the page all
+		 * frozen in the visibility map, the page is not all-frozen and there
+		 * will be no newly frozen tuples.
 		 */
-		if (!(presult->all_visible_except_removable && presult->all_frozen))
-		{
-			/* Avoids false conflicts when hot_standby_feedback in use */
-			presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
-			TransactionIdRetreat(presult->frz_conflict_horizon);
-		}
+		presult->all_frozen = false;
+		presult->nfrozen = 0;	/* avoid miscounts in instrumentation */
 	}
 
-	/* Any error while applying the changes is critical */
 	START_CRIT_SECTION();
 
-	/* Have we found any prunable items? */
-	if (do_prune)
+	if (do_hint)
 	{
-		/*
-		 * Apply the planned item changes, then repair page fragmentation, and
-		 * update the page's hint bit about whether it has free line pointers.
-		 */
-		heap_page_prune_execute(buffer,
-								prstate.redirected, prstate.nredirected,
-								prstate.nowdead, prstate.ndead,
-								prstate.nowunused, prstate.nunused);
-
 		/*
 		 * Update the page's pd_prune_xid field to either zero, or the lowest
 		 * XID of any soon-prunable tuple.
@@ -564,163 +578,159 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 		((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
 
 		/*
-		 * Also clear the "page is full" flag, since there's no point in
-		 * repeating the prune/defrag process until something else happens to
-		 * the page.
+		 * Clear the "page is full" flag if it is set since there's no point
+		 * in repeating the prune/defrag process until something else happens
+		 * to the page.
 		 */
 		PageClearFull(page);
 
-		MarkBufferDirty(buffer);
+		/*
+		 * We only needed to update pd_prune_xid and clear the page-is-full
+		 * hint bit, this is a non-WAL-logged hint. If we will also freeze or
+		 * prune the page, we will mark the buffer dirty below.
+		 */
+		if (!do_freeze && !do_prune)
+			MarkBufferDirtyHint(buffer, true);
+	}
 
+	if (do_prune || do_freeze)
+	{
 		/*
-		 * Emit a WAL XLOG_HEAP2_PRUNE record showing what we did
+		 * Apply the planned item changes, then repair page fragmentation, and
+		 * update the page's hint bit about whether it has free line pointers.
 		 */
-		if (RelationNeedsWAL(relation))
+		if (do_prune)
 		{
-			xl_heap_prune xlrec;
-			XLogRecPtr	recptr;
-
-			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
-			xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
-			xlrec.nredirected = prstate.nredirected;
-			xlrec.ndead = prstate.ndead;
-
-			XLogBeginInsert();
-			XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
-
-			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+			heap_page_prune_execute(buffer,
+									prstate.redirected, prstate.nredirected,
+									prstate.nowdead, prstate.ndead,
+									prstate.nowunused, prstate.nunused);
+		}
 
+		if (do_freeze)
+		{
 			/*
-			 * The OffsetNumber arrays are not actually in the buffer, but we
-			 * pretend that they are.  When XLogInsert stores the whole
-			 * buffer, the offset arrays need not be stored too.
+			 * We can use frz_conflict_horizon 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. This avoids false
+			 * conflicts when hot_standby_feedback is in use.
 			 */
-			if (prstate.nredirected > 0)
-				XLogRegisterBufData(0, (char *) prstate.redirected,
-									prstate.nredirected *
-									sizeof(OffsetNumber) * 2);
-
-			if (prstate.ndead > 0)
-				XLogRegisterBufData(0, (char *) prstate.nowdead,
-									prstate.ndead * sizeof(OffsetNumber));
-
-			if (prstate.nunused > 0)
-				XLogRegisterBufData(0, (char *) prstate.nowunused,
-									prstate.nunused * sizeof(OffsetNumber));
+			if (!(presult->all_visible_except_removable && presult->all_frozen))
+			{
+				presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+				TransactionIdRetreat(presult->frz_conflict_horizon);
+			}
+			heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+		}
 
-			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
+		MarkBufferDirty(buffer);
 
-			PageSetLSN(BufferGetPage(buffer), recptr);
-		}
-	}
-	else
-	{
 		/*
-		 * If we didn't prune anything, but have found a new value for the
-		 * pd_prune_xid field, update it and mark the buffer dirty. This is
-		 * treated as a non-WAL-logged hint.
-		 *
-		 * Also clear the "page is full" flag if it is set, since there's no
-		 * point in repeating the prune/defrag process until something else
-		 * happens to the page.
+		 * Emit a WAL XLOG_HEAP2_PRUNE record showing what we did
 		 */
-		if (((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
-			PageIsFull(page))
-		{
-			((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
-			PageClearFull(page);
-			MarkBufferDirtyHint(buffer, true);
-		}
+		if (RelationNeedsWAL(relation))
+			log_heap_prune_and_freeze(relation, buffer, &prstate, presult);
 	}
 
 	END_CRIT_SECTION();
 
-	/* Record number of newly-set-LP_DEAD items for caller */
-	presult->nnewlpdead = prstate.ndead;
-
-	if (do_freeze)
+	/*
+	 * If we froze tuples on the page, the caller can advance relfrozenxid and
+	 * relminmxid to the values in pagefrz->FreezePageRelfrozenXid and
+	 * pagefrz->FreezePageRelminMxid. Otherwise, it is only safe to advance to
+	 * the values in pagefrz->NoFreezePage[RelfrozenXid|RelminMxid]
+	 */
+	if (pagefrz)
 	{
-		START_CRIT_SECTION();
+		if (presult->nfrozen > 0)
+		{
+			presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
+			presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
+		}
+		else
+		{
+			presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
+			presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
+		}
+	}
+}
 
-		Assert(presult->nfrozen > 0);
 
-		heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+static void
+log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+						  PruneState *prstate, PruneFreezeResult *presult)
+{
+	xl_heap_prune xlrec;
+	XLogRecPtr	recptr;
 
-		MarkBufferDirty(buffer);
+	xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+	OffsetNumber offsets[MaxHeapTuplesPerPage];
+	bool		do_freeze = presult->nfrozen > 0;
 
-		/* Now WAL-log freezing if necessary */
-		if (RelationNeedsWAL(relation))
-		{
-			xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
-			OffsetNumber offsets[MaxHeapTuplesPerPage];
-			int			nplans;
-			xl_heap_freeze_page xlrec;
-			XLogRecPtr	recptr;
+	xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+	xlrec.nredirected = prstate->nredirected;
+	xlrec.ndead = prstate->ndead;
+	xlrec.nunused = prstate->nunused;
+	xlrec.nplans = 0;
 
-			/*
-			 * Prepare deduplicated representation for use in WAL record
-			 * Destructively sorts tuples array in-place.
-			 */
-			nplans = heap_log_freeze_plan(prstate.frozen, presult->nfrozen, plans, offsets);
+	/*
+	 * The snapshotConflictHorizon for the whole record should be the most
+	 * conservative of all the horizons calculated for any of the possible
+	 * modifications. If this record will prune tuples, any transactions on
+	 * the standby older than the youngest xmax of the most recently removed
+	 * tuple this record will prune will conflict. If this record will freeze
+	 * tuples, any transactions on the standby with xids older than the
+	 * youngest tuple this record will freeze will conflict.
+	 */
+	if (do_freeze)
+		xlrec.snapshotConflictHorizon = Max(prstate->snapshotConflictHorizon,
+											presult->frz_conflict_horizon);
+	else
+		xlrec.snapshotConflictHorizon = prstate->snapshotConflictHorizon;
 
-			xlrec.snapshotConflictHorizon = presult->frz_conflict_horizon;
-			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
-			xlrec.nplans = nplans;
+	/*
+	 * Prepare deduplicated representation for use in WAL record Destructively
+	 * sorts tuples array in-place.
+	 */
+	if (do_freeze)
+		xlrec.nplans = heap_log_freeze_plan(prstate->frozen,
+											presult->nfrozen, plans, offsets);
 
-			XLogBeginInsert();
-			XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+	XLogBeginInsert();
+	XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
 
-			/*
-			 * The freeze plan array and offset array are not actually in the
-			 * buffer, but pretend that they are.  When XLogInsert stores the
-			 * whole buffer, the arrays need not be stored too.
-			 */
-			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
-			XLogRegisterBufData(0, (char *) plans,
-								nplans * sizeof(xl_heap_freeze_plan));
-			XLogRegisterBufData(0, (char *) offsets,
-								presult->nfrozen * sizeof(OffsetNumber));
+	XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
 
-			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+	/*
+	 * The OffsetNumber arrays are not actually in the buffer, but we pretend
+	 * that they are.  When XLogInsert stores the whole buffer, the offset
+	 * arrays need not be stored too.
+	 */
+	if (xlrec.nplans > 0)
+		XLogRegisterBufData(0, (char *) plans,
+							xlrec.nplans * sizeof(xl_heap_freeze_plan));
 
-			PageSetLSN(page, recptr);
-		}
+	if (prstate->nredirected > 0)
+		XLogRegisterBufData(0, (char *) prstate->redirected,
+							prstate->nredirected *
+							sizeof(OffsetNumber) * 2);
 
-		END_CRIT_SECTION();
-	}
-	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
-	{
-		/*
-		 * If we will neither freeze tuples on the page nor set the page all
-		 * frozen in the visibility map, the page is not all frozen and there
-		 * will be no newly frozen tuples.
-		 */
-		presult->all_frozen = false;
-		presult->nfrozen = 0;	/* avoid miscounts in instrumentation */
-	}
+	if (prstate->ndead > 0)
+		XLogRegisterBufData(0, (char *) prstate->nowdead,
+							prstate->ndead * sizeof(OffsetNumber));
 
-	/* Caller won't update new_relfrozenxid and new_relminmxid */
-	if (!pagefrz)
-		return;
+	if (prstate->nunused > 0)
+		XLogRegisterBufData(0, (char *) prstate->nowunused,
+							prstate->nunused * sizeof(OffsetNumber));
+	if (xlrec.nplans > 0)
+		XLogRegisterBufData(0, (char *) offsets,
+							presult->nfrozen * sizeof(OffsetNumber));
 
-	/*
-	 * If we will freeze tuples on the page or, even if we don't freeze tuples
-	 * on the page, if we will set the page all-frozen in the visibility map,
-	 * we can advance relfrozenxid and relminmxid to the values in
-	 * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid.
-	 */
-	if (presult->all_frozen || presult->nfrozen > 0)
-	{
-		presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
-		presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
-	}
-	else
-	{
-		presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
-		presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
-	}
-}
+	recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
 
+	PageSetLSN(BufferGetPage(buffer), recptr);
+}
 
 /*
  * Perform visibility checks for heap pruning.
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 36a3d83c8c2..9f0a0341d40 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -179,43 +179,67 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
 	{
 		xl_heap_prune *xlrec = (xl_heap_prune *) rec;
 
-		appendStringInfo(buf, "snapshotConflictHorizon: %u, nredirected: %u, ndead: %u, isCatalogRel: %c",
+		appendStringInfo(buf, "snapshotConflictHorizon: %u, isCatalogRel: %c",
 						 xlrec->snapshotConflictHorizon,
-						 xlrec->nredirected,
-						 xlrec->ndead,
 						 xlrec->isCatalogRel ? 'T' : 'F');
 
 		if (XLogRecHasBlockData(record, 0))
 		{
-			OffsetNumber *end;
 			OffsetNumber *redirected;
 			OffsetNumber *nowdead;
 			OffsetNumber *nowunused;
 			int			nredirected;
+			int			ndead;
 			int			nunused;
+			int			nplans;
 			Size		datalen;
+			xl_heap_freeze_plan *plans;
+			OffsetNumber *frz_offsets;
 
-			redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0,
-															  &datalen);
-
+			nplans = xlrec->nplans;
 			nredirected = xlrec->nredirected;
-			end = (OffsetNumber *) ((char *) redirected + datalen);
-			nowdead = redirected + (nredirected * 2);
-			nowunused = nowdead + xlrec->ndead;
-			nunused = (end - nowunused);
-			Assert(nunused >= 0);
+			ndead = xlrec->ndead;
+			nunused = xlrec->nunused;
 
-			appendStringInfo(buf, ", nunused: %d", nunused);
-
-			appendStringInfoString(buf, ", redirected:");
-			array_desc(buf, redirected, sizeof(OffsetNumber) * 2,
-					   nredirected, &redirect_elem_desc, NULL);
-			appendStringInfoString(buf, ", dead:");
-			array_desc(buf, nowdead, sizeof(OffsetNumber), xlrec->ndead,
-					   &offset_elem_desc, NULL);
-			appendStringInfoString(buf, ", unused:");
-			array_desc(buf, nowunused, sizeof(OffsetNumber), nunused,
-					   &offset_elem_desc, NULL);
+			plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+			redirected = (OffsetNumber *) &plans[nplans];
+			nowdead = redirected + (nredirected * 2);
+			nowunused = nowdead + ndead;
+			frz_offsets = nowunused + nunused;
+
+			appendStringInfo(buf, ", nredirected: %u, ndead: %u, nunused: %u, nplans: %u,",
+							 nredirected,
+							 ndead,
+							 nunused,
+							 nplans);
+
+			if (nredirected > 0)
+			{
+				appendStringInfoString(buf, ", redirected:");
+				array_desc(buf, redirected, sizeof(OffsetNumber) * 2,
+						   nredirected, &redirect_elem_desc, NULL);
+			}
+
+			if (ndead > 0)
+			{
+				appendStringInfoString(buf, ", dead:");
+				array_desc(buf, nowdead, sizeof(OffsetNumber), ndead,
+						   &offset_elem_desc, NULL);
+			}
+
+			if (nunused > 0)
+			{
+				appendStringInfoString(buf, ", unused:");
+				array_desc(buf, nowunused, sizeof(OffsetNumber), nunused,
+						   &offset_elem_desc, NULL);
+			}
+
+			if (nplans > 0)
+			{
+				appendStringInfoString(buf, ", plans:");
+				array_desc(buf, plans, sizeof(xl_heap_freeze_plan), nplans,
+						   &plan_elem_desc, &frz_offsets);
+			}
 		}
 	}
 	else if (info == XLOG_HEAP2_VACUUM)
@@ -235,28 +259,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
 					   &offset_elem_desc, NULL);
 		}
 	}
-	else if (info == XLOG_HEAP2_FREEZE_PAGE)
-	{
-		xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) rec;
-
-		appendStringInfo(buf, "snapshotConflictHorizon: %u, nplans: %u, isCatalogRel: %c",
-						 xlrec->snapshotConflictHorizon, xlrec->nplans,
-						 xlrec->isCatalogRel ? 'T' : 'F');
-
-		if (XLogRecHasBlockData(record, 0))
-		{
-			xl_heap_freeze_plan *plans;
-			OffsetNumber *offsets;
-
-			plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
-			offsets = (OffsetNumber *) ((char *) plans +
-										(xlrec->nplans *
-										 sizeof(xl_heap_freeze_plan)));
-			appendStringInfoString(buf, ", plans:");
-			array_desc(buf, plans, sizeof(xl_heap_freeze_plan), xlrec->nplans,
-					   &plan_elem_desc, &offsets);
-		}
-	}
 	else if (info == XLOG_HEAP2_VISIBLE)
 	{
 		xl_heap_visible *xlrec = (xl_heap_visible *) rec;
@@ -361,9 +363,6 @@ heap2_identify(uint8 info)
 		case XLOG_HEAP2_VACUUM:
 			id = "VACUUM";
 			break;
-		case XLOG_HEAP2_FREEZE_PAGE:
-			id = "FREEZE_PAGE";
-			break;
 		case XLOG_HEAP2_VISIBLE:
 			id = "VISIBLE";
 			break;
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index e5ab7b78b78..f77051572fd 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -445,7 +445,6 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 			 * Everything else here is just low level physical stuff we're not
 			 * interested in.
 			 */
-		case XLOG_HEAP2_FREEZE_PAGE:
 		case XLOG_HEAP2_PRUNE:
 		case XLOG_HEAP2_VACUUM:
 		case XLOG_HEAP2_VISIBLE:
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..fe4a8ff0620 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -53,11 +53,10 @@
 #define XLOG_HEAP2_REWRITE		0x00
 #define XLOG_HEAP2_PRUNE		0x10
 #define XLOG_HEAP2_VACUUM		0x20
-#define XLOG_HEAP2_FREEZE_PAGE	0x30
-#define XLOG_HEAP2_VISIBLE		0x40
-#define XLOG_HEAP2_MULTI_INSERT 0x50
-#define XLOG_HEAP2_LOCK_UPDATED 0x60
-#define XLOG_HEAP2_NEW_CID		0x70
+#define XLOG_HEAP2_VISIBLE		0x30
+#define XLOG_HEAP2_MULTI_INSERT 0x40
+#define XLOG_HEAP2_LOCK_UPDATED 0x50
+#define XLOG_HEAP2_NEW_CID		0x60
 
 /*
  * xl_heap_insert/xl_heap_multi_insert flag values, 8 bits are available.
@@ -226,28 +225,65 @@ typedef struct xl_heap_update
 
 #define SizeOfHeapUpdate	(offsetof(xl_heap_update, new_offnum) + sizeof(OffsetNumber))
 
+/*
+ * This struct represents a 'freeze plan', which describes how to freeze a
+ * group of one or more heap tuples (appears in xl_heap_prune record)
+ */
+/* 0x01 was XLH_FREEZE_XMIN */
+#define		XLH_FREEZE_XVAC		0x02
+#define		XLH_INVALID_XVAC	0x04
+
+typedef struct xl_heap_freeze_plan
+{
+	TransactionId xmax;
+	uint16		t_infomask2;
+	uint16		t_infomask;
+	uint8		frzflags;
+
+	/* Length of individual page offset numbers array for this plan */
+	uint16		ntuples;
+} xl_heap_freeze_plan;
+
+/*
+ * As of Postgres 17, XLOG_HEAP2_PRUNE records replace
+ * XLOG_HEAP2_FREEZE_PAGE records.
+ */
+
 /*
  * This is what we need to know about page pruning (both during VACUUM and
  * during opportunistic pruning)
  *
  * The array of OffsetNumbers following the fixed part of the record contains:
+ *	* for each freeze plan: the freeze plan
  *	* for each redirected item: the item offset, then the offset redirected to
  *	* for each now-dead item: the item offset
  *	* for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ *	* for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
  *
- * Acquires a full cleanup lock.
+ * Acquires a full cleanup lock if heap_page_prune_execute() must be called
  */
 typedef struct xl_heap_prune
 {
 	TransactionId snapshotConflictHorizon;
+	uint16		nplans;
 	uint16		nredirected;
 	uint16		ndead;
+	uint16		nunused;
 	bool		isCatalogRel;	/* to handle recovery conflict during logical
 								 * decoding on standby */
-	/* OFFSET NUMBERS are in the block reference 0 */
+	/*--------------------------------------------------------------------
+	 * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+	 * following order:
+	 *
+	 *		* xl_heap_freeze_plan plans[nplans];
+	 * 		* OffsetNumber redirected[2 * nredirected];
+	 * 		* OffsetNumber nowdead[ndead];
+	 *		* OffsetNumber nowunused[nunused];
+	 * 		* OffsetNumber frz_offsets[...];
+	 *--------------------------------------------------------------------
+	 */
 } xl_heap_prune;
 
 #define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
@@ -315,47 +351,6 @@ typedef struct xl_heap_inplace
 
 #define SizeOfHeapInplace	(offsetof(xl_heap_inplace, offnum) + sizeof(OffsetNumber))
 
-/*
- * This struct represents a 'freeze plan', which describes how to freeze a
- * group of one or more heap tuples (appears in xl_heap_freeze_page record)
- */
-/* 0x01 was XLH_FREEZE_XMIN */
-#define		XLH_FREEZE_XVAC		0x02
-#define		XLH_INVALID_XVAC	0x04
-
-typedef struct xl_heap_freeze_plan
-{
-	TransactionId xmax;
-	uint16		t_infomask2;
-	uint16		t_infomask;
-	uint8		frzflags;
-
-	/* Length of individual page offset numbers array for this plan */
-	uint16		ntuples;
-} xl_heap_freeze_plan;
-
-/*
- * This is what we need to know about a block being frozen during vacuum
- *
- * Backup block 0's data contains an array of xl_heap_freeze_plan structs
- * (with nplans elements), followed by one or more page offset number arrays.
- * Each such page offset number array corresponds to a single freeze plan
- * (REDO routine freezes corresponding heap tuples using freeze plan).
- */
-typedef struct xl_heap_freeze_page
-{
-	TransactionId snapshotConflictHorizon;
-	uint16		nplans;
-	bool		isCatalogRel;	/* to handle recovery conflict during logical
-								 * decoding on standby */
-
-	/*
-	 * In payload of blk 0 : FREEZE PLANS and OFFSET NUMBER ARRAY
-	 */
-} xl_heap_freeze_page;
-
-#define SizeOfHeapFreezePage	(offsetof(xl_heap_freeze_page, isCatalogRel) + sizeof(bool))
-
 /*
  * This is what we need to know about setting a visibility map bit
  *
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0014-Vacuum-second-pass-emits-XLOG_HEAP2_PRUNE-record.patch"

From 94f7f2fe6b6b7fdcbe4d60a157d87e607ef70dc0 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 19 Mar 2024 18:50:33 -0400
Subject: [PATCH v4 14/19] Vacuum second pass emits XLOG_HEAP2_PRUNE record

Remove the XLOG_HEAP2_VACUUM record and update vacuum's second pass to
emit a XLOG_HEAP2_PRUNE record. This temporarily wastes some space but a
future commit will streamline xl_heap_prune and ensure that no unused
members are included in the WAL record.
---
 src/backend/access/heap/heapam.c         | 94 ++++--------------------
 src/backend/access/heap/pruneheap.c      | 67 ++++++++++-------
 src/backend/access/heap/vacuumlazy.c     | 12 ++-
 src/backend/access/rmgrdesc/heapdesc.c   | 20 -----
 src/backend/replication/logical/decode.c |  1 -
 src/include/access/heapam.h              |  2 +-
 src/include/access/heapam_xlog.h         | 33 +++++----
 7 files changed, 85 insertions(+), 144 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 532868039d5..16bab55ba02 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8717,23 +8717,34 @@ heap_xlog_prune(XLogReaderState *record)
 	BlockNumber blkno;
 	XLogRedoAction action;
 	bool		get_cleanup_lock;
+	bool		lp_truncate_only;
 
 	XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno);
 
+	lp_truncate_only = xlrec->flags & XLHP_LP_TRUNCATE_ONLY;
+
 	/*
 	 * If there are dead, redirected, or unused items set unused by
 	 * heap_page_prune_and_freeze(), heap_page_prune_execute() will call
 	 * PageRepairFragementation() which expects a full cleanup lock.
 	 */
 	get_cleanup_lock = xlrec->nredirected > 0 ||
-		xlrec->ndead > 0 || xlrec->nunused > 0;
+		xlrec->ndead > 0 ||
+		(xlrec->nunused > 0 && !lp_truncate_only);
+
+	if (lp_truncate_only)
+	{
+		Assert(xlrec->nredirected == 0);
+		Assert(xlrec->ndead == 0);
+		Assert(xlrec->nunused > 0);
+	}
 
 	/*
 	 * We are either about to remove tuples or freeze them. In Hot Standby
 	 * mode, ensure that there's no queries running for which any removed
 	 * tuples are still visible or which consider the frozen xids as running.
 	 */
-	if (InHotStandby)
+	if (xlrec->flags & XLHP_HAS_CONFLICT_HORIZON && InHotStandby)
 		ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
 											xlrec->isCatalogRel,
 											rlocator);
@@ -8772,7 +8783,7 @@ heap_xlog_prune(XLogReaderState *record)
 
 		/* Update all line pointers per the record, and repair fragmentation */
 		if (nredirected > 0 || ndead > 0 || nunused > 0)
-			heap_page_prune_execute(buffer,
+			heap_page_prune_execute(buffer, lp_truncate_only,
 									redirected, nredirected,
 									nowdead, ndead,
 									nowunused, nunused);
@@ -8819,7 +8830,7 @@ heap_xlog_prune(XLogReaderState *record)
 		UnlockReleaseBuffer(buffer);
 
 		/*
-		 * After pruning records from a page, it's useful to update the FSM
+		 * After modifying records on a page, it's useful to update the FSM
 		 * about it, as it may cause the page become target for insertions
 		 * later even if vacuum decides not to visit it (which is possible if
 		 * gets marked all-visible.)
@@ -8831,78 +8842,6 @@ heap_xlog_prune(XLogReaderState *record)
 	}
 }
 
-/*
- * Handles XLOG_HEAP2_VACUUM record type.
- *
- * Acquires an ordinary exclusive lock only.
- */
-static void
-heap_xlog_vacuum(XLogReaderState *record)
-{
-	XLogRecPtr	lsn = record->EndRecPtr;
-	xl_heap_vacuum *xlrec = (xl_heap_vacuum *) XLogRecGetData(record);
-	Buffer		buffer;
-	BlockNumber blkno;
-	XLogRedoAction action;
-
-	/*
-	 * If we have a full-page image, restore it	(without using a cleanup lock)
-	 * and we're done.
-	 */
-	action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL, false,
-										   &buffer);
-	if (action == BLK_NEEDS_REDO)
-	{
-		Page		page = (Page) BufferGetPage(buffer);
-		OffsetNumber *nowunused;
-		Size		datalen;
-		OffsetNumber *offnum;
-
-		nowunused = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
-		/* Shouldn't be a record unless there's something to do */
-		Assert(xlrec->nunused > 0);
-
-		/* Update all now-unused line pointers */
-		offnum = nowunused;
-		for (int i = 0; i < xlrec->nunused; i++)
-		{
-			OffsetNumber off = *offnum++;
-			ItemId		lp = PageGetItemId(page, off);
-
-			Assert(ItemIdIsDead(lp) && !ItemIdHasStorage(lp));
-			ItemIdSetUnused(lp);
-		}
-
-		/* Attempt to truncate line pointer array now */
-		PageTruncateLinePointerArray(page);
-
-		PageSetLSN(page, lsn);
-		MarkBufferDirty(buffer);
-	}
-
-	if (BufferIsValid(buffer))
-	{
-		Size		freespace = PageGetHeapFreeSpace(BufferGetPage(buffer));
-		RelFileLocator rlocator;
-
-		XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno);
-
-		UnlockReleaseBuffer(buffer);
-
-		/*
-		 * After vacuuming LP_DEAD items from a page, it's useful to update
-		 * the FSM about it, as it may cause the page become target for
-		 * insertions later even if vacuum decides not to visit it (which is
-		 * possible if gets marked all-visible.)
-		 *
-		 * Do this regardless of a full-page image being applied, since the
-		 * FSM data is not in the page anyway.
-		 */
-		XLogRecordPageWithFreeSpace(rlocator, blkno, freespace);
-	}
-}
-
 /*
  * Replay XLOG_HEAP2_VISIBLE record.
  *
@@ -9943,9 +9882,6 @@ heap2_redo(XLogReaderState *record)
 		case XLOG_HEAP2_PRUNE:
 			heap_xlog_prune(record);
 			break;
-		case XLOG_HEAP2_VACUUM:
-			heap_xlog_vacuum(record);
-			break;
 		case XLOG_HEAP2_VISIBLE:
 			heap_xlog_visible(record);
 			break;
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 19b50931b90..135fe2dba3e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -601,7 +601,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 		 */
 		if (do_prune)
 		{
-			heap_page_prune_execute(buffer,
+			heap_page_prune_execute(buffer, false,
 									prstate.redirected, prstate.nredirected,
 									prstate.nowdead, prstate.ndead,
 									prstate.nowunused, prstate.nunused);
@@ -668,12 +668,16 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
 	OffsetNumber offsets[MaxHeapTuplesPerPage];
 	bool		do_freeze = presult->nfrozen > 0;
 
+	xlrec.flags = 0;
+
 	xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
 	xlrec.nredirected = prstate->nredirected;
 	xlrec.ndead = prstate->ndead;
 	xlrec.nunused = prstate->nunused;
 	xlrec.nplans = 0;
 
+	xlrec.flags |= XLHP_HAS_CONFLICT_HORIZON;
+
 	/*
 	 * The snapshotConflictHorizon for the whole record should be the most
 	 * conservative of all the horizons calculated for any of the possible
@@ -1149,7 +1153,7 @@ heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum)
  * cleanup lock on the buffer.
  */
 void
-heap_page_prune_execute(Buffer buffer,
+heap_page_prune_execute(Buffer buffer, bool lp_truncate_only,
 						OffsetNumber *redirected, int nredirected,
 						OffsetNumber *nowdead, int ndead,
 						OffsetNumber *nowunused, int nunused)
@@ -1171,6 +1175,7 @@ heap_page_prune_execute(Buffer buffer,
 		ItemId		tolp PG_USED_FOR_ASSERTS_ONLY;
 
 #ifdef USE_ASSERT_CHECKING
+		Assert(!lp_truncate_only);
 
 		/*
 		 * Any existing item that we set as an LP_REDIRECT (any 'from' item)
@@ -1226,6 +1231,7 @@ heap_page_prune_execute(Buffer buffer,
 		ItemId		lp = PageGetItemId(page, off);
 
 #ifdef USE_ASSERT_CHECKING
+		Assert(!lp_truncate_only);
 
 		/*
 		 * An LP_DEAD line pointer must be left behind when the original item
@@ -1259,23 +1265,29 @@ heap_page_prune_execute(Buffer buffer,
 
 #ifdef USE_ASSERT_CHECKING
 
-		/*
-		 * When heap_page_prune_and_freeze() was called, mark_unused_now may
-		 * have been passed as true, which allows would-be LP_DEAD items to be
-		 * made LP_UNUSED instead. This is only possible if the relation has
-		 * no indexes. If there are any dead items, then mark_unused_now was
-		 * not true and every item being marked LP_UNUSED must refer to a
-		 * heap-only tuple.
-		 */
-		if (ndead > 0)
+		if (lp_truncate_only)
 		{
-			Assert(ItemIdHasStorage(lp) && ItemIdIsNormal(lp));
-			htup = (HeapTupleHeader) PageGetItem(page, lp);
-			Assert(HeapTupleHeaderIsHeapOnly(htup));
+			/* Setting LP_DEAD to LP_UNUSED in vacuum's second pass */
+			Assert(ItemIdIsDead(lp) && !ItemIdHasStorage(lp));
 		}
 		else
 		{
-			Assert(ItemIdIsUsed(lp));
+			/*
+			 * When heap_page_prune_and_freeze() was called, mark_unused_now
+			 * may have been passed as true, which allows would-be LP_DEAD
+			 * items to be made LP_UNUSED instead. This is only possible if
+			 * the relation has no indexes. If there are any dead items, then
+			 * mark_unused_now was not true and every item being marked
+			 * LP_UNUSED must refer to a heap-only tuple.
+			 */
+			if (ndead > 0)
+			{
+				Assert(ItemIdHasStorage(lp) && ItemIdIsNormal(lp));
+				htup = (HeapTupleHeader) PageGetItem(page, lp);
+				Assert(HeapTupleHeaderIsHeapOnly(htup));
+			}
+			else
+				Assert(ItemIdIsUsed(lp));
 		}
 
 #endif
@@ -1283,17 +1295,22 @@ heap_page_prune_execute(Buffer buffer,
 		ItemIdSetUnused(lp);
 	}
 
-	/*
-	 * Finally, repair any fragmentation, and update the page's hint bit about
-	 * whether it has free pointers.
-	 */
-	PageRepairFragmentation(page);
+	if (lp_truncate_only)
+		PageTruncateLinePointerArray(page);
+	else
+	{
+		/*
+		 * Finally, repair any fragmentation, and update the page's hint bit
+		 * about whether it has free pointers.
+		 */
+		PageRepairFragmentation(page);
 
-	/*
-	 * Now that the page has been modified, assert that redirect items still
-	 * point to valid targets.
-	 */
-	page_verify_redirects(page);
+		/*
+		 * Now that the page has been modified, assert that redirect items
+		 * still point to valid targets.
+		 */
+		page_verify_redirects(page);
+	}
 }
 
 
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index c4553a4159c..9dfb56475cf 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2394,18 +2394,24 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 	/* XLOG stuff */
 	if (RelationNeedsWAL(vacrel->rel))
 	{
-		xl_heap_vacuum xlrec;
+		xl_heap_prune xlrec;
 		XLogRecPtr	recptr;
 
+		xlrec.flags = XLHP_LP_TRUNCATE_ONLY;
+		xlrec.snapshotConflictHorizon = InvalidTransactionId;
+		xlrec.nplans = 0;
+		xlrec.nredirected = 0;
+		xlrec.ndead = 0;
 		xlrec.nunused = nunused;
+		xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(vacrel->rel);
 
 		XLogBeginInsert();
-		XLogRegisterData((char *) &xlrec, SizeOfHeapVacuum);
+		XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
 
 		XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
 		XLogRegisterBufData(0, (char *) unused, nunused * sizeof(OffsetNumber));
 
-		recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_VACUUM);
+		recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
 
 		PageSetLSN(page, recptr);
 	}
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 9f0a0341d40..ea03f902fc4 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -242,23 +242,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
 			}
 		}
 	}
-	else if (info == XLOG_HEAP2_VACUUM)
-	{
-		xl_heap_vacuum *xlrec = (xl_heap_vacuum *) rec;
-
-		appendStringInfo(buf, "nunused: %u", xlrec->nunused);
-
-		if (XLogRecHasBlockData(record, 0))
-		{
-			OffsetNumber *nowunused;
-
-			nowunused = (OffsetNumber *) XLogRecGetBlockData(record, 0, NULL);
-
-			appendStringInfoString(buf, ", unused:");
-			array_desc(buf, nowunused, sizeof(OffsetNumber), xlrec->nunused,
-					   &offset_elem_desc, NULL);
-		}
-	}
 	else if (info == XLOG_HEAP2_VISIBLE)
 	{
 		xl_heap_visible *xlrec = (xl_heap_visible *) rec;
@@ -360,9 +343,6 @@ heap2_identify(uint8 info)
 		case XLOG_HEAP2_PRUNE:
 			id = "PRUNE";
 			break;
-		case XLOG_HEAP2_VACUUM:
-			id = "VACUUM";
-			break;
 		case XLOG_HEAP2_VISIBLE:
 			id = "VISIBLE";
 			break;
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index f77051572fd..38d1bdd825e 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -446,7 +446,6 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 			 * interested in.
 			 */
 		case XLOG_HEAP2_PRUNE:
-		case XLOG_HEAP2_VACUUM:
 		case XLOG_HEAP2_VISIBLE:
 		case XLOG_HEAP2_LOCK_UPDATED:
 			break;
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 321a46185e1..d5cb8f99cac 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -349,7 +349,7 @@ extern void heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 									   HeapPageFreeze *pagefrz,
 									   PruneFreezeResult *presult,
 									   OffsetNumber *off_loc);
-extern void heap_page_prune_execute(Buffer buffer,
+extern void heap_page_prune_execute(Buffer buffer, bool lp_truncate_only,
 									OffsetNumber *redirected, int nredirected,
 									OffsetNumber *nowdead, int ndead,
 									OffsetNumber *nowunused, int nunused);
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index fe4a8ff0620..2393540cf68 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -52,11 +52,10 @@
  */
 #define XLOG_HEAP2_REWRITE		0x00
 #define XLOG_HEAP2_PRUNE		0x10
-#define XLOG_HEAP2_VACUUM		0x20
-#define XLOG_HEAP2_VISIBLE		0x30
-#define XLOG_HEAP2_MULTI_INSERT 0x40
-#define XLOG_HEAP2_LOCK_UPDATED 0x50
-#define XLOG_HEAP2_NEW_CID		0x60
+#define XLOG_HEAP2_VISIBLE		0x20
+#define XLOG_HEAP2_MULTI_INSERT 0x30
+#define XLOG_HEAP2_LOCK_UPDATED 0x40
+#define XLOG_HEAP2_NEW_CID		0x50
 
 /*
  * xl_heap_insert/xl_heap_multi_insert flag values, 8 bits are available.
@@ -266,6 +265,7 @@ typedef struct xl_heap_freeze_plan
  */
 typedef struct xl_heap_prune
 {
+	uint8		flags;
 	TransactionId snapshotConflictHorizon;
 	uint16		nplans;
 	uint16		nredirected;
@@ -288,19 +288,22 @@ typedef struct xl_heap_prune
 
 #define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
 
+/* Flags for xl_heap_prune */
+
 /*
- * The vacuum page record is similar to the prune record, but can only mark
- * already LP_DEAD items LP_UNUSED (during VACUUM's second heap pass)
- *
- * Acquires an ordinary exclusive lock only.
+ * During vacuum's second pass which sets LP_DEAD items LP_UNUSED, we will only
+ * truncate the line pointer array, not call PageRepairFragmentation. We need
+ * this flag to differentiate what kind of lock (exclusive or cleanup) to take
+ * on the buffer and whether to call PageTruncateLinePointerArray() or
+ * PageRepairFragementation().
  */
-typedef struct xl_heap_vacuum
-{
-	uint16		nunused;
-	/* OFFSET NUMBERS are in the block reference 0 */
-} xl_heap_vacuum;
+#define		XLHP_LP_TRUNCATE_ONLY       (1 << 1)
 
-#define SizeOfHeapVacuum (offsetof(xl_heap_vacuum, nunused) + sizeof(uint16))
+/*
+ * Vacuum's first pass and on-access pruning may need to include a snapshot
+ * conflict horizon.
+ */
+#define		XLHP_HAS_CONFLICT_HORIZON   (1 << 2)
 
 /* flags for infobits_set */
 #define XLHL_XMAX_IS_MULTI		0x01
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0015-Set-hastup-in-heap_page_prune.patch"

From 77a8d20e1c332444d1bde1a41682186713e51e5f Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 18 Mar 2024 20:12:18 -0400
Subject: [PATCH v4 15/19] Set hastup in heap_page_prune

lazy_scan_prune() loops through the line pointers and tuple visibility
information for each tuple on a page, setting hastup to true if there
are any LP_REDIRECT line pointers or tuples with storage which will not
be removed. We want to remove this extra loop from lazy_scan_prune(),
and we know about non-removable tuples during heap_page_prune() anyway.
Set hastup when recording LP_REDIRECT line pointers in
heap_prune_chain() and when LP_NORMAL line pointers refer to tuples
whose visibility status is not HEAPTUPLE_DEAD.
---
 src/backend/access/heap/pruneheap.c  | 64 ++++++++++++++++++----------
 src/backend/access/heap/vacuumlazy.c | 24 +----------
 src/include/access/heapam.h          |  2 +
 3 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 135fe2dba3e..d183912a402 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -71,7 +71,8 @@ static int	heap_prune_chain(Buffer buffer,
 
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
-									   OffsetNumber offnum, OffsetNumber rdoffnum);
+									   OffsetNumber offnum, OffsetNumber rdoffnum,
+									   PruneFreezeResult *presult);
 static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
 								   PruneFreezeResult *presult);
 static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum,
@@ -277,6 +278,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	presult->nnewlpdead = 0;
 	presult->nfrozen = 0;
 
+	presult->hastup = false;
+
 	/*
 	 * Caller will update the VM after pruning, collecting LP_DEAD items, and
 	 * freezing tuples. Keep track of whether or not the page is all_visible
@@ -419,30 +422,42 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 				break;
 		}
 
-		/*
-		 * Consider freezing any normal tuples which will not be removed
-		 */
-		if (presult->htsv[offnum] != HEAPTUPLE_DEAD && pagefrz)
+		if (presult->htsv[offnum] != HEAPTUPLE_DEAD)
 		{
-			bool		totally_frozen;
+			/*
+			 * Deliberately don't set hastup for LP_DEAD items.  We make the
+			 * soft assumption that any LP_DEAD items encountered here will
+			 * become LP_UNUSED later on, before count_nondeletable_pages is
+			 * reached.  If we don't make this assumption then rel truncation
+			 * will only happen every other VACUUM, at most.  Besides, VACUUM
+			 * must treat hastup/nonempty_pages as provisional no matter how
+			 * LP_DEAD items are handled (handled here, or handled later on).
+			 */
+			presult->hastup = true;
 
-			/* Tuple with storage -- consider need to freeze */
-			if ((heap_prepare_freeze_tuple(htup, pagefrz,
-										   &prstate.frozen[presult->nfrozen],
-										   &totally_frozen)))
+			/* Consider freezing any normal tuples which will not be removed */
+			if (pagefrz)
 			{
-				/* Save prepared freeze plan for later */
-				prstate.frozen[presult->nfrozen++].offset = offnum;
-			}
+				bool		totally_frozen;
 
-			/*
-			 * If any tuple isn't either totally frozen already or eligible to
-			 * become totally frozen (according to its freeze plan), then the
-			 * page definitely cannot be set all-frozen in the visibility map
-			 * later on
-			 */
-			if (!totally_frozen)
-				presult->all_frozen = false;
+				/* Tuple with storage -- consider need to freeze */
+				if ((heap_prepare_freeze_tuple(htup, pagefrz,
+											   &prstate.frozen[presult->nfrozen],
+											   &totally_frozen)))
+				{
+					/* Save prepared freeze plan for later */
+					prstate.frozen[presult->nfrozen++].offset = offnum;
+				}
+
+				/*
+				 * If any tuple isn't either totally frozen already or
+				 * eligible to become totally frozen (according to its freeze
+				 * plan), then the page definitely cannot be set all-frozen in
+				 * the visibility map later on
+				 */
+				if (!totally_frozen)
+					presult->all_frozen = false;
+			}
 		}
 	}
 
@@ -1049,7 +1064,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 		if (i >= nchain)
 			heap_prune_record_dead_or_unused(prstate, rootoffnum, presult);
 		else
-			heap_prune_record_redirect(prstate, rootoffnum, chainitems[i]);
+			heap_prune_record_redirect(prstate, rootoffnum, chainitems[i], presult);
 	}
 	else if (nchain < 2 && ItemIdIsRedirected(rootlp))
 	{
@@ -1083,7 +1098,8 @@ heap_prune_record_prunable(PruneState *prstate, TransactionId xid)
 /* Record line pointer to be redirected */
 static void
 heap_prune_record_redirect(PruneState *prstate,
-						   OffsetNumber offnum, OffsetNumber rdoffnum)
+						   OffsetNumber offnum, OffsetNumber rdoffnum,
+						   PruneFreezeResult *presult)
 {
 	Assert(prstate->nredirected < MaxHeapTuplesPerPage);
 	prstate->redirected[prstate->nredirected * 2] = offnum;
@@ -1093,6 +1109,8 @@ heap_prune_record_redirect(PruneState *prstate,
 	prstate->marked[offnum] = true;
 	Assert(!prstate->marked[rdoffnum]);
 	prstate->marked[rdoffnum] = true;
+
+	presult->hastup = true;
 }
 
 /* Record line pointer to be marked dead */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 9dfb56475cf..370721a619a 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1420,7 +1420,6 @@ lazy_scan_prune(LVRelState *vacrel,
 				live_tuples,
 				recently_dead_tuples;
 	HeapPageFreeze pagefrz;
-	bool		hastup = false;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
 
 	Assert(BufferGetBlockNumber(buf) == blkno);
@@ -1477,28 +1476,12 @@ lazy_scan_prune(LVRelState *vacrel,
 		vacrel->offnum = offnum;
 		itemid = PageGetItemId(page, offnum);
 
-		if (!ItemIdIsUsed(itemid))
-			continue;
-
 		/* Redirect items mustn't be touched */
-		if (ItemIdIsRedirected(itemid))
-		{
-			/* page makes rel truncation unsafe */
-			hastup = true;
+		if (ItemIdIsRedirected(itemid) || !ItemIdIsUsed(itemid))
 			continue;
-		}
 
 		if (ItemIdIsDead(itemid))
 		{
-			/*
-			 * Deliberately don't set hastup for LP_DEAD items.  We make the
-			 * soft assumption that any LP_DEAD items encountered here will
-			 * become LP_UNUSED later on, before count_nondeletable_pages is
-			 * reached.  If we don't make this assumption then rel truncation
-			 * will only happen every other VACUUM, at most.  Besides, VACUUM
-			 * must treat hastup/nonempty_pages as provisional no matter how
-			 * LP_DEAD items are handled (handled here, or handled later on).
-			 */
 			deadoffsets[lpdead_items++] = offnum;
 			continue;
 		}
@@ -1566,9 +1549,6 @@ lazy_scan_prune(LVRelState *vacrel,
 				elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
 				break;
 		}
-
-		hastup = true;			/* page makes rel truncation unsafe */
-
 	}
 
 	vacrel->offnum = InvalidOffsetNumber;
@@ -1650,7 +1630,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	vacrel->recently_dead_tuples += recently_dead_tuples;
 
 	/* Can't truncate this page */
-	if (hastup)
+	if (presult.hastup)
 		vacrel->nonempty_pages = blkno + 1;
 
 	/* Did we find LP_DEAD items? */
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index d5cb8f99cac..25dbae8139e 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -202,6 +202,8 @@ typedef struct PruneFreezeResult
 	int			nnewlpdead;		/* Number of newly LP_DEAD items */
 	bool		all_visible;	/* Whether or not the page is all visible */
 	bool		all_visible_except_removable;
+	bool		hastup;			/* Does page make rel truncation unsafe */
+
 	/* Whether or not the page can be set all frozen in the VM */
 	bool		all_frozen;
 
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0016-Count-tuples-for-vacuum-logging-in-heap_page_prun.patch"

From f9356564f651fa9f91e99adbbac50914601e3551 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 22 Jan 2024 17:25:56 -0500
Subject: [PATCH v4 16/19] Count tuples for vacuum logging in heap_page_prune

lazy_scan_prune() loops through all of the tuple visibility information
that was recorded in heap_page_prune() and then counts live and recently
dead tuples. That information is available in heap_page_prune(), so just
record it there. Add live and recently dead tuple counters to the
PruneResult. Doing this counting in heap_page_prune() eliminates the
need for saving the tuple visibility status information in the
PruneResult. Instead, save it in the PruneState where it can be
referenced by heap_prune_chain().
---
 src/backend/access/heap/pruneheap.c  | 99 ++++++++++++++++++++++++----
 src/backend/access/heap/vacuumlazy.c | 93 +-------------------------
 src/include/access/heapam.h          | 29 +-------
 3 files changed, 93 insertions(+), 128 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index d183912a402..f59b03222b0 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -55,6 +55,18 @@ typedef struct
 	 */
 	bool		marked[MaxHeapTuplesPerPage + 1];
 
+	/*
+	 * Tuple visibility is only computed once for each tuple, for correctness
+	 * and efficiency reasons; see comment in heap_page_prune_and_freeze() for
+	 * details. This is of type int8[], instead of HTSV_Result[], so we can
+	 * use -1 to indicate no visibility has been computed, e.g. for LP_DEAD
+	 * items.
+	 *
+	 * This needs to be MaxHeapTuplesPerPage + 1 long as FirstOffsetNumber is
+	 * 1. Otherwise every access would need to subtract 1.
+	 */
+	int8		htsv[MaxHeapTuplesPerPage + 1];
+
 	/*
 	 * One entry for every tuple that we may freeze.
 	 */
@@ -69,6 +81,7 @@ static int	heap_prune_chain(Buffer buffer,
 							 OffsetNumber rootoffnum,
 							 PruneState *prstate, PruneFreezeResult *presult);
 
+static inline HTSV_Result htsv_get_valid_status(int status);
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
 									   OffsetNumber offnum, OffsetNumber rdoffnum,
@@ -269,7 +282,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	prstate.nredirected = prstate.ndead = prstate.nunused = 0;
 
 	/*
-	 * presult->htsv is not initialized here because all ntuple spots in the
+	 * prstate.htsv is not initialized here because all ntuple spots in the
 	 * array will be set either to a valid HTSV_Result value or -1.
 	 */
 	memset(prstate.marked, 0, sizeof(prstate.marked));
@@ -280,6 +293,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	presult->hastup = false;
 
+	presult->live_tuples = 0;
+	presult->recently_dead_tuples = 0;
+
 	/*
 	 * Caller will update the VM after pruning, collecting LP_DEAD items, and
 	 * freezing tuples. Keep track of whether or not the page is all_visible
@@ -329,7 +345,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 		/* Nothing to do if slot doesn't contain a tuple */
 		if (!ItemIdIsNormal(itemid))
 		{
-			presult->htsv[offnum] = -1;
+			prstate.htsv[offnum] = -1;
 			continue;
 		}
 
@@ -345,9 +361,29 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 		if (off_loc)
 			*off_loc = offnum;
 
-		presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
-															buffer);
-		switch (presult->htsv[offnum])
+		prstate.htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
+														   buffer);
+
+		/*
+		 * The criteria for counting a tuple as live in this block need to
+		 * match what analyze.c's acquire_sample_rows() does, otherwise VACUUM
+		 * and ANALYZE may produce wildly different reltuples values, e.g.
+		 * when there are many recently-dead tuples.
+		 *
+		 * The logic here is a bit simpler than acquire_sample_rows(), as
+		 * VACUUM can't run inside a transaction block, which makes some cases
+		 * impossible (e.g. in-progress insert from the same transaction).
+		 *
+		 * We treat LP_DEAD items (which are the closest thing to DEAD tuples
+		 * that might be seen here) differently, too: we assume that they'll
+		 * become LP_UNUSED before VACUUM finishes.  This difference is only
+		 * superficial.  VACUUM effectively agrees with ANALYZE about DEAD
+		 * items, in the end.  VACUUM won't remember LP_DEAD items, but only
+		 * because they're not supposed to be left behind when it is done.
+		 * (Cases where we bypass index vacuuming will violate this optimistic
+		 * assumption, but the overall impact of that should be negligible.)
+		 */
+		switch (prstate.htsv[offnum])
 		{
 			case HEAPTUPLE_DEAD:
 
@@ -367,6 +403,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 				break;
 			case HEAPTUPLE_LIVE:
 
+				/*
+				 * Count it as live.  Not only is this natural, but it's also
+				 * what acquire_sample_rows() does.
+				 */
+				presult->live_tuples++;
+
 				/*
 				 * Is the tuple definitely visible to all transactions?
 				 *
@@ -408,13 +450,34 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 				}
 				break;
 			case HEAPTUPLE_RECENTLY_DEAD:
+
+				/*
+				 * If tuple is recently dead then we must not remove it from
+				 * the relation.  (We only remove items that are LP_DEAD from
+				 * pruning.)
+				 */
+				presult->recently_dead_tuples++;
 				presult->all_visible = false;
 				break;
 			case HEAPTUPLE_INSERT_IN_PROGRESS:
+
+				/*
+				 * We do not count these rows as live, because we expect the
+				 * inserting transaction to update the counters at commit, and
+				 * we assume that will happen only after we report our
+				 * results.  This assumption is a bit shaky, but it is what
+				 * acquire_sample_rows() does, so be consistent.
+				 */
 				presult->all_visible = false;
 				break;
 			case HEAPTUPLE_DELETE_IN_PROGRESS:
-				/* This is an expected case during concurrent vacuum */
+
+				/*
+				 * This an expected case during concurrent vacuum. Count such
+				 * rows as live.  As above, we assume the deleting transaction
+				 * will commit and update the counters after we report.
+				 */
+				presult->live_tuples++;
 				presult->all_visible = false;
 				break;
 			default:
@@ -422,7 +485,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 				break;
 		}
 
-		if (presult->htsv[offnum] != HEAPTUPLE_DEAD)
+		if (prstate.htsv[offnum] != HEAPTUPLE_DEAD)
 		{
 			/*
 			 * Deliberately don't set hastup for LP_DEAD items.  We make the
@@ -772,10 +835,24 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
 }
 
 
+/*
+ * Pruning calculates tuple visibility once and saves the results in an array
+ * of int8. See PruneState.htsv for details. This helper function is meant to
+ * guard against examining visibility status array members which have not yet
+ * been computed.
+ */
+static inline HTSV_Result
+htsv_get_valid_status(int status)
+{
+	Assert(status >= HEAPTUPLE_DEAD &&
+		   status <= HEAPTUPLE_DELETE_IN_PROGRESS);
+	return (HTSV_Result) status;
+}
+
 /*
  * Prune specified line pointer or a HOT chain originating at line pointer.
  *
- * Tuple visibility information is provided in presult->htsv.
+ * Tuple visibility information is provided in prstate->htsv.
  *
  * If the item is an index-referenced tuple (i.e. not a heap-only tuple),
  * the HOT chain is pruned by removing all DEAD tuples at the start of the HOT
@@ -826,7 +903,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 	 */
 	if (ItemIdIsNormal(rootlp))
 	{
-		Assert(presult->htsv[rootoffnum] != -1);
+		Assert(prstate->htsv[rootoffnum] != -1);
 		htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
 
 		if (HeapTupleHeaderIsHeapOnly(htup))
@@ -849,7 +926,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 			 * either here or while following a chain below.  Whichever path
 			 * gets there first will mark the tuple unused.
 			 */
-			if (presult->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
+			if (prstate->htsv[rootoffnum] == HEAPTUPLE_DEAD &&
 				!HeapTupleHeaderIsHotUpdated(htup))
 			{
 				heap_prune_record_unused(prstate, rootoffnum);
@@ -950,7 +1027,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 		 */
 		tupdead = recent_dead = false;
 
-		switch (htsv_get_valid_status(presult->htsv[offnum]))
+		switch (htsv_get_valid_status(prstate->htsv[offnum]))
 		{
 			case HEAPTUPLE_DEAD:
 				tupdead = true;
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 370721a619a..a3c971cd26d 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1378,22 +1378,6 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
  *
  * Caller must hold pin and buffer cleanup lock on the buffer.
  *
- * Prior to PostgreSQL 14 there were very rare cases where
- * heap_page_prune_and_freeze() was allowed to disagree with our
- * HeapTupleSatisfiesVacuum() call about whether or not a tuple should be
- * considered DEAD.  This happened when an inserting transaction concurrently
- * aborted (after our heap_page_prune_and_freeze() call, before our
- * HeapTupleSatisfiesVacuum() call).  There was rather a lot of complexity just
- * so we could deal with tuples that were DEAD to VACUUM, but nevertheless were
- * left with storage after pruning.
- *
- * As of Postgres 17, we circumvent this problem altogether by reusing the
- * result of heap_page_prune_and_freeze()'s visibility check. Without the
- * second call to HeapTupleSatisfiesVacuum(), there is no new HTSV_Result and
- * there can be no disagreement. We'll just handle such tuples as if they had
- * become fully dead right after this operation completes instead of in the
- * middle of it.
- *
  * 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
@@ -1415,10 +1399,8 @@ lazy_scan_prune(LVRelState *vacrel,
 	OffsetNumber offnum,
 				maxoff;
 	ItemId		itemid;
+	int			lpdead_items = 0;
 	PruneFreezeResult presult;
-	int			lpdead_items,
-				live_tuples,
-				recently_dead_tuples;
 	HeapPageFreeze pagefrz;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
 
@@ -1438,9 +1420,6 @@ lazy_scan_prune(LVRelState *vacrel,
 	pagefrz.NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
 	pagefrz.NoFreezePageRelminMxid = vacrel->NewRelminMxid;
 	pagefrz.cutoffs = &vacrel->cutoffs;
-	lpdead_items = 0;
-	live_tuples = 0;
-	recently_dead_tuples = 0;
 
 	/*
 	 * Prune all HOT-update chains and potentially freeze tuples on this page.
@@ -1476,9 +1455,6 @@ lazy_scan_prune(LVRelState *vacrel,
 		vacrel->offnum = offnum;
 		itemid = PageGetItemId(page, offnum);
 
-		/* Redirect items mustn't be touched */
-		if (ItemIdIsRedirected(itemid) || !ItemIdIsUsed(itemid))
-			continue;
 
 		if (ItemIdIsDead(itemid))
 		{
@@ -1486,69 +1462,6 @@ lazy_scan_prune(LVRelState *vacrel,
 			continue;
 		}
 
-		Assert(ItemIdIsNormal(itemid));
-
-		/*
-		 * The criteria for counting a tuple as live in this block need to
-		 * match what analyze.c's acquire_sample_rows() does, otherwise VACUUM
-		 * and ANALYZE may produce wildly different reltuples values, e.g.
-		 * when there are many recently-dead tuples.
-		 *
-		 * The logic here is a bit simpler than acquire_sample_rows(), as
-		 * VACUUM can't run inside a transaction block, which makes some cases
-		 * impossible (e.g. in-progress insert from the same transaction).
-		 *
-		 * We treat LP_DEAD items (which are the closest thing to DEAD tuples
-		 * that might be seen here) differently, too: we assume that they'll
-		 * become LP_UNUSED before VACUUM finishes.  This difference is only
-		 * superficial.  VACUUM effectively agrees with ANALYZE about DEAD
-		 * items, in the end.  VACUUM won't remember LP_DEAD items, but only
-		 * because they're not supposed to be left behind when it is done.
-		 * (Cases where we bypass index vacuuming will violate this optimistic
-		 * assumption, but the overall impact of that should be negligible.)
-		 */
-		switch (htsv_get_valid_status(presult.htsv[offnum]))
-		{
-			case HEAPTUPLE_LIVE:
-
-				/*
-				 * Count it as live.  Not only is this natural, but it's also
-				 * what acquire_sample_rows() does.
-				 */
-				live_tuples++;
-				break;
-			case HEAPTUPLE_RECENTLY_DEAD:
-
-				/*
-				 * If tuple is recently dead then we must not remove it from
-				 * the relation.  (We only remove items that are LP_DEAD from
-				 * pruning.)
-				 */
-				recently_dead_tuples++;
-				break;
-			case HEAPTUPLE_INSERT_IN_PROGRESS:
-
-				/*
-				 * We do not count these rows as live, because we expect the
-				 * inserting transaction to update the counters at commit, and
-				 * we assume that will happen only after we report our
-				 * results.  This assumption is a bit shaky, but it is what
-				 * acquire_sample_rows() does, so be consistent.
-				 */
-				break;
-			case HEAPTUPLE_DELETE_IN_PROGRESS:
-
-				/*
-				 * This an expected case during concurrent vacuum. Count such
-				 * rows as live.  As above, we assume the deleting transaction
-				 * will commit and update the counters after we report.
-				 */
-				live_tuples++;
-				break;
-			default:
-				elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
-				break;
-		}
 	}
 
 	vacrel->offnum = InvalidOffsetNumber;
@@ -1626,8 +1539,8 @@ lazy_scan_prune(LVRelState *vacrel,
 	vacrel->tuples_deleted += presult.ndeleted;
 	vacrel->tuples_frozen += presult.nfrozen;
 	vacrel->lpdead_items += lpdead_items;
-	vacrel->live_tuples += live_tuples;
-	vacrel->recently_dead_tuples += recently_dead_tuples;
+	vacrel->live_tuples += presult.live_tuples;
+	vacrel->recently_dead_tuples += presult.recently_dead_tuples;
 
 	/* Can't truncate this page */
 	if (presult.hastup)
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 25dbae8139e..23ba23b5b01 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -198,6 +198,8 @@ typedef struct HeapPageFreeze
  */
 typedef struct PruneFreezeResult
 {
+	int			live_tuples;
+	int			recently_dead_tuples;
 	int			ndeleted;		/* Number of tuples deleted from the page */
 	int			nnewlpdead;		/* Number of newly LP_DEAD items */
 	bool		all_visible;	/* Whether or not the page is all visible */
@@ -211,19 +213,6 @@ typedef struct PruneFreezeResult
 	int			nfrozen;
 	TransactionId frz_conflict_horizon; /* Newest xmin on the page */
 
-	/*
-	 * Tuple visibility is only computed once for each tuple, for correctness
-	 * and efficiency reasons; see comment in heap_page_prune_and_freeze() for
-	 * details. This is of type int8[], instead of HTSV_Result[], so we can
-	 * use -1 to indicate no visibility has been computed, e.g. for LP_DEAD
-	 * items.
-	 *
-	 * This needs to be MaxHeapTuplesPerPage + 1 long as FirstOffsetNumber is
-	 * 1. Otherwise every access would need to subtract 1.
-	 */
-	int8		htsv[MaxHeapTuplesPerPage + 1];
-
-
 	/* New value of relfrozenxid found by heap_page_prune_and_freeze() */
 	TransactionId new_relfrozenxid;
 
@@ -231,20 +220,6 @@ typedef struct PruneFreezeResult
 	MultiXactId new_relminmxid;
 } PruneFreezeResult;
 
-/*
- * Pruning calculates tuple visibility once and saves the results in an array
- * of int8. See PruneFreezeResult.htsv for details. This helper function is
- * meant to guard against examining visibility status array members which have
- * not yet been computed.
- */
-static inline HTSV_Result
-htsv_get_valid_status(int status)
-{
-	Assert(status >= HEAPTUPLE_DEAD &&
-		   status <= HEAPTUPLE_DELETE_IN_PROGRESS);
-	return (HTSV_Result) status;
-}
-
 /* ----------------
  *		function prototypes for heap access method
  *
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0017-Save-dead-tuple-offsets-during-heap_page_prune.patch"

From a322901bb7fa86a0a7bc64b339d52c1446488915 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 22 Jan 2024 16:55:28 -0500
Subject: [PATCH v4 17/19] Save dead tuple offsets during heap_page_prune

After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
 src/backend/access/heap/pruneheap.c  |  7 ++++
 src/backend/access/heap/vacuumlazy.c | 61 ++++++----------------------
 src/include/access/heapam.h          |  2 +
 3 files changed, 22 insertions(+), 48 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f59b03222b0..5decb1127d0 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -295,6 +295,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	presult->live_tuples = 0;
 	presult->recently_dead_tuples = 0;
+	presult->lpdead_items = 0;
 
 	/*
 	 * Caller will update the VM after pruning, collecting LP_DEAD items, and
@@ -1002,7 +1003,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 			if (unlikely(prstate->mark_unused_now))
 				heap_prune_record_unused(prstate, offnum);
 			else
+			{
 				presult->all_visible = false;
+				presult->deadoffsets[presult->lpdead_items++] = offnum;
+			}
 
 			break;
 		}
@@ -1206,6 +1210,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
 	 * all_visible.
 	 */
 	presult->all_visible = false;
+
+	/* Record the dead offset for vacuum */
+	presult->deadoffsets[presult->lpdead_items++] = offnum;
 }
 
 /*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a3c971cd26d..295846b854f 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1396,23 +1396,11 @@ lazy_scan_prune(LVRelState *vacrel,
 				bool *has_lpdead_items)
 {
 	Relation	rel = vacrel->rel;
-	OffsetNumber offnum,
-				maxoff;
-	ItemId		itemid;
-	int			lpdead_items = 0;
 	PruneFreezeResult presult;
 	HeapPageFreeze pagefrz;
-	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
 
 	Assert(BufferGetBlockNumber(buf) == blkno);
 
-	/*
-	 * maxoff might be reduced following line pointer array truncation in
-	 * heap_page_prune_and_freeze().  That's safe for us to ignore, since the
-	 * reclaimed space will continue to look like LP_UNUSED items below.
-	 */
-	maxoff = PageGetMaxOffsetNumber(page);
-
 	/* Initialize pagefrz */
 	pagefrz.freeze_required = false;
 	pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1425,9 +1413,9 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * Prune all HOT-update chains and potentially freeze tuples on this page.
 	 *
 	 * We count the number of tuples removed from the page by the pruning step
-	 * in presult.ndeleted. It should not be confused with lpdead_items;
-	 * lpdead_items's final value can be thought of as the number of tuples
-	 * that were deleted from indexes.
+	 * in 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.
 	 *
 	 * If the relation has no indexes, we can immediately mark would-be dead
 	 * items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1437,33 +1425,10 @@ lazy_scan_prune(LVRelState *vacrel,
 							   &pagefrz, &presult, &vacrel->offnum);
 
 	/*
-	 * Now scan the page to collect LP_DEAD items and check for tuples
-	 * requiring freezing among remaining tuples with storage. 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 and able to
-	 * become all_frozen.
-	 *
+	 * 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.
 	 */
-	for (offnum = FirstOffsetNumber;
-		 offnum <= maxoff;
-		 offnum = OffsetNumberNext(offnum))
-	{
-		/*
-		 * Set the offset number so that we can display it along with any
-		 * error that occurred while processing this tuple.
-		 */
-		vacrel->offnum = offnum;
-		itemid = PageGetItemId(page, offnum);
-
-
-		if (ItemIdIsDead(itemid))
-		{
-			deadoffsets[lpdead_items++] = offnum;
-			continue;
-		}
-
-	}
-
 	vacrel->offnum = InvalidOffsetNumber;
 
 	Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1499,7 +1464,7 @@ lazy_scan_prune(LVRelState *vacrel,
 		TransactionId debug_cutoff;
 		bool		debug_all_frozen;
 
-		Assert(lpdead_items == 0);
+		Assert(presult.lpdead_items == 0);
 
 		if (!heap_page_is_all_visible(vacrel, buf,
 									  &debug_cutoff, &debug_all_frozen))
@@ -1515,7 +1480,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	/*
 	 * Now save details of the LP_DEAD items from the page in vacrel
 	 */
-	if (lpdead_items > 0)
+	if (presult.lpdead_items > 0)
 	{
 		VacDeadItems *dead_items = vacrel->dead_items;
 		ItemPointerData tmp;
@@ -1524,9 +1489,9 @@ lazy_scan_prune(LVRelState *vacrel,
 
 		ItemPointerSetBlockNumber(&tmp, blkno);
 
-		for (int i = 0; i < lpdead_items; i++)
+		for (int i = 0; i < presult.lpdead_items; i++)
 		{
-			ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+			ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
 			dead_items->items[dead_items->num_items++] = tmp;
 		}
 
@@ -1538,7 +1503,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	/* Finally, add page-local counts to whole-VACUUM counts */
 	vacrel->tuples_deleted += presult.ndeleted;
 	vacrel->tuples_frozen += presult.nfrozen;
-	vacrel->lpdead_items += lpdead_items;
+	vacrel->lpdead_items += presult.lpdead_items;
 	vacrel->live_tuples += presult.live_tuples;
 	vacrel->recently_dead_tuples += presult.recently_dead_tuples;
 
@@ -1547,7 +1512,7 @@ lazy_scan_prune(LVRelState *vacrel,
 		vacrel->nonempty_pages = blkno + 1;
 
 	/* Did we find LP_DEAD items? */
-	*has_lpdead_items = (lpdead_items > 0);
+	*has_lpdead_items = (presult.lpdead_items > 0);
 
 	Assert(!presult.all_visible || !(*has_lpdead_items));
 
@@ -1615,7 +1580,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
 	 * however.
 	 */
-	else if (lpdead_items > 0 && PageIsAllVisible(page))
+	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);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 23ba23b5b01..bdde17eb230 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -218,6 +218,8 @@ typedef struct PruneFreezeResult
 
 	/* New value of relminmxid found by heap_page_prune_and_freeze() */
 	MultiXactId new_relminmxid;
+	int			lpdead_items;	/* includes existing LP_DEAD items */
+	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
 } PruneFreezeResult;
 
 /* ----------------
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0018-Initialize-xl_heap_prune-deserialization-variable.patch"

From 89c1686279f59ac3e8536fcd6618291e3a22c199 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 19 Mar 2024 16:05:42 -0400
Subject: [PATCH v4 18/19] Initialize xl_heap_prune deserialization variables

Future commits will depend on these being initialized
---
 src/backend/access/heap/heapam.c       | 18 +++++++++---------
 src/backend/access/rmgrdesc/heapdesc.c | 16 ++++++++--------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16bab55ba02..2ef7decdd05 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8758,16 +8758,16 @@ heap_xlog_prune(XLogReaderState *record)
 	if (action == BLK_NEEDS_REDO)
 	{
 		Page		page = (Page) BufferGetPage(buffer);
-		OffsetNumber *redirected;
-		OffsetNumber *nowdead;
-		OffsetNumber *nowunused;
-		int			nredirected;
-		int			ndead;
-		int			nunused;
-		int			nplans;
+		OffsetNumber *redirected = NULL;
+		OffsetNumber *nowdead = NULL;
+		OffsetNumber *nowunused = NULL;
+		int			nredirected = 0;
+		int			ndead = 0;
+		int			nunused = 0;
+		int			nplans = 0;
 		Size		datalen;
-		xl_heap_freeze_plan *plans;
-		OffsetNumber *frz_offsets;
+		xl_heap_freeze_plan *plans = NULL;
+		OffsetNumber *frz_offsets = NULL;
 		int			curoff = 0;
 
 		nplans = xlrec->nplans;
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index ea03f902fc4..1fe5c78031f 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -185,15 +185,15 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
 
 		if (XLogRecHasBlockData(record, 0))
 		{
-			OffsetNumber *redirected;
-			OffsetNumber *nowdead;
-			OffsetNumber *nowunused;
-			int			nredirected;
-			int			ndead;
-			int			nunused;
-			int			nplans;
 			Size		datalen;
-			xl_heap_freeze_plan *plans;
+			OffsetNumber *redirected = NULL;
+			OffsetNumber *nowdead = NULL;
+			OffsetNumber *nowunused = NULL;
+			int			nredirected = 0;
+			int			nunused = 0;
+			int			ndead = 0;
+			int			nplans = 0;
+			xl_heap_freeze_plan *plans = NULL;
 			OffsetNumber *frz_offsets;
 
 			nplans = xlrec->nplans;
-- 
2.40.1


--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0019-Streamline-XLOG_HEAP2_PRUNE-record-and-use-for-fr.patch"

From 74411532903ec17cc9c29e17786fdd35ba1b0eac Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 19 Mar 2024 20:48:28 -0400
Subject: [PATCH v4 19/19] Streamline XLOG_HEAP2_PRUNE record and use for
 freeze and vacuum

xl_heap_prune struct for the XLOG_HEAP2_PRUNE record type had members
for counting the number of freeze plans and number of redirected, dead,
and newly unused line pointers. However, only some of those are used in
many XLOG_HEAP2_PRUNE records.

Put all of those members in the XLOG buffer data and use flags to
indicate which are present. This makes it feasible to use the
XLOG_HEAP2_PRUNE record smaller than it was and smaller than the
previously used XLOG_HEAP2_VACUUM and XLOG_HEAP2_FREEZE records.

The snapshot conflict horizon is not used for vacuum but is for the
other record types. It must go in the main data (not per buffer data) so
that it can be used even if the record contains an FPI.

The new prune record is composed of sub-records for each type of
modification freezing tuples and setting line pointers unused, redirect,
or dead.
---
 src/backend/access/heap/heapam.c       |  33 +++----
 src/backend/access/heap/pruneheap.c    |  98 ++++++++++++++-----
 src/backend/access/heap/vacuumlazy.c   |  24 +++--
 src/backend/access/rmgrdesc/heapdesc.c |  27 ++---
 src/backend/access/rmgrdesc/xactdesc.c |  68 +++++++++++++
 src/include/access/heapam_xlog.h       | 130 +++++++++++++++----------
 src/tools/pgindent/typedefs.list       |   3 +
 7 files changed, 273 insertions(+), 110 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 2ef7decdd05..adc259fdca7 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8728,15 +8728,15 @@ heap_xlog_prune(XLogReaderState *record)
 	 * heap_page_prune_and_freeze(), heap_page_prune_execute() will call
 	 * PageRepairFragementation() which expects a full cleanup lock.
 	 */
-	get_cleanup_lock = xlrec->nredirected > 0 ||
-		xlrec->ndead > 0 ||
-		(xlrec->nunused > 0 && !lp_truncate_only);
+	get_cleanup_lock = xlrec->flags & XLHP_HAS_REDIRECTIONS ||
+		xlrec->flags & XLHP_HAS_DEAD_ITEMS ||
+		(xlrec->flags & XLHP_HAS_NOW_UNUSED_ITEMS && !lp_truncate_only);
 
 	if (lp_truncate_only)
 	{
-		Assert(xlrec->nredirected == 0);
-		Assert(xlrec->ndead == 0);
-		Assert(xlrec->nunused > 0);
+		Assert(!(xlrec->flags & XLHP_HAS_REDIRECTIONS));
+		Assert(!(xlrec->flags & XLHP_HAS_DEAD_ITEMS));
+		Assert(xlrec->flags & XLHP_HAS_NOW_UNUSED_ITEMS);
 	}
 
 	/*
@@ -8745,9 +8745,13 @@ heap_xlog_prune(XLogReaderState *record)
 	 * tuples are still visible or which consider the frozen xids as running.
 	 */
 	if (xlrec->flags & XLHP_HAS_CONFLICT_HORIZON && InHotStandby)
-		ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
-											xlrec->isCatalogRel,
+	{
+		xlhp_conflict_horizon *horizon = (xlhp_conflict_horizon *) (xlrec + SizeOfHeapPrune);
+
+		ResolveRecoveryConflictWithSnapshot(horizon->xid,
+											xlrec->flags & XLHP_IS_CATALOG_REL,
 											rlocator);
+	}
 
 	/*
 	 * If we have a full-page image, restore it and we're done.
@@ -8770,16 +8774,11 @@ heap_xlog_prune(XLogReaderState *record)
 		OffsetNumber *frz_offsets = NULL;
 		int			curoff = 0;
 
-		nplans = xlrec->nplans;
-		nredirected = xlrec->nredirected;
-		ndead = xlrec->ndead;
-		nunused = xlrec->nunused;
+		char	   *cursor = XLogRecGetBlockData(record, 0, &datalen);
 
-		plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
-		redirected = (OffsetNumber *) &plans[nplans];
-		nowdead = redirected + (nredirected * 2);
-		nowunused = nowdead + ndead;
-		frz_offsets = nowunused + nunused;
+		heap_xlog_deserialize_prune_and_freeze(cursor, xlrec->flags,
+											   &nredirected, &redirected, &ndead, &nowdead,
+											   &nunused, &nowunused, &nplans, &plans, &frz_offsets);
 
 		/* Update all line pointers per the record, and repair fragmentation */
 		if (nredirected > 0 || ndead > 0 || nunused > 0)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 5decb1127d0..26147f63c4c 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -741,19 +741,22 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
 						  PruneState *prstate, PruneFreezeResult *presult)
 {
 	xl_heap_prune xlrec;
+	xlhp_conflict_horizon horizon;
 	XLogRecPtr	recptr;
+	xlhp_freeze freeze;
+	xlhp_prune_items redirect,
+				dead,
+				unused;
 
+	int			nplans = 0;
 	xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
-	OffsetNumber offsets[MaxHeapTuplesPerPage];
-	bool		do_freeze = presult->nfrozen > 0;
+	OffsetNumber frz_offsets[MaxHeapTuplesPerPage];
+	bool		do_freeze = (presult->nfrozen > 0);
 
 	xlrec.flags = 0;
 
-	xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
-	xlrec.nredirected = prstate->nredirected;
-	xlrec.ndead = prstate->ndead;
-	xlrec.nunused = prstate->nunused;
-	xlrec.nplans = 0;
+	if (RelationIsAccessibleInLogicalDecoding(relation))
+		xlrec.flags |= XLHP_IS_CATALOG_REL;
 
 	xlrec.flags |= XLHP_HAS_CONFLICT_HORIZON;
 
@@ -767,22 +770,25 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
 	 * youngest tuple this record will freeze will conflict.
 	 */
 	if (do_freeze)
-		xlrec.snapshotConflictHorizon = Max(prstate->snapshotConflictHorizon,
-											presult->frz_conflict_horizon);
+		horizon.xid = Max(prstate->snapshotConflictHorizon,
+						  presult->frz_conflict_horizon);
 	else
-		xlrec.snapshotConflictHorizon = prstate->snapshotConflictHorizon;
+		horizon.xid = prstate->snapshotConflictHorizon;
 
 	/*
 	 * Prepare deduplicated representation for use in WAL record Destructively
 	 * sorts tuples array in-place.
 	 */
 	if (do_freeze)
-		xlrec.nplans = heap_log_freeze_plan(prstate->frozen,
-											presult->nfrozen, plans, offsets);
-
+		nplans = heap_log_freeze_plan(prstate->frozen,
+									  presult->nfrozen, plans,
+									  frz_offsets);
 	XLogBeginInsert();
 	XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
 
+	xlrec.flags |= XLHP_HAS_CONFLICT_HORIZON;
+	XLogRegisterData((char *) &horizon, SizeOfSnapshotConflictHorizon);
+
 	XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
 
 	/*
@@ -790,25 +796,73 @@ log_heap_prune_and_freeze(Relation relation, Buffer buffer,
 	 * that they are.  When XLogInsert stores the whole buffer, the offset
 	 * arrays need not be stored too.
 	 */
-	if (xlrec.nplans > 0)
+	if (nplans > 0)
+	{
+		xlrec.flags |= XLHP_HAS_FREEZE_PLANS;
+
+		freeze = (xlhp_freeze)
+		{
+			.nplans = nplans
+		};
+
+		XLogRegisterBufData(0, (char *) &freeze, offsetof(xlhp_freeze, plans));
+
 		XLogRegisterBufData(0, (char *) plans,
-							xlrec.nplans * sizeof(xl_heap_freeze_plan));
+							sizeof(xl_heap_freeze_plan) * freeze.nplans);
+	}
+
 
 	if (prstate->nredirected > 0)
+	{
+		xlrec.flags |= XLHP_HAS_REDIRECTIONS;
+
+		redirect = (xlhp_prune_items)
+		{
+			.ntargets = prstate->nredirected
+		};
+
+		XLogRegisterBufData(0, (char *) &redirect,
+							offsetof(xlhp_prune_items, data));
+
 		XLogRegisterBufData(0, (char *) prstate->redirected,
-							prstate->nredirected *
-							sizeof(OffsetNumber) * 2);
+							sizeof(OffsetNumber[2]) * prstate->nredirected);
+	}
 
 	if (prstate->ndead > 0)
+	{
+		xlrec.flags |= XLHP_HAS_DEAD_ITEMS;
+
+		dead = (xlhp_prune_items)
+		{
+			.ntargets = prstate->ndead
+		};
+
+		XLogRegisterBufData(0, (char *) &dead,
+							offsetof(xlhp_prune_items, data));
+
 		XLogRegisterBufData(0, (char *) prstate->nowdead,
-							prstate->ndead * sizeof(OffsetNumber));
+							sizeof(OffsetNumber) * dead.ntargets);
+	}
 
 	if (prstate->nunused > 0)
+	{
+		xlrec.flags |= XLHP_HAS_NOW_UNUSED_ITEMS;
+
+		unused = (xlhp_prune_items)
+		{
+			.ntargets = prstate->nunused
+		};
+
+		XLogRegisterBufData(0, (char *) &unused,
+							offsetof(xlhp_prune_items, data));
+
 		XLogRegisterBufData(0, (char *) prstate->nowunused,
-							prstate->nunused * sizeof(OffsetNumber));
-	if (xlrec.nplans > 0)
-		XLogRegisterBufData(0, (char *) offsets,
-							presult->nfrozen * sizeof(OffsetNumber));
+							sizeof(OffsetNumber) * unused.ntargets);
+	}
+
+	if (nplans > 0)
+		XLogRegisterBufData(0, (char *) frz_offsets,
+							sizeof(OffsetNumber) * presult->nfrozen);
 
 	recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
 
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 295846b854f..1e79cbbb107 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2253,21 +2253,27 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 	if (RelationNeedsWAL(vacrel->rel))
 	{
 		xl_heap_prune xlrec;
+		xlhp_prune_items unused_rec;
 		XLogRecPtr	recptr;
 
-		xlrec.flags = XLHP_LP_TRUNCATE_ONLY;
-		xlrec.snapshotConflictHorizon = InvalidTransactionId;
-		xlrec.nplans = 0;
-		xlrec.nredirected = 0;
-		xlrec.ndead = 0;
-		xlrec.nunused = nunused;
-		xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(vacrel->rel);
+		xlrec.flags = XLHP_HAS_NOW_UNUSED_ITEMS;
+
+		xlrec.flags |= XLHP_LP_TRUNCATE_ONLY;
+
+		unused_rec = (xlhp_prune_items)
+		{
+			.ntargets = nunused
+		};
 
 		XLogBeginInsert();
 		XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
-
 		XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
-		XLogRegisterBufData(0, (char *) unused, nunused * sizeof(OffsetNumber));
+
+		XLogRegisterBufData(0, (char *) &unused_rec,
+							offsetof(xlhp_prune_items, data));
+
+		XLogRegisterBufData(0, (char *) unused,
+							sizeof(OffsetNumber) * unused_rec.ntargets);
 
 		recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
 
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 1fe5c78031f..f542bcb94b6 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -179,9 +179,16 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
 	{
 		xl_heap_prune *xlrec = (xl_heap_prune *) rec;
 
-		appendStringInfo(buf, "snapshotConflictHorizon: %u, isCatalogRel: %c",
-						 xlrec->snapshotConflictHorizon,
-						 xlrec->isCatalogRel ? 'T' : 'F');
+		if (xlrec->flags & XLHP_HAS_CONFLICT_HORIZON)
+		{
+			xlhp_conflict_horizon *horizon = (xlhp_conflict_horizon *) (xlrec + SizeOfHeapPrune);
+
+			appendStringInfo(buf, "snapshotConflictHorizon: %u",
+							 horizon->xid);
+		}
+
+		appendStringInfo(buf, ", isCatalogRel: %c",
+						 xlrec->flags & XLHP_IS_CATALOG_REL ? 'T' : 'F');
 
 		if (XLogRecHasBlockData(record, 0))
 		{
@@ -196,16 +203,12 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
 			xl_heap_freeze_plan *plans = NULL;
 			OffsetNumber *frz_offsets;
 
-			nplans = xlrec->nplans;
-			nredirected = xlrec->nredirected;
-			ndead = xlrec->ndead;
-			nunused = xlrec->nunused;
+			char	   *cursor = XLogRecGetBlockData(record, 0, &datalen);
+
+			heap_xlog_deserialize_prune_and_freeze(cursor, xlrec->flags,
+												   &nredirected, &redirected, &ndead, &nowdead,
+												   &nunused, &nowunused, &nplans, &plans, &frz_offsets);
 
-			plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
-			redirected = (OffsetNumber *) &plans[nplans];
-			nowdead = redirected + (nredirected * 2);
-			nowunused = nowdead + ndead;
-			frz_offsets = nowunused + nunused;
 
 			appendStringInfo(buf, ", nredirected: %u, ndead: %u, nunused: %u, nplans: %u,",
 							 nredirected,
diff --git a/src/backend/access/rmgrdesc/xactdesc.c b/src/backend/access/rmgrdesc/xactdesc.c
index 41b842d80ec..e120805e5e0 100644
--- a/src/backend/access/rmgrdesc/xactdesc.c
+++ b/src/backend/access/rmgrdesc/xactdesc.c
@@ -14,6 +14,7 @@
  */
 #include "postgres.h"
 
+#include "access/heapam_xlog.h"
 #include "access/transam.h"
 #include "access/xact.h"
 #include "replication/origin.h"
@@ -21,6 +22,73 @@
 #include "storage/standbydefs.h"
 #include "utils/timestamp.h"
 
+/*
+ * Given a MAXALIGNed buffer returned by XLogRecGetBlockData() and pointed to
+ * by cursor and any xl_heap_prune flags, deserialize the arrays of
+ * OffsetNumbers contained in an xl_heap_prune record. This is in this file so
+ * it can be shared between heap2_redo and heap2_desc code, the latter of which
+ * is used in frontend code.
+ */
+void
+heap_xlog_deserialize_prune_and_freeze(char *cursor, uint8 flags,
+									   int *nredirected, OffsetNumber **redirected,
+									   int *ndead, OffsetNumber **nowdead,
+									   int *nunused, OffsetNumber **nowunused,
+									   int *nplans, xl_heap_freeze_plan **plans,
+									   OffsetNumber **frz_offsets)
+{
+	if (flags & XLHP_HAS_FREEZE_PLANS)
+	{
+		xlhp_freeze *freeze = (xlhp_freeze *) cursor;
+
+		*nplans = freeze->nplans;
+		Assert(*nplans > 0);
+		*plans = freeze->plans;
+
+		cursor += offsetof(xlhp_freeze, plans);
+		cursor += sizeof(xl_heap_freeze_plan) * freeze->nplans;
+	}
+
+	if (flags & XLHP_HAS_REDIRECTIONS)
+	{
+		xlhp_prune_items *subrecord = (xlhp_prune_items *) cursor;
+
+		*nredirected = subrecord->ntargets;
+		Assert(nredirected > 0);
+		*redirected = &subrecord->data[0];
+
+		cursor += offsetof(xlhp_prune_items, data);
+		cursor += sizeof(OffsetNumber[2]) * *nredirected;
+	}
+
+	if (flags & XLHP_HAS_DEAD_ITEMS)
+	{
+		xlhp_prune_items *subrecord = (xlhp_prune_items *) cursor;
+
+		*ndead = subrecord->ntargets;
+		Assert(ndead > 0);
+		*nowdead = subrecord->data;
+
+		cursor += offsetof(xlhp_prune_items, data);
+		cursor += sizeof(OffsetNumber) * *ndead;
+	}
+
+	if (flags & XLHP_HAS_NOW_UNUSED_ITEMS)
+	{
+		xlhp_prune_items *subrecord = (xlhp_prune_items *) cursor;
+
+		*nunused = subrecord->ntargets;
+		Assert(nunused > 0);
+		*nowunused = subrecord->data;
+
+		cursor += offsetof(xlhp_prune_items, data);
+		cursor += sizeof(OffsetNumber) * *nunused;
+	}
+
+	if (nplans > 0)
+		*frz_offsets = (OffsetNumber *) cursor;
+}
+
 /*
  * Parse the WAL format of an xact commit and abort records into an easier to
  * understand format.
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 2393540cf68..dfeb703d136 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -224,9 +224,64 @@ typedef struct xl_heap_update
 
 #define SizeOfHeapUpdate	(offsetof(xl_heap_update, new_offnum) + sizeof(OffsetNumber))
 
+/*
+ * This is what we need to know about page pruning and freezing, both during
+ * VACUUM and during opportunistic pruning.
+ *
+ * If XLPH_HAS_REDIRECTIONS, XLHP_HAS_DEAD_ITEMS, or XLHP_HAS_NOW_UNUSED is set,
+ * acquires a full cleanup lock. Otherwise an ordinary exclusive lock is
+ * enough. This can happen if freezing was the only modification to the page.
+ *
+ * The data for block reference 0 contains "sub-records" depending on which
+ * of the XLHP_HAS_* flags are set. See xlhp_* struct definitions below.
+ * The layout is in the same order as the XLHP_* flags.
+ *
+ * OFFSET NUMBERS are in the block reference 0
+ *
+ * If only unused item offsets are included because the record is constructed
+ * during vacuum's second pass (marking LP_DEAD items LP_UNUSED) then only an
+ * ordinary exclusive lock is required to replay.
+ */
+typedef struct xl_heap_prune
+{
+	uint8		flags;
+} xl_heap_prune;
+
+/* to handle recovery conflict during logical decoding on standby */
+#define		XLHP_IS_CATALOG_REL			(1 << 1)
+
+/*
+ * During vacuum's second pass which sets LP_DEAD items LP_UNUSED, we will only
+ * truncate the line pointer array, not call PageRepairFragmentation. We need
+ * this flag to differentiate what kind of lock (exclusive or cleanup) to take
+ * on the buffer and whether to call PageTruncateLinePointerArray() or
+ * PageRepairFragementation().
+ */
+#define		XLHP_LP_TRUNCATE_ONLY       (1 << 2)
+
+/*
+ * Vacuum's first pass and on-access pruning may need to include a snapshot
+ * conflict horizon.
+ */
+#define		XLHP_HAS_CONFLICT_HORIZON   (1 << 3)
+#define		XLHP_HAS_FREEZE_PLANS		(1 << 4)
+#define		XLHP_HAS_REDIRECTIONS		(1 << 5)
+#define		XLHP_HAS_DEAD_ITEMS	        (1 << 6)
+#define		XLHP_HAS_NOW_UNUSED_ITEMS   (1 << 7)
+
+#define SizeOfHeapPrune (offsetof(xl_heap_prune, flags) + sizeof(uint8))
+
+typedef struct xlhp_conflict_horizon
+{
+	TransactionId xid;
+} xlhp_conflict_horizon;
+
+#define SizeOfSnapshotConflictHorizon (offsetof(xlhp_conflict_horizon, xid) + sizeof(uint32))
+
 /*
  * This struct represents a 'freeze plan', which describes how to freeze a
- * group of one or more heap tuples (appears in xl_heap_prune record)
+ * group of one or more heap tuples (appears in xl_heap_prune's xlhp_freeze
+ * record)
  */
 /* 0x01 was XLH_FREEZE_XMIN */
 #define		XLH_FREEZE_XVAC		0x02
@@ -246,64 +301,32 @@ typedef struct xl_heap_freeze_plan
 /*
  * As of Postgres 17, XLOG_HEAP2_PRUNE records replace
  * XLOG_HEAP2_FREEZE_PAGE records.
- */
-
-/*
- * This is what we need to know about page pruning (both during VACUUM and
- * during opportunistic pruning)
  *
- * The array of OffsetNumbers following the fixed part of the record contains:
- *	* for each freeze plan: the freeze plan
- *	* for each redirected item: the item offset, then the offset redirected to
- *	* for each now-dead item: the item offset
- *	* for each now-unused item: the item offset
- *	* for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
- * The total number of OffsetNumbers is therefore
- * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
+ * This is what we need to know about a block being frozen during vacuum
  *
- * Acquires a full cleanup lock if heap_page_prune_execute() must be called
+ * Backup block 0's data contains an array of xl_heap_freeze_plan structs
+ * (with nplans elements), followed by one or more page offset number arrays.
+ * Each such page offset number array corresponds to a single freeze plan
+ * (REDO routine freezes corresponding heap tuples using freeze plan).
  */
-typedef struct xl_heap_prune
+typedef struct xlhp_freeze
 {
-	uint8		flags;
-	TransactionId snapshotConflictHorizon;
 	uint16		nplans;
-	uint16		nredirected;
-	uint16		ndead;
-	uint16		nunused;
-	bool		isCatalogRel;	/* to handle recovery conflict during logical
-								 * decoding on standby */
-	/*--------------------------------------------------------------------
-	 * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
-	 * following order:
-	 *
-	 *		* xl_heap_freeze_plan plans[nplans];
-	 * 		* OffsetNumber redirected[2 * nredirected];
-	 * 		* OffsetNumber nowdead[ndead];
-	 *		* OffsetNumber nowunused[nunused];
-	 * 		* OffsetNumber frz_offsets[...];
-	 *--------------------------------------------------------------------
-	 */
-} xl_heap_prune;
-
-#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
-
-/* Flags for xl_heap_prune */
+	xl_heap_freeze_plan plans[FLEXIBLE_ARRAY_MEMBER];
+} xlhp_freeze;
 
 /*
- * During vacuum's second pass which sets LP_DEAD items LP_UNUSED, we will only
- * truncate the line pointer array, not call PageRepairFragmentation. We need
- * this flag to differentiate what kind of lock (exclusive or cleanup) to take
- * on the buffer and whether to call PageTruncateLinePointerArray() or
- * PageRepairFragementation().
+ * Sub-record type contained in block reference 0 of a prune record if
+ * XLHP_HAS_REDIRECTIONS/XLHP_HAS_DEAD_ITEMS/XLHP_HAS_NOW_UNUSED_ITEMS is set.
+ * Note that in the XLHP_HAS_REDIRECTIONS variant, there are actually 2 *
+ * length number of OffsetNumbers in the data.
  */
-#define		XLHP_LP_TRUNCATE_ONLY       (1 << 1)
+typedef struct xlhp_prune_items
+{
+	uint16		ntargets;
+	OffsetNumber data[FLEXIBLE_ARRAY_MEMBER];
+} xlhp_prune_items;
 
-/*
- * Vacuum's first pass and on-access pruning may need to include a snapshot
- * conflict horizon.
- */
-#define		XLHP_HAS_CONFLICT_HORIZON   (1 << 2)
 
 /* flags for infobits_set */
 #define XLHL_XMAX_IS_MULTI		0x01
@@ -416,4 +439,11 @@ extern XLogRecPtr log_heap_visible(Relation rel, Buffer heap_buffer,
 								   TransactionId snapshotConflictHorizon,
 								   uint8 vmflags);
 
+extern void heap_xlog_deserialize_prune_and_freeze(char *cursor, uint8 flags,
+												   int *nredirected, OffsetNumber **redirected,
+												   int *ndead, OffsetNumber **nowdead,
+												   int *nunused, OffsetNumber **nowunused,
+												   int *nplans, xl_heap_freeze_plan **plans,
+												   OffsetNumber **frz_offsets);
+
 #endif							/* HEAPAM_XLOG_H */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index b2ddc1e2549..40fb694c836 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -4007,6 +4007,9 @@ xl_xact_stats_items
 xl_xact_subxacts
 xl_xact_twophase
 xl_xact_xinfo
+xlhp_conflict_horizon
+xlhp_freeze
+xlhp_prune_items
 xmlBuffer
 xmlBufferPtr
 xmlChar
-- 
2.40.1


--tez7m2a73jtztiij--