Re: index prefetching

On Tue, Apr 22, 2025 at 2:34 PM Tomas Vondra <tomas@vondra.me> wrote:
> Yeah, that makes sense, although I've been thinking about this a bit
> differently. I haven't been trying to establish a new "component" to
> manage prefetching. For me the question was what's the right layer, so
> that unnecessary details don't leak into AM and/or executor.

FWIW that basically seems equivalent to what I said. If there's any
difference at all between what each of us has said, then it's only a
difference in emphasis. The "index scan manager" doesn't just manage
prefetching -- it manages the whole index scan, including details that
were previously only supposed to be known inside index AMs. It can do
so while weighing all relevant factors -- regardless of whether
they're related to the index structure or the heap structure.

It would be possible to (say) do everything at the index AM level
instead. But then we'd be teaching index AMs about heap/table AM
related costs, which would be a bad design, primarily because it would
have to duplicate the same logic in every supported index AM. Better
to have one dedicated layer that has an abstract-ish understanding of
both index AM scan costs, and table AM scan costs. It needs to be
abstract, but not too abstract -- costs like "read one index leaf
page" generalize well across all index AMs. And costs like "read one
table AM page" should also generalize quite well, at least across
block-based table AMs.

You primarily care about "doing the layering right", while I primarily
care about "making sure that one layer can see all relevant costs".
ISTM that these are two sides of the same coin.

> It requires exchanging some additional details with the AM, provided by
> the new callbacks.

I think of it as primarily externalizing decisions about index page
accesses. The index AM reads the next leaf page to be read because the
index scan manager tells it to. The index AM performs killitems
exactly as instructed by the index scan manager. And the index AM
doesn't really own as much context about the progress of the scan --
that all lives inside the scan manager instead. The scan manager has a
fairly fuzzy idea about how the index AM organizes data, but that
shouldn't matter.

> It seems the indexam.c achieves both your and mine goals, more or less.

Agreed.

> Yes. I wonder if we should introduce a separate abstraction for this, as
> a subset of indexam.c.

I like that idea.

> My argument was (a) ability to disable prefetching, and fall back to the
> old code if needed, and (b) handling use cases where prefetching does
> not work / is not implemented, even if only temporarily (e.g. ordered
> scan in SP-GiST). Maybe (a) is unnecessarily defensive, and (b) may not
> be needed. Not sure.

We don't need to make a decision on this for some time, but I still
lean towards forcing index AMs to make a choice between this new
interface, and the old amgettuple interface.

> > I don't see why prefetching should be mandatory with this new
> > interface. Surely it has to have adaptive "ramp-up" behavior already,
> > even when we're pretty sure that prefetching is a good idea from the
> > start?
> >
>
> Possibly, I may be too defensive. And perhaps in cases where we know the
> prefetching can't help we could disable that for the read_stream.

Shouldn't the index scan manager be figuring all this out for us,
automatically? Maybe that works in a very trivial way, at first. The
important point is that the design be able to support these
requirements in some later iteration of the feature -- though it's
unlikely to happen in the first Postgres version that the scan manager
thing appears in.

> I think hash should be fairly easy to support. But I was really thinking
> about doing SP-GiST, exactly because it's very different in some
> aspects, and I wanted to validate the design on that (for hash I think
> it's almost certain it's OK).

WFM.

There are still bugs in SP-GiST (and GiST) index-only scans:

https://www.postgresql.org/message-id/CAH2-Wz%3DPqOziyRSrnN5jAtfXWXY7-BJcHz9S355LH8Dt%3D5qxWQ@mail.gmail.com

It would be nice if the new index scan manager interface could fix
that bug, at least in the case of SP-GiST. By generalizing the
approach that nbtree takes, where we hang onto a leaf buffer pin.
Admittedly this would necessitate changes to SP-GiST VACUUM, which
doesn't cleanup lock any pages, but really has to in order to fix the
underlying bug. There are draft patches that try to fix the bug, which
might be a useful starting point.

> I think the cases affected by this the most are index-only scans on
> all-visible tables that fit into shared buffers, with
> correlated/sequential pattern. Or even regular index scans with all data
> in shred buffers.

My hope is that the index scan manager can be taught to back off when
this is happening, to avoid the regressions. Or that it can avoid them
by only gradually ramping up the prefetching. Does that sound
plausible to you?

-- 
Peter Geoghegan