Re: relfrozenxid may disagree with row XIDs after 1ccc1e05ae

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>, Noah Misch <noah@leadboat.com>, Peter Geoghegan <pg@bowt.ie>, Alexander Lakhin <exclusion@gmail.com>, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2024-03-22T19:34:27Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Ensure vacuum removes all visibly dead tuples older than OldestXmin

  2. Combine freezing and pruning steps in VACUUM

  3. Handle non-chain tuples outside of heap_prune_chain()

  4. Fix false reports in pg_visibility

  5. Remove retry loop in heap_page_prune().

  6. vacuumlazy.c: document vistest and OldestXmin.

  7. Deduplicate choice of horizon for a relation procarray.c.

  8. Remove tupgone special case from vacuumlazy.c.

  9. Simplify state managed by VACUUM.

  10. Recycle nbtree pages deleted during same VACUUM.

  11. snapshot scalability: Don't compute global horizons while building snapshots.

  12. Raise error when affecting tuple moved into different partition.

On Fri, Mar 22, 2024 at 2:41 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> I'm still catching up here, so forgive me if this is a dumb question:
> Does using GlobalVisState instead of VacuumCutoffs->OldestXmin when
> freezing and determining relfrozenxid not solve the problem?

I think we need to understand what's actually happening here before we
jump to solutions. I think it's clear that we can't determine a
relfrozenxid in a way that is untethered from the decisions made while
pruning, but surely whoever wrote this code intended for there to be
some relationship between GlobalVisState and
VacuumCutoffs->OldestXmin. For example, it may be that they intended
for VacuumCutoffs->OldestXmin to always contain an XID that is older
than any XID that the GlobalVisState could possibly have pruned; but
they might have done that incorrectly, in such a fashion that when
GlobalVisState->maybe_needed moves backward the intended invariant no
longer holds.

But I think that we surely cannot think that they just threw some XID
that fell off the cart into VacuumCutoffs->OldestXmin and some other
XID that they pulled out of the air into GlobalVisState and hoped
everything worked out. It seems really likely that they had some
reason for thinking that we needed two things here rather than just
one. If not, OK, let's get rid of one of them and make it simpler, but
if we just assume that there's no real reason for having both fields
here and simplify accordingly, we're probably just going to be trading
this bug for some other one.

-- 
Robert Haas
EDB: http://www.enterprisedb.com