On Wed, Oct 28, 2026 at 2:13 PM shihao zhong <[email protected]> wrote: > Hi Tomas, > > Thanks for the test script, I repro the same thing as you posted, and the > request size seems to just follow the fadvise range. > > > I don't have a great solution to this. We'd need to do I/O combining, > > similarly to what happens on the primary. I'm not sure if it's feasible, > > or if it'd be better to rework the prefetcher to use read_stream (which > > has all of this built in). > > Merging adjacent blocks into one smgrprefetch() call in the prefetcher > would be a small fix. But it is still only advice, so it does not help > io_direct. It also cannot merge across other relations. > > I would rather move to AIO, then we do not need to change > this code in the future. But there are few interesting issues in my mind > > 1. WAL mixes many relations, so need to have multiple read streams > 2. Redo may need to takes a cleanup lock... and prefetcher could also > pinned it, so we may need to change CheckBufferIsPinnedOnce()? > > Maybe the right way is to implement a new API for redo or just use > Start/WaitReadBuffers()? WDYT?
For the record, it was always the plan to use the new I/O infrastructure for that, and I am planning to post some patches I have for that soonish. Some history: the earliest prototypes of xlogprefetcher.c had dedicated IO workers (directly inspired by the "pg_prefaulter" project, which was incidentally written by people running without full page writes), but then I signed up to help Andres with his more ambitious general AIO project and they evolved to become io_method=worker, and then the next generation of the xlogprefetcher.c prototype did actually use an ancestor of read_stream.c and through that, all the centralised I/O combining and distance heuristics. When AIO turned out to take a lot longer to bake, xlogprefetcher.c was simplified to use something with a vaguely similar interface as an intermediate step, as this comment hints: /* * A simple circular queue of LSNs, using to control the number of * (potentially) inflight IOs. This stands in for a later more general IO * control mechanism, which is why it has the apparently unnecessary * indirection through a function pointer. */ typedef struct LsnReadQueue In the short term, I'm studying these reports, analysis and patches (apologies to Satya for not responding publicly sooner, I did put some effort into studying how the true-AIO-for-recovery patchset address this and related problems, but was not yet considering this to require a back-patchable fix... obviously worth reconsidering now), and also trying out an alternative idea. More very soon...
