Robert Haas 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. I think it normally won't matter > because the xmax will probably be hinted invalid anyway, since we just > pruned the page which should have set hint bits everywhere, but if > those hint bits were lost then we'd eventually end up with an > accessible xmax pointing off into space.
Good catch. Also consider multixact freezing: if there is a long-running transaction which is a lock-only member of tuple's Xmax, and the multixact needs freezing because it's older than the multixact cutoff, we set the xmax to a new multixact which includes that old locker. See FreezeMultiXactId. > My first thought was to just delete the "else" but that would be bad > because we'd fail to set all-frozen immediately in a lot of cases > where we should. This needs a bit more thought than I have time to > give it right now. How about changing the return tuple of heap_prepare_freeze_tuple to a bitmap? Two flags: "Freeze [not] done" and "[No] more freezing needed" -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers