Horton

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

Melanie Plageman <melanieplageman@gmail.com>

View thread
From: Melanie Plageman <melanieplageman@gmail.com>
To: John Naylor <johncnaylorls@gmail.com>
Cc: Peter Geoghegan <pg@bowt.ie>, Pg Hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>, Noah Misch <noah@leadboat.com>
Date: 2025-06-23T22:30:24Z
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

Attachments

On Mon, Jun 23, 2025 at 1:44 AM John Naylor <johncnaylorls@gmail.com> wrote:
>
> On Fri, Jun 20, 2025 at 9:45 PM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
>
> > Using auto_explain, I determined that the cursor was using an index-only scan with lower row counts. That meant it pinned an index leaf page instead of a heap page and the first round of index vacuuming couldn't complete because btree index vacuuming requires we acquire a cleanup lock on every leaf page.
> >
> > I solved this by disabling all index scans in the cursor's session.
>
> Interesting find! I wondered how it would look if the cursor referred
> to a extra non-indexed column, but the above seems fine and the whole
> thing probably easier to reason about with only a single column.

Yea, it was a fun little investigation. We could eliminate the
possibility of an index-only scan by adding another column and
referring to it, but I think forcing a sequential scan explicitly
makes the test clearer.

> It seems backwards to have the comment influence the code -- the
> comment should document decisions around the code. 2000 is already
> 100x smaller than pg16, so it's not really buying us much to be
> platform-aware.

You're right. It is odd to have the location of the comment affect the
code :). I've changed it as you suggested.

> + (PARALLEL 0 is a future-proofing measure in case we adopt
> + # parallel heap vacuuming)
>
> Maybe it's possible to phrase this so it's true regardless of whether
> we adopt that or not?
> "PARALLEL 0 shouldn't be necessary, but guards against the possibility
> of parallel heap vacuuming"

I've adopted this phrasing.

Attached v3 has all of the above. I think the only thing that is
needed to be changed for the backpatch to 17 is removing
io_combine_limit.

- Melanie