On Tue, Jun 3, 2025 at 2:00 PM Peter Geoghegan <[email protected]> wrote:
>
> BTW, I don't think that you have to worry about removing/freezing xmax
> when it comes to generating a safe snapshotConflictHorizon.
>
> Nothing makes it unsafe for VACUUM to remove an xmax set by an updater
> that is known to have aborted right away -- no matter how that xmax
> compares to OldestXmin. Pruning has always been able to remove the
> successor version right away, no matter how recently the abort
> happened, so why shouldn't we *also* be able to set the xmax in the
> original version to InvalidTransactionId? That doesn't need to be
> taken into account by snapshotConflictHorizon handling. (Note that
> HeapTupleHeaderAdvanceConflictHorizon() is specifically aware that an
> aborted tuple's xmin shouldn't need to affect a prune record's
> conflict horizon; an aborted update can also skip consideration by
> snapshotConflictHorizon maintenance.)
>
> I also think that a locker-only xmax can be removed/frozen/set to
> InvalidTransactionId, as long as the tuple lock is no longer held on
> the primary -- it can safely be set to InvalidTransactionId, without
> it needing to affect snapshotConflictHorizon tracking at all. In a
> way, we already do this with Multis.
>
> It's already possible (though not particularly common) for VACUUM to
> fully set a Multi xmax to InvalidTransactionId when the Multi is >
> OldestMxact -- it can even happen when most of the individual members
> are still > OldestXmin. As long as the operation cannot affect tuple
> visibility on standbys, no special snapshotConflictHorizon is
> required. We do currently end up using OldestXmin-1 as our
> snapshotConflictHorizon when this happens, but as I said in the other
> email, I don't think that that's really required.

I wrote the attached patch to keep track of the newest to-be-frozen
xid. I think the freeze_xmin and replace_xvac cases are right. But I'm
not sure about the freeze_xmax and replace_xmax for multixacts cases.

For the freeze_xmax case for regular transaction IDs, are you saying
that the only way you can have one older than OldestXmin is if the
update transaction aborted? Or are there other ways you can have an
xmax older than OldestXmin?

My patch does not consider any multi member xids when calculating the
newest to-be-frozen xid. Locker-only xmaxes shouldn't affect
visibility on the standby. And I think, partially based on what you
are saying above and partially from reading the code, that update XIDs
older than OldestXmin don't matter because they would be from aborted
transactions and XIDs newer than OldestXmin won't be removed or
frozen. Does this sound right?

- Melanie
From 76fd95670c487ed089bd9eadecd1b84882020652 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 2 Mar 2026 11:39:28 -0500
Subject: [PATCH v1] Use the newest to-be-frozen xid as the conflict horizon
 for freezing

Previously WAL records that froze tuples used OldestXmin as the snapshot
conflict horizon. However, OldestXmin is newer than the newest frozen
tuple's xid. By tracking the newest to-be-frozen xid and using it as the
snapshot conflict horizon instead, we end up with an older horizon that
will result in fewer query cancellations on the standby.
---
 src/backend/access/heap/heapam.c    | 16 +++++++++++
 src/backend/access/heap/pruneheap.c | 41 +++++++----------------------
 src/include/access/heapam.h         |  8 ++++++
 3 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a231563f0df..0f3d673ad43 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6781,6 +6781,10 @@ heap_inplace_unlock(Relation relation,
  * NB: Caller should avoid needlessly calling heap_tuple_should_freeze when we
  * have already forced page-level freezing, since that might incur the same
  * SLRU buffer misses that we specifically intended to avoid by freezing.
+ *
+ * We won't update the FreezePageConflictXid because any lockers don't affect
+ * visibility on the standby, and we don't ahve to worry about the update XID
+ * since the only way it can be older than OldestXmin is if it is aborted.
  */
 static TransactionId
 FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
@@ -7173,7 +7177,11 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 
 		/* Verify that xmin committed if and when freeze plan is executed */
 		if (freeze_xmin)
+		{
 			frz->checkflags |= HEAP_FREEZE_CHECK_XMIN_COMMITTED;
+			if (TransactionIdFollows(xid, pagefrz->FreezePageConflictXid))
+				pagefrz->FreezePageConflictXid = xid;
+		}
 	}
 
 	/*
@@ -7192,6 +7200,9 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 		 */
 		replace_xvac = pagefrz->freeze_required = true;
 
+		if (TransactionIdFollows(xid, pagefrz->FreezePageConflictXid))
+			pagefrz->FreezePageConflictXid = xid;
+
 		/* Will set replace_xvac flags in freeze plan below */
 	}
 
@@ -7316,7 +7327,11 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 		 * independent of this, since the lock is released at xact end.)
 		 */
 		if (freeze_xmax && !HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask))
