Re: Vacuum ERRORs out considering freezing dead tuples from before OldestXmin

Noah Misch <noah@leadboat.com>

From: Noah Misch <noah@leadboat.com>
To: Melanie Plageman <melanieplageman@gmail.com>, Peter Geoghegan <pg@bowt.ie>
Cc: Robert Haas <robertmhaas@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2024-06-25T01:30:04Z
Lists: pgsql-hackers

Commits

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

  2. Lower minimum maintenance_work_mem to 64kB

  3. Add accidentally omitted test to meson build file

  4. Use DELETE instead of UPDATE to speed up vacuum test

  5. Revert "Test that vacuum removes tuples older than OldestXmin"

  6. Ensure vacuum removes all visibly dead tuples older than OldestXmin

On Mon, Jun 24, 2024 at 04:51:24PM -0400, Peter Geoghegan wrote:
> On Mon, Jun 24, 2024 at 4:36 PM Robert Haas <robertmhaas@gmail.com> wrote:
> > I'm not sure I understand. The most important thing here is fixing the
> > bug. But if we have a choice of how to fix the bug, I'd prefer to do
> > it by having the pruning code test one horizon that is always correct,
> > rather than (as I think the patch does) having it test against two
> > horizons because as a way of covering possible discrepancies between
> > those values.
> 
> Your characterizing of OldestXmin + vistest as two horizons seems
> pretty arbitrary to me. I know what you mean, of course, but it seems
> like a distinction without a difference.

"Two horizons" matches how I model it.  If the two were _always_ indicating
the same notion of visibility, we wouldn't have this thread.

On Mon, Jun 24, 2024 at 03:23:39PM -0400, Melanie Plageman wrote:
> Right now, in master, we do use a single horizon when determining what
> is pruned -- that from GlobalVisState. OldestXmin is only used for
> freezing and full page visibility determinations. Using a different
> horizon for pruning by vacuum than freezing is what is causing the
> error on master.

Agreed, and I think using different sources for pruning and freezing is a
recipe for future bugs.  Fundamentally, both are about answering "is
snapshot_considers_xid_in_progress(snapshot, xid) false for every snapshot?"
That's not to say this thread shall unify the two, but I suspect that's the
right long-term direction.