On Sun, Mar 24, 2024 at 2:22 PM Tomas Vondra <tomas.von...@enterprisedb.com> wrote: > > On 3/24/24 18:38, Melanie Plageman wrote: > > I haven't had a chance yet to reproduce the regressions you saw in the > > streaming read user patch or to look closely at the performance results. > > So you tried to reproduce it and didn't hit the issue? Or didn't have > time to look into that yet? FWIW with v7 it failed almost immediately > (only a couple queries until hitting one triggering the issue), but v9 > that's not the case (hundreds of queries without an error).
I haven't started trying to reproduce it yet. > I however wonder what the plan with these patches is - do we still plan > to get some of this into v17? It seems to me we're getting uncomfortably > close to the end of the cycle, with a fairly incomplete idea of how it > affects performance. > > Which is why I've been focusing more on the refactoring patches (up to > 0015), to make sure those don't cause regressions if committed. And I > think that's generally true. Thank you for testing the refactoring patches with this in mind! Out of the refactoring patches, I think there is a subset of them that have independent value without the streaming read user. I think it is worth committing the first few patches because they remove a table AM layering violation. IMHO, all of the patches up to "Make table_scan_bitmap_next_block() async friendly" make the code nicer and better. And, if folks like the patch "Remove table_scan_bitmap_next_block()", then I think I could rebase that back in on top of "Make table_scan_bitmap_next_block() async friendly". This would mean table AMs would only have to implement one callback (table_scan_bitmap_next_tuple()) which I also think is a net improvement and simplification. The other refactoring patches may not be interesting without the streaming read user. > But for the main StreamingRead API the situation is very different. My intent for the bitmapheapscan streaming read user was to get it into 17, but I'm not sure that looks likely. The main issues Thomas is looking into right now are related to regressions for a fully cached scan (noticeable with the pg_prewarm streaming read user). With all of these fixed, I anticipate we will still see enough behavioral differences with the bitmapheap scan streaming read user that it may not be committable in time. Though, I have yet to work on reproducing the regressions with the BHS streaming read user mostly because I was focused on getting the refactoring ready and not as much because the streaming read API is unstable. - Melanie