Re: Correcting freeze conflict horizon calculation

Thanks for taking a look!

On Tue, Mar 3, 2026 at 9:28 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Mon, Mar 2, 2026 at 12:15 PM Melanie Plageman
> <melanieplageman(at)gmail(dot)com> 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