Re: Reviewing freeze map code

On Sat, Jun 11, 2016 at 1:24 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> 3. vacuumlazy.c includes this code:
>
> if (heap_prepare_freeze_tuple(tuple.t_data, FreezeLimit,
> MultiXactCutoff,
&frozen[nfrozen]))
> frozen[nfrozen++].offset = offnum;
> else if (heap_tuple_needs_eventual_freeze(tuple.t_data))
> all_frozen = false;
>
> That's wrong, because a "true" return value from
> heap_prepare_freeze_tuple() means only that it has done *some*
> freezing work on the tuple, not that it's done all of the freezing
> work that will ever need to be done. So, if the tuple's xmin can be
> frozen and is aborted but not older than vacuum_freeze_min_age, then
> heap_prepare_freeze_tuple() won't free xmax, but the page will still
> be marked all-frozen, which is bad.
>

To clarify, are you talking about a case where insertion has aborted?
Won't in such a case all_visible flag be set to false due to return value
from HeapTupleSatisfiesVacuum() and if so, later code shouldn't mark it as
all_frozen?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com