Thanks for taking a look!
On Tue, Mar 3, 2026 at 9:28 PM Peter Geoghegan <[email protected]> wrote:
>
> On Mon, Mar 2, 2026 at 12:15 PM Melanie Plageman
> <[email protected]> wrote:
>
> > 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?
>
> I don't quite follow. Are you talking about xmin or xmax? The xmax in
> question must come from the original tuple, not the updated successor
> version? (The successor is the tuple that pruning must have already
> removed by the time we reach heap_prepare_freeze_tuple, preventing
> heap_prepare_freeze_tuple from seeing an aborted xmin.)
I'm talking about xmax.
> The important principle here is that we don't need a recovery conflict
> to handle cleanup after an aborted update/delete, regardless of the
> details. This is a logical consequence of the fact that an aborted
> transaction "never existed in the logical database".
> > Or are there other ways you can have an
> > xmax older than OldestXmin?
>
> Again, are you talking about xmin or xmax? It's normal for
> heap_prepare_freeze_tuple to see an xmax older than OldestXmin, last I
> checked.
Based on the following code in heap_prepare_freeze_tuple(), a normal
xmax that is older than OldestXmin is assumed to be an aborted
transaction -- which, as you say, does not need to affect recovery
conflict at all.
else if (TransactionIdIsNormal(xid))
{
/* Raw xmax is normal XID */
freeze_xmax = TransactionIdPrecedes(xid, cutoffs->OldestXmin);
/*
* Verify that xmax aborted if and when freeze plan is executed,
* provided it's from an update. (A lock-only xmax can be removed
* 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;
> Don't forget about plain XIDs that end up as xmax due to a SELECT FOR
> UPDATE. They usually don't result from aborted transactions.
I assume that in the SELECT FOR UPDATE case, HEAP_XMAX_IS_LOCKED_ONLY
would return true -- so this is a case where lockers don't affect the
horizon (even though it is a normal xid and not a multi).
I am trying to determine if I need to advance FreezePageConflictXid in
the above case when freeze_xmax is true. So far, if the only xmaxes
older than OldestXmin are from aborted update/deletes or SELECT FOR
UPDATE, then it seems like I wouldn't need to advance the horizon when
freeze_xmax is true.
In attached v2, I've removed the code which may advance
FreezePageConflictXid in the above case.
I wondering if I should keep the comment I added above
FreezeMultiXactId() explaining why I don't update
FreezePageConflictXid there.
> > 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.
>
> But neither should the XIDs of updaters that aborted. I don't think
> you should handle those at all.
Cool. I now ignore those for FreezePageConflictXid.
> I think that as a general rule VACUUM should never generate a
> snapshotConflictHorizon that exactly equals OldestXmin (or a later
> one). See commit 66fbcb0d.
>
> I notice that you have an assertion
> prstate->pagefrz.FreezePageConflictXid <= OldestXmin. I wonder if you
> should strengthen the assertion to
> prstate->pagefrz.FreezePageConflictXid < OldestXmin?
I did that in v2.
> I also wonder if
> the existing assertion can fail due to an aborted update leaving an
> xmax > OldestXmin.
I think that can't happen now because I do not ever advance
FreezePageConflictXid for xmaxes.
- Melanie
From b3decaf08a27b4dec1e9c08d611a7d321caa0632 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 2 Mar 2026 11:39:28 -0500
Subject: [PATCH v2] 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.
Author: Melanie Plageman <[email protected]>
Reviewed-by: Peter Geoghegan <[email protected]>
Discussion: https://postgr.es/m/CAAKRu_bbaUV8OUjAfVa_iALgKnTSfB4gO3jnkfpcFgrxEpSGJQ%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 12 +++++++++
src/backend/access/heap/pruneheap.c | 41 +++++++----------------------
src/include/access/heapam.h | 8 ++++++
3 files changed, 30 insertions(+), 31 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a231563f0df..649ee6e7669 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 */
}
@@ -7501,6 +7512,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..a4728423ba0 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(TransactionIdPrecedes(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
