Re: Correcting freeze conflict horizon calculation

On Mon, Jun 2, 2025 at 3:05 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Mon, Jun 2, 2025 at 2:50 PM Melanie Plageman
> <melanieplageman(at)gmail(dot)com> wrote:
> > As you've explained, it will always be <= OldestXmin. And, if the
> > record only freezes tuples (meaning it makes no other changes to the
> > page) and all of those tuples' xmins were considered when calculating
> > prunestate->visibility_cutoff_xid, then, even if the page isn't
> > all-frozen, how could it be incorrect to use the
> > prunestate->visibility_cutoff_xid as the horizon?
>
> Obviously, it isn't safe to use prunestate->visibility_cutoff_xid
> unless it has actually been maintained using all of the tuples on the
> page. It is primarily intended for use by the VM record (though it
> doesn't have to continue to work that way, of course).

Oh, and, more specifically, in my previous email, I was wondering if,
and why, in 16 this diff wouldn't be correct

diff --git a/src/backend/access/heap/vacuumlazy.c
b/src/backend/access/heap/vacuumlazy.c
index 9fa88960ada..f1831bba95c 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1833,7 +1833,7 @@ retry:
* once we're done with it. Otherwise we generate a conservative
* cutoff by stepping back from OldestXmin.
*/
- if (prunestate->all_visible && prunestate->all_frozen)
+ if (prunestate->all_visible)
{
/* Using same cutoff when setting VM is now unnecessary */
snapshotConflictHorizon = prunestate->visibility_cutoff_xid;

- Melanie