+		{
 			frz->checkflags |= HEAP_FREEZE_CHECK_XMAX_ABORTED;
+			if (TransactionIdFollows(xid, pagefrz->FreezePageConflictXid))
+				pagefrz->FreezePageConflictXid = xid;
+		}
 	}
 	else if (!TransactionIdIsValid(xid))
 	{
@@ -7501,6 +7516,7 @@ heap_freeze_tuple(HeapTupleHeader tuple,
 	pagefrz.freeze_required = true;
 	pagefrz.FreezePageRelfrozenXid = FreezeLimit;
 	pagefrz.FreezePageRelminMxid = MultiXactCutoff;
+	pagefrz.FreezePageConflictXid = InvalidTransactionId;
 	pagefrz.NoFreezePageRelfrozenXid = FreezeLimit;
 	pagefrz.NoFreezePageRelminMxid = MultiXactCutoff;
 
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 632c2427952..852c537897d 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -129,13 +129,6 @@ typedef struct
 	int			lpdead_items;	/* number of items in the array */
 	OffsetNumber *deadoffsets;	/* points directly to presult->deadoffsets */
 
-	/*
-	 * The snapshot conflict horizon used when freezing tuples. The final
-	 * snapshot conflict horizon for the record may be newer if pruning
-	 * removes newer transaction IDs.
-	 */
-	TransactionId frz_conflict_horizon;
-
 	/*
 	 * all_visible and all_frozen indicate if the all-visible and all-frozen
 	 * bits in the visibility map can be set for this page after pruning.
@@ -363,6 +356,7 @@ prune_freeze_setup(PruneFreezeParams *params,
 
 	/* initialize page freezing working state */
 	prstate->pagefrz.freeze_required = false;
+	prstate->pagefrz.FreezePageConflictXid = InvalidTransactionId;
 	if (prstate->attempt_freeze)
 	{
 		Assert(new_relfrozen_xid && new_relmin_mxid);
@@ -393,7 +387,6 @@ prune_freeze_setup(PruneFreezeParams *params,
 	 * PruneState.
 	 */
 	prstate->deadoffsets = presult->deadoffsets;
-	prstate->frz_conflict_horizon = InvalidTransactionId;
 
 	/*
 	 * Vacuum may update the VM after we're done.  We can keep track of
@@ -734,22 +727,8 @@ heap_page_will_freeze(Relation relation, Buffer buffer,
 		 * critical section.
 		 */
 		heap_pre_freeze_checks(buffer, prstate->frozen, prstate->nfrozen);
-
-		/*
-		 * Calculate what the snapshot conflict horizon should be for a record
-		 * freezing tuples. We can use the visibility_cutoff_xid as our cutoff
-		 * for conflicts when the whole page is eligible to become all-frozen
-		 * in the VM once we're done with it. Otherwise, we generate a
-		 * conservative cutoff by stepping back from OldestXmin.
-		 */
-		if (prstate->all_frozen)
-			prstate->frz_conflict_horizon = prstate->visibility_cutoff_xid;
-		else
-		{
-			/* Avoids false conflicts when hot_standby_feedback in use */
-			prstate->frz_conflict_horizon = prstate->cutoffs->OldestXmin;
-			TransactionIdRetreat(prstate->frz_conflict_horizon);
-		}
+		Assert(TransactionIdPrecedesOrEquals(prstate->pagefrz.FreezePageConflictXid,
+											 prstate->cutoffs->OldestXmin));
 	}
 	else if (prstate->nfrozen > 0)
 	{
@@ -944,17 +923,17 @@ heap_page_prune_and_freeze(PruneFreezeParams *params,
 			 * 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.
+			 * queries on the standby older than the youngest xid of the most
+			 * recently removed tuple this record will prune will conflict. If
+			 * this record will freeze tuples, any queries on the standby with
+			 * xids older than the youngest tuple this record will freeze will
+			 * conflict.
 			 */
 			TransactionId conflict_xid;
 
-			if (TransactionIdFollows(prstate.frz_conflict_horizon,
+			if (TransactionIdFollows(prstate.pagefrz.FreezePageConflictXid,
 									 prstate.latest_xid_removed))
-				conflict_xid = prstate.frz_conflict_horizon;
+				conflict_xid = prstate.pagefrz.FreezePageConflictXid;
 			else
 				conflict_xid = prstate.latest_xid_removed;
 
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 3c0961ab36b..fae79b37f0d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -208,6 +208,14 @@ typedef struct HeapPageFreeze
 	TransactionId FreezePageRelfrozenXid;
 	MultiXactId FreezePageRelminMxid;
 
+	/*
+	 * The youngest XID that will be frozen or removed during freezing. It is
+	 * used to calculate the snapshot conflict horizon for a WAL record
+	 * freezing tuples. Because it is only used if we do end up freezing
+	 * tuples, there is no need for a "no freeze" version.
+	 */
+	TransactionId FreezePageConflictXid;
+
 	/*
 	 * "No freeze" NewRelfrozenXid/NewRelminMxid trackers.
 	 *
-- 
2.43.0

Reply via email to