Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()

Peter Geoghegan <pg@bowt.ie>

From: Peter Geoghegan <pg@bowt.ie>
To: Noah Misch <noah@leadboat.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>, robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2024-01-06T21:30:40Z
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 Sat, Jan 6, 2024 at 12:24 PM Noah Misch <noah@leadboat.com> wrote:
> On Sun, Dec 31, 2023 at 03:53:34PM -0800, Peter Geoghegan wrote:
> > My guess is that there is a decent chance that backpatching 1ccc1e05ae
> > would be okay, but that isn't much use. I really don't know either way
> > right now. And I wouldn't like to speculate too much further before
> > gaining a proper understanding of what's going on here.
>
> Fair enough.  While I agree there's a decent chance back-patching would be
> okay, I think there's also a decent chance that 1ccc1e05ae creates the problem
> Matthias theorized.  Something like: we update relfrozenxid based on
> OldestXmin, even though GlobalVisState caused us to retain a tuple older than
> OldestXmin.  Then relfrozenxid disagrees with table contents.

Either every relevant code path has the same OldestXmin to work off
of, or the whole NewRelfrozenXid/relfrozenxid-tracking thing can't be
expected to work as designed. I find it a bit odd that
pruneheap.c/GlobalVisState has no direct understanding of this
dependency (none that I can discern, at least). Wouldn't it at least
be more natural if pruneheap.c could access OldestXmin when run inside
VACUUM? (Could just be used by defensive hardening code.)

We're also relying on vacuumlazy.c's call to vacuum_get_cutoffs()
(which itself calls GetOldestNonRemovableTransactionId) taking place
before vacuumlazy.c goes on to call GlobalVisTestFor() a few lines
further down (I think). It seems like even the code in procarray.c
might have something to say about the vacuumlazy.c dependency, too.
But offhand it doesn't look like it does, either. Why shouldn't we
expect random implementation details in code like ComputeXidHorizons()
to break the assumption/dependency within vacuumlazy.c?

I also worry about the possibility that GlobalVisTestShouldUpdate()
masks problems in this area (as opposed to causing the problems). It
seems very hard to test.

> I did find this thread while researching the symptoms I was seeing.  No
> partitioning where I saw them.

If this was an isolated incident then it could perhaps have been a
symptom of corruption. Though corruption seems highly unlikely to be
involved with the cases that I've seen, since they appear
intermittently, across a variety of different contexts/versions.

-- 
Peter Geoghegan