Re: BitmapHeapScan streaming read user and prelim refactoring

On Fri, Mar 29, 2024 at 12:05:15PM +0100, Tomas Vondra wrote:
> 
> 
> On 3/29/24 02:12, Thomas Munro wrote:
> > On Fri, Mar 29, 2024 at 10:43 AM Tomas Vondra
> > <tomas.vondra@enterprisedb.com> wrote:
> >> I think there's some sort of bug, triggering this assert in heapam
> >>
> >>   Assert(BufferGetBlockNumber(hscan->rs_cbuf) == tbmres->blockno);
> > 
> > Thanks for the repro.  I can't seem to reproduce it (still trying) but
> > I assume this is with Melanie's v11 patch set which had
> > v11-0016-v10-Read-Stream-API.patch.
> > 
> > Would you mind removing that commit and instead applying the v13
> > stream_read.c patches[1]?  v10 stream_read.c was a little confused
> > about random I/O combining, which I fixed with a small adjustment to
> > the conditions for the "if" statement right at the end of
> > read_stream_look_ahead().  Sorry about that.  The fixed version, with
> > eic=4, with your test query using WHERE a < a, ends its scan with:
> > 
> 
> I'll give that a try. Unfortunately unfortunately the v11 still has the
> problem I reported about a week ago:
> 
>   ERROR:  prefetch and main iterators are out of sync
> 
> So I can't run the full benchmarks :-( but master vs. streaming read API
> should work, I think.

Odd, I didn't notice you reporting this ERROR popping up. Now that I
take a look, v11 (at least, maybe also v10) had this very sill mistake:

  if (scan->bm_parallel == NULL &&
      scan->rs_pf_bhs_iterator &&
      hscan->pfblockno > hscan->rs_base.blockno)
       elog(ERROR, "prefetch and main iterators are out of sync");

It errors out if the prefetch block is ahead of the current block --
which is the opposite of what we want. I've fixed this in attached v12.

This version also has v13 of the streaming read API. I noticed one
mistake in my bitmapheap scan streaming read user -- it freed the
streaming read object at the wrong time. I don't know if this was
causing any other issues, but it at least is fixed in this version.

- Melanie