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

On Mon, Jun 23, 2025 at 1:44 AM John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
>
> On Fri, Jun 20, 2025 at 9:45 PM Melanie Plageman
> <melanieplageman(at)gmail(dot)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