Re: Correcting freeze conflict horizon calculation

On Mon, Mar 9, 2026 at 4:15 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> I'm not sure that you need to add any new comments above
> FreezeMultiXactId. The underlying principles that justify ignoring
> xmax when it is a multi are exactly the same as those that apply when
> xmax is a normal XID. I think that what you actually need is a single
> comment block (maybe 2) near the start of or above
> heap_prepare_freeze_tuple explaining your new snapshotConflictHorizon
> maintenance code, mentioning:
>
[...snip...]
>
> I'm trying not to be too prescriptive here; I just think that
> emphasizing high-level logical database concepts over physical
> database implementation details makes sense. I don't expect you to
> follow what I've written here all too closely. I didn't have the time
> to distill it down myself.

I've taken a stab at distilling down the comment and put it above
heap_prepare_freeze_tuple(). This is what I came up with:

* FreezePageConflictXid is advanced only for xmin/xvac freezing, not for xmax
* changes. We only remove xmax state here when it is lock-only, or when the
* updater XID (including an updater member of a MultiXact) must be aborted;
* otherwise, the tuple would already be removable. Neither case affects
* visibility on a standby.

I don't mention why we need a conflict horizon when freezing there,
but I do in the comment above the struct member:

/*
* Newest XID that this page's freeze actions will remove from tuple
* visibility metadata (currently xmin and/or xvac). It is used to derive
* the snapshot conflict horizon for a WAL record that freezes tuples. On
* a standby, we must not replay that change while any snapshot could
* still treat that XID as running.
*
* It's only used if we execute freeze plans for this page, so there is no
* corresponding "no freeze" tracker.
*/
TransactionId FreezePageConflictXid;

Do these seem correct enough and understandable?

- Melanie