Re: Eagerly scan all-visible pages to amortize aggressive vacuum

On Wed, Feb 5, 2025 at 12:23 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
> I started getting worried thinking about this. If you have a cursor
> for select * from a table and fetch forward far enough, couldn't
> vacuum fail to get cleanup locks on a whole range of blocks?

I don't think so. A given scan holds at most 1 heap pin at a time, so
I don't see how a cursor doing a FETCH FORWARD would make anything
happen that wouldn't otherwise. I do think it's theoretically possible
that a running query and a running VACUUM could be going at exactly
the same speed so that the VACUUM always tries to pin every page at
the exact same moment the SELECT has it pinned, but in practice I
think it is very unlikely that the timing will work out like that.
Like win-the-lottery kind of unlikely.

> Yea, I guess that means the freeze limit caps wasted read I/O -- but
> you could end up doing no read I/O and still hitting the freeze fail
> limit because it is trying to detect when data is too new to be worth
> eager scanning. But that's probably the behavior we want.

The too-new case is very scary, I think. It's already a problem that
when there are long-running transactions in play, autovacuum commands
a VACUUM which finds nothing it can usefully clean up. It would be
very valuable for someone to figure out a way to prevent that, but the
relevance to this patch is that we need to try hard to avoid making
that problem substantially worse than it is already. Even if the whole
database is memory-resident, the too-new case allows for wasting a
bunch of effort that we would rather not waste; the cap here tries to
make sure that isn't any worse than it needs to be.

> > > Interestingly, we call heap_tuple_should_freeze() in
> > > lazy_scan_noprune(), so we actually could tell whether or not there
> > > are some tuples on the page old enough to trigger freezing if we had
> > > gotten the cleanup lock. One option would be to add a new output
> > > parameter to lazy_scan_noprune() that indicates whether or not there
> > > were tuples with xids older than the FreezeLimit, and only if there
> > > were not, count it as a failed eager scan.
> >
> > Yeah, that could be done, and it doesn't sound like a bad idea.
> > However, I'm also unsure whether we need to add additional logic for
> > this case. My intuition is that it just won't happen very often. I'm
> > not quite confident that I'm correct about that, but if I had to
> > guess, my guess would be "rare scenario".
>
> I've not done this in the attached v16. I have added a comment about it.
> I think not doing it is a judgment call and not a bug, right?

As of this writing, I do not believe that this is an essential part of
this patch, but it is always possible that with the benefit of
hindsight it will seem otherwise. That's just life, though. If I knew
in advance which of my decisions would later seem like mistakes, I
would make very few mistakes.

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