On Wed, Apr 7, 2021 at 5:09 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> 0001 + 0002 get rid of the callback interface and replace it with a
> state machine, making it the client's problem to supply data when it
> returns XLREAD_NEED_DATA. I found this interface nicer to work with,
> for my WAL decoding buffer patch (CF 2410), and I understand that the
> encryption patch set can also benefit from it. Certainly when I
> rebased my project on this patch set, I prefered the result.
+ state->readLen = pageHeaderSize;
This variable is used for the XLogPageReader to say how much data it
wants, but also for the caller to indicate how much data is loaded.
Wouldn't it be better to split this into two variables: bytesWanted
and bytesAvailable? (I admit that I spent a whole afternoon debugging
after confusing myself about that, when rebasing my WAL readahead
patch recently).
I wonder if it would be better to have the client code access these
values through functions (even if they just access the variables in a
static inline function), to create a bit more separation? Something
like XLogReaderGetWanted(&page_lsn, &bytes_wanted), and then
XLogReaderSetAvailable(state, 42)? Just an idea.