On Wed, Apr 7, 2021 at 5:09 AM Thomas Munro <thomas.mu...@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.
