Re: BitmapHeapScan streaming read user and prelim refactoring

Hi,

On 2024-03-14 17:39:30 -0400, Melanie Plageman wrote:
> I will soon send out a summary of what we investigated off-list about
> 0010 (though we didn't end up concluding anything). My "fix" (leaving
> BitmapAdjustPrefetchIterator() above table_scan_bitmap_next_block())
> eliminates the regression in 0010 on the one example that I repro'd
> upthread, but it would be good to know if it eliminates the
> regressions across some other tests.

I spent a good amount of time looking into this with Melanie. After a bunch of
wrong paths I think I found the issue: We end up prefetching blocks we have
already read. Notably this happens even as-is on master - just not as
frequently as after moving BitmapAdjustPrefetchIterator().

From what I can tell the prefetching in parallel bitmap heap scans is
thoroughly broken.  I added some tracking of the last block read, the last
block prefetched to ParallelBitmapHeapState and found that with a small
effective_io_concurrency we end up with ~18% of prefetches being of blocks we
already read! After moving the BitmapAdjustPrefetchIterator() to rises to 86%,
no wonder it's slower...

The race here seems fairly substantial - we're moving the two iterators
independently from each other, in multiple processes, without useful locking.

I'm inclined to think this is a bug we ought to fix in the backbranches.

Greetings,

Andres Freund