While I haven't identified how exactly read stream is related to this
issue, what I've observed through debugging this issue is, during a
single read_stream_next_buffer() call, I observed that
heap_vac_scan_next_block() is invoked multiple times to return blocks
to the read stream and that we continued processing eagerly scanned
pages even after the success counter reaches zero while processing the
previous block. Even when I set both io_combine_limit and
maintenance_io_concurrency to 1, the debug logs showed that vacuum
still performs eager scanning two blocks ahead of the current block
being processed.
I spent some more time looking into this today. This assert no longer works with streaming vacuum.
Even with the minimums: io_combine_limit 1 and maintenance_io_concurrency set to 0, we will still invoke read_stream_get_block() one extra time at the start of the read stream.
To your point about eagerly scanning beyond the success and failure caps, it is possible, but it is hard to get into the situation where you far exceed them.
Though max_pinned_buffers is derived from io_combine_limit * maintenance_io_concurrency, it is limited by the size of the buffer access strategy ring (by default 2 MB for vacuum) and the size of shared buffers. And because we won't prefetch if sequential access is detected (and the prefetch distance has to first ramp up), it is unlikely we will both prefetch the maximum distance of randomly located blocks and combine blocks to the maximum IO size before returning to vacuum code from read stream code. That is to say, the maximum number of extra all-visible blocks scanned should not be very large.
We ran into a similar concern with the autoprewarm read stream user and decided the overrun would not be large enough to merit special handling.
In this case, because we don't know if we will successfully freeze eagerly scanned blocks until after they have been yielded by the read stream API, there is no way to use the caps to limit the prefetch distance or read combine size. Doing so would entail mixing logic into vacuum code about how the read stream API works -- at least as far as I can tell.
I think the best course of action is to either change the assert to a guard
if (vacrel->eager_scan_remaining_successes > 0)
vacrel->eager_scan_remaining_successes--;
or make the counter signed and allow it to go negative (as you mentioned).
