Re: Old row version in hot chain become visible after a freeze

Michael Paquier wrote:

> I have also spent a couple more hours looking at the proposed patch
> and eye-balling the surrounding code, and my suggestion about
> heap_tuple_needs_freeze() is proving to be wrong. So I am arriving at
> the conclusion that your patch is taking the right approach to skip
> freezing completely if the tuple is not to be removed yet if it is for
> vacuum either DEAD or RECENTLY_DEAD.

I think in the "tupkeep" case we must not mark the page as frozen in VM;
in other words I think that block needs to look like this:

// tupgone = false
{
bool tuple_totally_frozen;

num_tuples += 1;
hastup = true;

/*
* Each non-removable tuple that we do not keep must be checked
* to see if it needs freezing. Note we already have exclusive
* buffer lock.
*/
if (!tupkeep &&
heap_prepare_freeze_tuple(tuple.t_data, FreezeLimit,
MultiXactCutoff,
&frozen[nfrozen],
&tuple_totally_frozen))
frozen[nfrozen++].offset = offnum;

if (tupkeep || !tuple_totally_frozen)
all_frozen = false;
}

Otherwise, we risk marking the page as all-frozen, and it would be
skipped by vacuum. If we never come around to HOT-pruning the page, a
non-permanent xid (or a multixact? not sure that that can happen;
probably not) would linger unnoticed and cause a DoS condition later
("cannot open file pg_clog/1234") when the tuple header is read.

Now, it is possible that HOT pruning would fix the page promptly without
causing an actual DoS, but nonetheless it seems dangerous to leave
things like this.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services