On Wed, Apr 8, 2020 at 4:24 AM Thomas Munro <thomas.mu...@gmail.com> wrote: > Thanks for all that feedback. It's been a strange couple of weeks, > but I finally have a new version that addresses most of that feedback > (but punts on a couple of suggestions for later development, due to > lack of time).
Here's an executive summary of an off-list chat with Andres: * he withdrew his objection to the new definition of GetWalRcvWriteRecPtr() based on my argument that any external code will fail to compile anyway * he doesn't like the naive code that detects sequential access and skips prefetching; I agreed to rip it out for now and revisit if/when we have better evidence that that's worth bothering with; the code path that does that and the pg_stat_recovery_prefetch.skip_seq counter will remain, but be used only to skip prefetching of repeated access to the *same* block for now * he gave some feedback on the read_local_xlog_page() modifications: I probably need to reconsider the change to logical.c that passes NULL instead of cxt to the read_page callback; and the switch statement in read_local_xlog_page() probably should have a case for the preexisting mode * he +1s the plan to commit with the feature enabled, and revisit before release * he thinks the idea of a variant of ReadBuffer() that takes a PrefetchBufferResult (as sketched by the v6 0006 patch) broadly makes sense as a stepping stone towards his asychronous I/O proposal, but there's no point in committing something like 0006 without a user I'm going to go and commit the first few patches in this series, and come back in a bit with a new version of the main patch to fix the above and a compiler warning reported by cfbot.
