Re: index prefetching
Peter Geoghegan <pg@bowt.ie>
On Tue, Mar 24, 2026 at 1:27 PM Andres Freund <andres@anarazel.de> wrote: > > But that means that it won't be triggered when we don't enter the "if > > (hscan->xs_blk != ItemPointerGetBlockNumber(tid))" block that contains > > all this code. Besides, it just doesn't seem possible that > > heap_page_prune_opt would release its caller's pin. > > I was more concerned about read_stream_next_buffer() returning the wrong > block, due to prefetching somehow "desynchronizing" with the scan position and > catching that when it's clear that we just read a new block, rather than in a > place where it could be either the continuation of a scan on the same page or > a new page. Then I don't follow. The existing assertions will catch that (I should know, they've failed enough times during development). Basically, I don't get the concern about heap_page_prune_opt releasing its caller's pin. Even if that happened, the existing assertions would still catch it. > I think I had largely missed the "danger" of index only scans here. I think > it'd be good to call that out more explicitly in these comments. Will do. > > > Does this only happen when paused? > > > > This "prefetchPos->valid = false" stuff is approximately the opposite > > of pausing. Pausing resolves the problem of prefetchPos getting so far > > ahead of scanPos that the batch ring buffer runs out of slots. Whereas > > this prefetchPos invalidation code helps the read stream deal with > > prefetchPos falling behind scanPos. > > Because I had somewhat missed the real cause of the problem - not calling the > read stream code due to index only scans - I thought that somehow we could end > up in this state due to not resuming prefetching before the scan position > overtakes the prefetch position. But I don't think that actually happen. Right, it can't happen. In any case the assertions we have are quite effective at catching problems like that. For example, if we don't resume prefetching and consume another batch, there's an assertion for that. Actually, there's more than one. There's a direct assertion, on the scan side. And the read stream callback itself has a precondition assertion that the read stream is not paused. > > > Wonder if it's worth somehow asserting that after this the page is actually > > > unguarded after the call. > > > > We used to, but the new layering forced me to remove it. Any ideas > > about how to add it back? > > Adding an "isGuarded" field to IndexScanBatchData would be the easiest > way. That way we can make assertions about the state without knowing anything > about the internal mechanism of how guarding is implemented. > > I doubt setting/clearing that field even when assertions are disabled will be > measurable, as long as you place it alongside the other booleans where there's > padding space available. I've prototyped that, and it works well. It'll be in v18. > After replacing the pause with an error I found that it's surprisingly easy to > hit on slow storage (or on fast storage if you set needed_wait=true in > read_stream_next_buffer()). I've not done any performance validation on > whether that means the limit is too low. It's been a while since I last validated performance to justify the current maximum number of batches. I used buffered I/O for that. I'm sure that a higher maximum with very slow storage and a very high effective_io_concurrency will provide some benefit. But perfectly handling that isn't essential for the first committed version of index prefetching. I must admit I'm unsure how to evaluate the maximum number of batches. It can make sense to pursue diminishing returns. But up to what point, and according to what principle? -- Peter Geoghegan
Commits
-
read stream: Split decision about look ahead for AIO and combining
- 8ca147d582a5 19 (unreleased) landed
-
read_stream: Only increase read-ahead distance when waiting for IO
- f63ca3379025 19 (unreleased) landed
-
aio: io_uring: Trigger async processing for large IOs
- a9ee66881744 19 (unreleased) landed
-
heapam: Keep buffer pins across index scan resets.
- 2d3490dd99f0 19 (unreleased) landed
-
heapam: Track heap block in IndexFetchHeapData.
- c7d09595e46f 19 (unreleased) landed
-
Move heapam_handler.c index scan code to new file.
- a29fdd6c8d81 19 (unreleased) landed
-
Rename heapam_index_fetch_tuple argument for clarity.
- 1adff1a0c558 19 (unreleased) landed
-
Optimize fast-path FK checks with batched index probes
- b7b27eb41a5c 19 (unreleased) cited
-
read_stream: Prevent distance from decaying too quickly
- 6e36930f9aaf 19 (unreleased) landed
-
read_stream: Issue IO synchronously while in fast path
- cceb1bf45e3a 19 (unreleased) landed
-
bufmgr: Return whether WaitReadBuffers() needed to wait
- 513374a47a71 19 (unreleased) landed
-
aio: io_uring: Allow IO methods to check if IO completed in the background
- 6e648e353fa0 19 (unreleased) landed
-
bufmgr: Make UnlockReleaseBuffer() more efficient
- f39cb8c01106 19 (unreleased) cited
-
Add fake LSN support to hash index AM.
- e5836f7b7d9a 19 (unreleased) landed
-
Make IndexScanInstrumentation a pointer in executor scan nodes.
- f026fbf059f2 19 (unreleased) landed
-
Use fake LSNs to improve nbtree dropPin behavior.
- 8a879119a1d1 19 (unreleased) landed
-
Move fake LSN infrastructure out of GiST.
- d774072f0040 19 (unreleased) landed
-
Use simplehash for backend-private buffer pin refcounts.
- a367c433ad01 19 (unreleased) landed
-
nbtree: Avoid allocating _bt_search stack.
- d071e1cfec23 19 (unreleased) landed
-
bufmgr: Fix use of wrong variable in GetPrivateRefCountEntrySlow()
- 6322a028fa43 19 (unreleased) landed
-
Conditional locking in pgaio_worker_submit_internal
- 29a0fb215779 19 (unreleased) landed
-
Reduce ExecSeqScan* code size using pg_assume()
- b227b0bb4e03 19 (unreleased) cited
-
Fix rare bug in read_stream.c's split IO handling.
- b421223172a2 19 (unreleased) cited
-
Remove HeapBitmapScan's skip_fetch optimization
- 459e7bf8e2f8 18.0 cited
-
Optimize nbtree backwards scans.
- 1bd4bc85cac2 18.0 cited
-
Fix multiranges to behave more like dependent types.
- 3e8235ba4f9c 17.0 cited
-
Add EXPLAIN (MEMORY) to report planner memory consumption
- 5de890e3610d 17.0 cited
-
Optimize nbtree backward scan boundary cases.
- c9c0589fda0e 17.0 cited
-
Increment xactCompletionCount during subtransaction abort.
- 90c885cdab8b 14.0 cited
-
Add nbtree Valgrind buffer lock checks.
- 4a70f829d86c 14.0 cited
-
Add nbtree high key "continuescan" optimization.
- 29b64d1de7c7 12.0 cited
-
Reduce pinning and buffer content locking for btree scans.
- 2ed5b87f96d4 9.5.0 cited
-
Teach btree to handle ScalarArrayOpExpr quals natively.
- 9e8da0f75731 9.2.0 cited