Re: Correcting freeze conflict horizon calculation

On Mon, Mar 9, 2026 at 4:15 PM Peter Geoghegan <pg@bowt.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