Re: Reviewing freeze map code

On Sat, Jun 11, 2016 at 1:24 AM, Robert Haas <robertmhaas@gmail.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?