Re: WIP: WAL prefetch (another approach)

On Wed, Apr 8, 2020 at 4:24 AM Thomas Munro <thomas.munro@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.