On Fri, Jan 5, 2024 at 5:51 AM Nazir Bilal Yavuz <byavu...@gmail.com> wrote: > > On Fri, 5 Jan 2024 at 02:25, Jim Nasby <jim.na...@gmail.com> wrote: > > > > On 1/4/24 2:23 PM, Andres Freund wrote: > > > > On 2024-01-02 12:36:18 -0500, Melanie Plageman wrote: > > > > Subject: [PATCH v2 1/6] lazy_scan_skip remove unnecessary local var > > rel_pages > > Subject: [PATCH v2 2/6] lazy_scan_skip remove unneeded local var > > nskippable_blocks > > > > I think these may lead to worse code - the compiler has to reload > > vacrel->rel_pages/next_unskippable_block for every loop iteration, because > > it > > can't guarantee that they're not changed within one of the external > > functions > > called in the loop body. > > > > Admittedly I'm not up to speed on recent vacuum changes, but I have to > > wonder if the concept of skipping should go away in the context of vector > > IO? Instead of thinking about "we can skip this range of blocks", why not > > maintain a list of "here's the next X number of blocks that we need to > > vacuum"? > > Sorry if I misunderstood. AFAIU, with the help of the vectored IO; > "the next X number of blocks that need to be vacuumed" will be > prefetched by calculating the unskippable blocks ( using the > lazy_scan_skip() function ) and the X will be determined by Postgres > itself. Do you have something different in your mind?
I think you are both right. As we gain more control of readahead from within Postgres, we will likely want to revisit this heuristic as it may not serve us anymore. But the streaming read interface/vectored I/O is also not a drop-in replacement for it. To change anything and ensure there is no regression, we will probably have to do cross-platform benchmarking, though. That being said, I would absolutely love to get rid of the skippable ranges because I find them very error-prone and confusing. Hopefully now that the skipping logic is isolated to a single function, it will be easier not to trip over it when working on lazy_scan_heap(). - Melanie
