Re: index prefetching

On Tue, Sep 15, 2026 at 2:47 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> Attached is v36, which is just to keep the patch series applying.

Attached is v37. Changes:

* Major overhaul of the design of the hashgetbatch changes.

Working on recent bug fix commit 173cf85a gave me the impression that
hash scans traverse sibling links in an overly complicated way, which
is particularly awkward with amgetbatch. The logic that determines how
to visit the next sibling page is simulataneously in low level code
like _hash_readpage (actually, in its helper functions _hash_readnext
and _hash_readprev) and in _hash_next, which is high level code. It's
just too hard to follow how sibling traversal works right now.

v37 moves all of the logic related to hash index sibling traversal
into one new function called _hash_readnextpage. This is modeled on
nbtree's _bt_readnextpage function. There's no longer any sibling
traversal logicc in _hash_readpage; now it just reads a page as
instructed by its caller, without any locking or pinning side-effects
(again, like nbtree).

Arguably this is independent refactoring work. However, I found the
previous arrangement with buffer pins too weird under hashgetbatch, so
I don't think so. Now our assumption is that every buffer pinned and
locked by _hash_getbuf is likely to be returned within a batch (with
its original pin managed by that patch), while any independently held
scan state buffer pins (pins stored in hashso_bucket_buf or in
hashso_split_bucket_buf) need a IncrBufferRefCount of their own -- not
the other way around.

* New experimental "Add read stream request deduplication" patch. This
fixes several remaining regressions originally reported by Tomas [1]
[2] (and recently echoed by Manu), which all turned out to have the
same underlying cause.

Duplicate requests can confuse the read stream heuristics.
Essentially, the same heap blocks are hit repeatedly, causing them to
be falsely counted as independent events. Index prefetching is the
only in-core read stream user that asks for the same block twice, so
it's not surprising that it presents unique challenges. As Tomas has
put it, this causes the prefetch distance to "collapse" to ~2.0, even
in cases where a larger distance actually makes sense.

I found that with worker + eic 16, dedup takes the patch set's runtime
from 1.15x of what I got on master, to only 0.58x as long as the same
master baseline with Tomas' original test case from [1] and [2]. This
makes intuitive sense; we always expected prefetching to *at least*
have some modest benefit with this query.

The request deduplication patch maintains a small cache of 32 recently
requested block numbers within the read stream. Any repeat requests
within this window are recorded and then handled as synchronous
requests. The pages are virtually guaranteed to still be in
shared_buffers when those requests happen. This effectively prevents
them from impacting the read stream heuristics.

Without dedup, a repeat of a block whose read is in flight comes back
as a foreign I/O. It occupies one of the max_ios slots and bumps the
I/O count without any device read, so look-ahead stops when the slots
fill. Repeats also prevent IO combining, which seems to be the main
issue with Tomas' test case. Without dedup the stream issues 1.45M
single-block operations, with all 16 slots busy, and 18k waits; with
it, 26,974 reads averaging 15.97 blocks. So it's pretty obvious that
we need something like this dedup patch.

* SP-GiST bug fix from Rui Zhao folded into the spgistgetbatch patch.

[1] https://postgr.es/m/ce485718-f49e-4cbc-99de-5b935fe98926@vondra.me
[2] https://postgr.es/m/8f5d66cf-44e9-40e0-8349-d5590ba8efb4@vondra.me
[3] https://postgr.es/m/efac3238-6f34-41ea-a393-26cc0441b506@vondra.me
--
Peter Geoghegan