Remove page-read callback from XLogReaderState. - Mailing list pgsql-hackers

Hello. As mentioned before [1], read_page callback in
XLogReaderState is a cause of headaches. Adding another
remote-controlling stuff to xlog readers makes things messier [2].

I refactored XLOG reading functions so that we don't need the
callback. In short, ReadRecrod now calls XLogPageRead directly
with the attached patch set.

|     while (XLogReadRecord(xlogreader, RecPtr, &record, &errormsg)
|            == XLREAD_NEED_DATA)
|         XLogPageRead(xlogreader, fetching_ckpt, emode, randAccess);

On the other hand, XLogReadRecord became a bit complex. The patch
makes XLogReadRecord a state machine. I'm not confident that the
additional complexity is worth doing. Anyway I'll gegister this
to the next CF.

[1] https://www.postgresql.org/message-id/47215279-228d-f30d-35d1-16af695e53f3@iki.fi

[2] https://www.postgresql.org/message-id/20190412.122711.158276916.horiguchi.kyotaro@lab.ntt.co.jp

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 3de14fb47987e9dd8189bfad3ca7264c26b719eb Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Apr 2019 10:22:49 +0900
Subject: [PATCH 01/10] Define macros to make XLogReadRecord a state machine

To minimize apparent imapct on code, use some macros as syntax sugar. This is a similar stuff with ExecInterpExpr but a
bitdifferent. The most significant difference is that  this stuff allows some functions are leaved midst of their work
thencontinue. Roughly speaking this is used as the follows.
 

enum retval
some_func()
{
  static .. internal_variables;

  XLR_SWITCH();
  ...
  XLR_LEAVE(STATUS1, RETVAL_CONTINUE);
  ...
  XLR_LEAVE(STATUS2, RETVAL_CONTINUE2);
  ...
  XLR_SWITCH_END();

  XLR_RETURN(RETVAL_FINISH);
}

The caller uses the function as follows:

  while (some_func() != RETVAL_FINISH)
  {
    <do some work>;
  }
---
 src/backend/access/transam/xlogreader.c | 63 +++++++++++++++++++++++++++++++++
 src/include/access/xlogreader.h         |  3 ++
 2 files changed, 66 insertions(+)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 9196aa3aae..5299765040 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -29,6 +29,69 @@
 #include "utils/memutils.h"
 #endif
 
+/*
+ * Use computed-goto-based opcode dispatch when computed gotos are available.
+ * But use a separate symbol so that it's easy to adjust locally in this file
+ * for development and testing.
+ */
+#ifdef HAVE_COMPUTED_GOTO
+#define XLR_USE_COMPUTED_GOTO
+#endif                            /* HAVE_COMPUTED_GOTO */
+
+/*
+ * Macros for opcode dispatch.
+ *
+ * XLR_SWITCH - just hides the switch if not in use.
+ * XLR_CASE - labels the implementation of named expression step type.
+ * XLR_DISPATCH - jump to the implementation of the step type for 'op'.
+ * XLR_LEAVE - leave the function and return here at the next call.
+ * XLR_RETURN - return from the function and set state to initial state.
+ * XLR_END - just hides the closing brace if not in use.
+ */
+#if defined(XLR_USE_COMPUTED_GOTO)
+#define XLR_SWITCH()                                        \
+    /* Don't call duplicatedly */                            \
+    static int callcnt = 0 PG_USED_FOR_ASSERTS_ONLY;        \
+    do {                                                    \
+        if ((XLR_STATE).j)                                    \
+            goto *((void *) (XLR_STATE).j);                    \
+        XLR_CASE(XLR_INIT_STATE);                            \
+        Assert(++callcnt == 1);                                \
+    } while (0)
+#define XLR_CASE(name)        name:
+#define XLR_DISPATCH()        goto *((void *) (XLR_STATE).j)
+#define XLR_LEAVE(name, code) do {                \
+        (XLR_STATE).j = (&&name); return (code);    \
+        XLR_CASE(name);                                \
+    } while (0)
+#define XLR_RETURN(code) \
+  do {                                                        \
+      Assert(--callcnt == 0);                                \
+      (XLR_STATE).j = (&&XLR_INIT_STATE); return (code);    \
+  } while (0)
+#define XLR_SWITCH_END()
+#else                            /* !XLR_USE_COMPUTED_GOTO */
+#define XLR_SWITCH()                                 \
+    /* Don't call duplicatedly */                     \
+    static int callcnt = 0 PG_USED_FOR_ASSERTS_ONLY; \
+    switch ((XLR_STATE).c) {                         \
+    XLR_CASE(XLR_INIT_STATE);                         \
+    Assert(++callcnt == 1);                             \
+#define XLR_CASE(name)        case name:
+#define XLR_DISPATCH()        goto starteval
+#define XLR_LEAVE(name, code) \
+  do {                                            \
+      (XLR_STATE).c = (name); return (code);    \
+      XLR_CASE(name);                            \
+  } while (0)
+#define XLR_RETURN(code) \
+  do {                                                        \
+      Assert(--callcnt == 0);                                \
+      (XLR_STATE).c = (XLR_INIT_STATE); return (code);        \
+  } while (0)
+#define XLR_SWITCH_END()    }
+#endif                            /* XLR_USE_COMPUTED_GOTO */
+
 static bool allocate_recordbuf(XLogReaderState *state, uint32 reclength);
 
 static bool ValidXLogRecordHeader(XLogReaderState *state, XLogRecPtr RecPtr,
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index f3bae0bf49..30500c35c7 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -240,6 +240,9 @@ extern bool DecodeXLogRecord(XLogReaderState *state, XLogRecord *record,
 #define XLogRecBlockImageApply(decoder, block_id) \
     ((decoder)->blocks[block_id].apply_image)
 
+/* Reset the reader state */
+#define XLREAD_RESET(state) ((state)->xlnd_state.j = (state)->xlread_state.j = 0)
+
 extern bool RestoreBlockImage(XLogReaderState *recoder, uint8 block_id, char *dst);
 extern char *XLogRecGetBlockData(XLogReaderState *record, uint8 block_id, Size *len);
 extern bool XLogRecGetBlockTag(XLogReaderState *record, uint8 block_id,
-- 
2.16.3

From 59675513983b70fc1f71c7770b345553a0320c23 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Wed, 17 Apr 2019 14:15:16 +0900
Subject: [PATCH 02/10] Make ReadPageInternal a state machine

This patch set aims to remove read_page call back from
XLogReaderState. This is done in two steps. This patch does the first
stap. Makes ReadPageInternal, which currently calling read_page
callback, a state machine and move the caller sites to XLogReadRecord
and other direct callers of the callback.
---
 src/backend/access/transam/xlog.c              |  15 ++-
 src/backend/access/transam/xlogreader.c        | 175 +++++++++++++++----------
 src/backend/access/transam/xlogutils.c         |   8 +-
 src/backend/replication/logical/logicalfuncs.c |   6 +-
 src/backend/replication/walsender.c            |  10 +-
 src/bin/pg_rewind/parsexlog.c                  |  17 ++-
 src/bin/pg_waldump/pg_waldump.c                |   8 +-
 src/include/access/xlogreader.h                |  46 +++++--
 src/include/access/xlogutils.h                 |   2 +-
 src/include/replication/logicalfuncs.h         |   2 +-
 10 files changed, 181 insertions(+), 108 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c00b63c751..2130671d36 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -885,7 +885,7 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
 static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
              int source, bool notfoundOk);
 static int    XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source);
-static int XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
+static void XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
              int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
              TimeLineID *readTLI);
 static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
@@ -11507,7 +11507,7 @@ CancelBackup(void)
  * XLogPageRead() to try fetching the record from another source, or to
  * sleep and retry.
  */
-static int
+static void
 XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen,
              XLogRecPtr targetRecPtr, char *readBuf, TimeLineID *readTLI)
 {
@@ -11566,7 +11566,8 @@ retry:
             readLen = 0;
             readSource = 0;
 
-            return -1;
+            xlogreader->readLen = -1;
+            return;
         }
     }
 
@@ -11661,7 +11662,8 @@ retry:
         goto next_record_is_invalid;
     }
 
-    return readLen;
+    xlogreader->readLen = readLen;
+    return;
 
 next_record_is_invalid:
     lastSourceFailed = true;
@@ -11675,8 +11677,9 @@ next_record_is_invalid:
     /* In standby-mode, keep trying */
     if (StandbyMode)
         goto retry;
-    else
-        return -1;
+
+    xlogreader->readLen = -1;
+    return;
 }
 
 /*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 5299765040..f8375eceeb 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -98,8 +98,7 @@ static bool ValidXLogRecordHeader(XLogReaderState *state, XLogRecPtr RecPtr,
                       XLogRecPtr PrevRecPtr, XLogRecord *record, bool randAccess);
 static bool ValidXLogRecord(XLogReaderState *state, XLogRecord *record,
                 XLogRecPtr recptr);
-static int ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr,
-                 int reqLen);
+static bool XLogNeedData(XLogReaderState *state, XLogRecPtr pageptr, int reqLen);
 static void report_invalid_record(XLogReaderState *state, const char *fmt,...) pg_attribute_printf(2, 3);
 
 static void ResetDecoder(XLogReaderState *state);
@@ -286,7 +285,6 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
     uint32        targetRecOff;
     uint32        pageHeaderSize;
     bool        gotheader;
-    int            readOff;
 
     /*
      * randAccess indicates whether to verify the previous-record pointer of
@@ -338,15 +336,17 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
      * byte to cover the whole record header, or at least the part of it that
      * fits on the same page.
      */
-    readOff = ReadPageInternal(state,
-                               targetPagePtr,
-                               Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ));
-    if (readOff < 0)
+    while (XLogNeedData(state, targetPagePtr,
+                        Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ)))
+        state->read_page(state, state->loadPagePtr, state->loadLen,
+                         state->currRecPtr, state->readBuf,
+                         &state->readPageTLI);
+    
+    if (state->readLen < 0)
         goto err;
 
     /*
-     * ReadPageInternal always returns at least the page header, so we can
-     * examine it now.
+     * We have loaded at least the page header, so we can examine it now.
      */
     pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) state->readBuf);
     if (targetRecOff == 0)
@@ -372,8 +372,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
         goto err;
     }
 
-    /* ReadPageInternal has verified the page header */
-    Assert(pageHeaderSize <= readOff);
+    /* XLogNeedData has verified the page header */
+    Assert(pageHeaderSize <= state->readLen);
 
     /*
      * Read the record length.
@@ -450,14 +450,17 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
             targetPagePtr += XLOG_BLCKSZ;
 
             /* Wait for the next page to become available */
-            readOff = ReadPageInternal(state, targetPagePtr,
-                                       Min(total_len - gotlen + SizeOfXLogShortPHD,
-                                           XLOG_BLCKSZ));
+            while (XLogNeedData(state, targetPagePtr,
+                                Min(total_len - gotlen + SizeOfXLogShortPHD,
+                                    XLOG_BLCKSZ)))
+                state->read_page(state, state->loadPagePtr, state->loadLen,
+                                 state->currRecPtr, state->readBuf,
+                                 &state->readPageTLI);
 
-            if (readOff < 0)
+            if (state->readLen < 0)
                 goto err;
 
-            Assert(SizeOfXLogShortPHD <= readOff);
+            Assert(SizeOfXLogShortPHD <= state->readLen);
 
             /* Check that the continuation on next page looks valid */
             pageHeader = (XLogPageHeader) state->readBuf;
@@ -486,20 +489,28 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
             /* Append the continuation from this page to the buffer */
             pageHeaderSize = XLogPageHeaderSize(pageHeader);
 
-            if (readOff < pageHeaderSize)
-                readOff = ReadPageInternal(state, targetPagePtr,
-                                           pageHeaderSize);
+            if (state->readLen < pageHeaderSize)
+            {
+                while (XLogNeedData(state, targetPagePtr, pageHeaderSize))
+                    state->read_page(state, state->loadPagePtr, state->loadLen,
+                                     state->currRecPtr, state->readBuf,
+                                     &state->readPageTLI);
+            }
 
-            Assert(pageHeaderSize <= readOff);
+            Assert(pageHeaderSize <= state->readLen);
 
             contdata = (char *) state->readBuf + pageHeaderSize;
             len = XLOG_BLCKSZ - pageHeaderSize;
             if (pageHeader->xlp_rem_len < len)
                 len = pageHeader->xlp_rem_len;
 
-            if (readOff < pageHeaderSize + len)
-                readOff = ReadPageInternal(state, targetPagePtr,
-                                           pageHeaderSize + len);
+            if (state->readLen < pageHeaderSize + len)
+            {
+                if (XLogNeedData(state, targetPagePtr, pageHeaderSize + len))
+                    state->read_page(state, state->loadPagePtr, state->loadLen,
+                                     state->currRecPtr, state->readBuf,
+                                     &state->readPageTLI);
+            }
 
             memcpy(buffer, (char *) contdata, len);
             buffer += len;
@@ -530,9 +541,13 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
     else
     {
         /* Wait for the record data to become available */
-        readOff = ReadPageInternal(state, targetPagePtr,
-                                   Min(targetRecOff + total_len, XLOG_BLCKSZ));
-        if (readOff < 0)
+        while (XLogNeedData(state, targetPagePtr,
+                            Min(targetRecOff + total_len, XLOG_BLCKSZ)))
+            state->read_page(state, state->loadPagePtr, state->loadLen,
+                             state->currRecPtr, state->readBuf,
+                             &state->readPageTLI);
+
+        if (state->readLen < 0)
             goto err;
 
         /* Record does not cross a page boundary */
@@ -575,32 +590,41 @@ err:
 }
 
 /*
- * Read a single xlog page including at least [pageptr, reqLen] of valid data
- * via the read_page() callback.
+ * Checks that an xlog page loaded in state->readBuf is including at least
+ * [pageptr, reqLen] and the page is valid.
  *
- * Returns -1 if the required page cannot be read for some reason; errormsg_buf
- * is set in that case (unless the error occurs in the read_page callback).
+ * Returns false if the required data is fully loaded. state->readLen is set to
+ * -1 when the loaded data is found to be invalid.
  *
- * We fetch the page from a reader-local cache if we know we have the required
- * data and if there hasn't been any error since caching the data.
+ * Otherwise, returns true and requests data using state->loadPagePtr and
+ * state->loadLen. The caller should load the region to state->readBuf and
+ * call this function again.
  */
-static int
-ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen)
+static bool
+XLogNeedData(XLogReaderState *state, XLogRecPtr pageptr, int reqLen)
 {
-    int            readLen;
-    uint32        targetPageOff;
-    XLogSegNo    targetSegNo;
-    XLogPageHeader hdr;
+    /*
+     * This function is a state machine that can exit and reenter at any place
+     * marked as XLR_LEAVE. All internal state is preserved through multiple
+     * calls.
+     */
+    static uint32        targetPageOff;
+    static XLogSegNo    targetSegNo;
+    static XLogPageHeader hdr;
 
-    Assert((pageptr % XLOG_BLCKSZ) == 0);
+#define XLR_STATE state->xlnd_state
+#define XLR_INIT_STATE XLND_STATE_INIT
+
+    XLR_SWITCH ();
 
     XLByteToSeg(pageptr, targetSegNo, state->wal_segment_size);
     targetPageOff = XLogSegmentOffset(pageptr, state->wal_segment_size);
+    Assert((pageptr % XLOG_BLCKSZ) == 0);
 
     /* check whether we have all the requested data already */
     if (targetSegNo == state->readSegNo && targetPageOff == state->readOff &&
         reqLen <= state->readLen)
-        return state->readLen;
+        XLR_RETURN(false);
 
     /*
      * Data is not in our buffer.
@@ -616,18 +640,17 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen)
      */
     if (targetSegNo != state->readSegNo && targetPageOff != 0)
     {
-        XLogRecPtr    targetSegmentPtr = pageptr - targetPageOff;
+        state->loadPagePtr = pageptr - targetPageOff;
+        state->loadLen = XLOG_BLCKSZ;
+        XLR_LEAVE(XLND_STATE_SEGHEAD, true);
 
-        readLen = state->read_page(state, targetSegmentPtr, XLOG_BLCKSZ,
-                                   state->currRecPtr,
-                                   state->readBuf, &state->readPageTLI);
-        if (readLen < 0)
+        if (state->readLen < 0)
             goto err;
 
         /* we can be sure to have enough WAL available, we scrolled back */
-        Assert(readLen == XLOG_BLCKSZ);
+        Assert(state->readLen == XLOG_BLCKSZ);
 
-        if (!XLogReaderValidatePageHeader(state, targetSegmentPtr,
+        if (!XLogReaderValidatePageHeader(state, state->loadPagePtr,
                                           state->readBuf))
             goto err;
     }
@@ -636,48 +659,53 @@ ReadPageInternal(XLogReaderState *state, XLogRecPtr pageptr, int reqLen)
      * First, read the requested data length, but at least a short page header
      * so that we can validate it.
      */
-    readLen = state->read_page(state, pageptr, Max(reqLen, SizeOfXLogShortPHD),
-                               state->currRecPtr,
-                               state->readBuf, &state->readPageTLI);
-    if (readLen < 0)
+    state->loadPagePtr = pageptr;
+    state->loadLen = Max(reqLen, SizeOfXLogShortPHD);
+    XLR_LEAVE(XLND_STATE_PAGEHEAD, true);
+
+    if (state->readLen < 0)
         goto err;
 
-    Assert(readLen <= XLOG_BLCKSZ);
+    Assert(state->readLen <= XLOG_BLCKSZ);
 
     /* Do we have enough data to check the header length? */
-    if (readLen <= SizeOfXLogShortPHD)
+    if (state->readLen <= SizeOfXLogShortPHD)
         goto err;
 
-    Assert(readLen >= reqLen);
+    Assert(state->readLen >= state->loadLen);
 
     hdr = (XLogPageHeader) state->readBuf;
 
     /* still not enough */
-    if (readLen < XLogPageHeaderSize(hdr))
+    if (state->readLen < XLogPageHeaderSize(hdr))
     {
-        readLen = state->read_page(state, pageptr, XLogPageHeaderSize(hdr),
-                                   state->currRecPtr,
-                                   state->readBuf, &state->readPageTLI);
-        if (readLen < 0)
+        state->loadPagePtr = pageptr;
+        state->loadLen = XLogPageHeaderSize(hdr);
+        XLR_LEAVE(XLND_STATE_PAGEFULLHEAD, true);
+
+        if (state->readLen < 0)
             goto err;
     }
 
+    XLR_SWITCH_END();
+
     /*
      * Now that we know we have the full header, validate it.
      */
-    if (!XLogReaderValidatePageHeader(state, pageptr, (char *) hdr))
-        goto err;
+    if (!XLogReaderValidatePageHeader(state, pageptr, (char *) state->readBuf))
+            goto err;
 
     /* update read state information */
     state->readSegNo = targetSegNo;
     state->readOff = targetPageOff;
-    state->readLen = readLen;
 
-    return readLen;
+    XLR_RETURN(false);
 
 err:
     XLogReaderInvalReadState(state);
-    return -1;
+    state->readLen = -1;
+
+    XLR_RETURN(false);
 }
 
 /*
@@ -976,7 +1004,6 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
         XLogRecPtr    targetPagePtr;
         int            targetRecOff;
         uint32        pageHeaderSize;
-        int            readLen;
 
         /*
          * Compute targetRecOff. It should typically be equal or greater than
@@ -984,7 +1011,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
          * that, except when caller has explicitly specified the offset that
          * falls somewhere there or when we are skipping multi-page
          * continuation record. It doesn't matter though because
-         * ReadPageInternal() is prepared to handle that and will read at
+         * CheckPage() is prepared to handle that and will read at
          * least short page-header worth of data
          */
         targetRecOff = tmpRecPtr % XLOG_BLCKSZ;
@@ -993,8 +1020,12 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
         targetPagePtr = tmpRecPtr - targetRecOff;
 
         /* Read the page containing the record */
-        readLen = ReadPageInternal(state, targetPagePtr, targetRecOff);
-        if (readLen < 0)
+        while(XLogNeedData(state, targetPagePtr, targetRecOff))
+            state->read_page(state, state->loadPagePtr, state->loadLen,
+                             state->currRecPtr, state->readBuf,
+                             &state->readPageTLI);
+
+        if (state->readLen < 0)
             goto err;
 
         header = (XLogPageHeader) state->readBuf;
@@ -1002,8 +1033,12 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
         pageHeaderSize = XLogPageHeaderSize(header);
 
         /* make sure we have enough data for the page header */
-        readLen = ReadPageInternal(state, targetPagePtr, pageHeaderSize);
-        if (readLen < 0)
+        while (XLogNeedData(state, targetPagePtr, pageHeaderSize))
+            state->read_page(state, state->loadPagePtr, state->loadLen,
+                             state->currRecPtr, state->readBuf,
+                             &state->readPageTLI);
+               
+        if (state->readLen < 0)
             goto err;
 
         /* skip over potential continuation data */
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 10a663bae6..c853e1f0e3 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -907,7 +907,7 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
  * exists for normal backends, so we have to do a check/sleep/repeat style of
  * loop for now.
  */
-int
+void
 read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
                      int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
                      TimeLineID *pageTLI)
@@ -1009,7 +1009,8 @@ read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
     else if (targetPagePtr + reqLen > read_upto)
     {
         /* not enough data there */
-        return -1;
+        state->readLen = -1;
+        return;
     }
     else
     {
@@ -1026,5 +1027,6 @@ read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
              XLOG_BLCKSZ);
 
     /* number of valid bytes in the buffer */
-    return count;
+    state->readLen = count;
+    return;
 }
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index d974400d6e..16c6095179 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -114,12 +114,12 @@ check_permissions(void)
                  (errmsg("must be superuser or replication role to use replication slots"))));
 }
 
-int
+void
 logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
                              int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
 {
-    return read_local_xlog_page(state, targetPagePtr, reqLen,
-                                targetRecPtr, cur_page, pageTLI);
+    read_local_xlog_page(state, targetPagePtr, reqLen,
+                         targetRecPtr, cur_page, pageTLI);
 }
 
 /*
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 440b6aac4b..1ef952b39a 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -761,7 +761,7 @@ StartReplication(StartReplicationCmd *cmd)
  * which has to do a plain sleep/busy loop, because the walsender's latch gets
  * set every time WAL is flushed.
  */
-static int
+static void
 logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
                        XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
 {
@@ -779,7 +779,10 @@ logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int req
 
     /* fail if not (implies we are going to shut down) */
     if (flushptr < targetPagePtr + reqLen)
-        return -1;
+    {
+        state->readLen = -1;
+        return;
+    }
 
     if (targetPagePtr + XLOG_BLCKSZ <= flushptr)
         count = XLOG_BLCKSZ;    /* more than one block available */
@@ -789,7 +792,8 @@ logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int req
     /* now actually read the data, we know it's there */
     XLogRead(cur_page, targetPagePtr, XLOG_BLCKSZ);
 
-    return count;
+    state->readLen = count;
+    return;
 }
 
 /*
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 7709b96e00..c3b9e738f7 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -48,7 +48,7 @@ typedef struct XLogPageReadPrivate
     int            tliIndex;
 } XLogPageReadPrivate;
 
-static int SimpleXLogPageRead(XLogReaderState *xlogreader,
+static void SimpleXLogPageRead(XLogReaderState *xlogreader,
                    XLogRecPtr targetPagePtr,
                    int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
                    TimeLineID *pageTLI);
@@ -237,7 +237,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
 }
 
 /* XLogreader callback function, to read a WAL page */
-static int
+static void
 SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
                    int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
                    TimeLineID *pageTLI)
@@ -292,7 +292,8 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
         if (xlogreadfd < 0)
         {
             pg_log_error("could not open file \"%s\": %m", xlogfpath);
-            return -1;
+            xlogreader->readLen = -1;
+            return;
         }
     }
 
@@ -305,7 +306,8 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
     if (lseek(xlogreadfd, (off_t) targetPageOff, SEEK_SET) < 0)
     {
         pg_log_error("could not seek in file \"%s\": %m", xlogfpath);
-        return -1;
+        xlogreader->readLen = -1;
+        return;
     }
 
 
@@ -318,13 +320,16 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
             pg_log_error("could not read file \"%s\": read %d of %zu",
                    xlogfpath, r, (Size) XLOG_BLCKSZ);
 
-        return -1;
+        xlogreader->readLen = -1;
+        return;
     }
 
     Assert(targetSegNo == xlogreadsegno);
 
     *pageTLI = targetHistory[private->tliIndex].tli;
-    return XLOG_BLCKSZ;
+
+    xlogreader->readLen = XLOG_BLCKSZ;
+    return;
 }
 
 /*
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index e106fb2ed1..9ad70a2f5c 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -421,7 +421,7 @@ XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
 /*
  * XLogReader read_page callback
  */
-static int
+static void
 XLogDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
                  XLogRecPtr targetPtr, char *readBuff, TimeLineID *curFileTLI)
 {
@@ -437,14 +437,16 @@ XLogDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
         else
         {
             private->endptr_reached = true;
-            return -1;
+            state->readLen = -1;
+            return;
         }
     }
 
     XLogDumpXLogRead(private->inpath, private->timeline, targetPagePtr,
                      readBuff, count);
 
-    return count;
+    state->readLen = count;
+    return;
 }
 
 /*
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 30500c35c7..268252e88f 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -30,7 +30,7 @@
 typedef struct XLogReaderState XLogReaderState;
 
 /* Function type definition for the read_page callback */
-typedef int (*XLogPageReadCB) (XLogReaderState *xlogreader,
+typedef void (*XLogPageReadCB) (XLogReaderState *xlogreader,
                                XLogRecPtr targetPagePtr,
                                int reqLen,
                                XLogRecPtr targetRecPtr,
@@ -66,6 +66,15 @@ typedef struct
     uint16        data_bufsz;
 } DecodedBkpBlock;
 
+/* internal state of XLogNeedData() */
+typedef enum xlnd_stateid
+{
+    XLND_STATE_INIT,
+    XLND_STATE_SEGHEAD,
+    XLND_STATE_PAGEHEAD,
+    XLND_STATE_PAGEFULLHEAD
+} xlnd_stateid;
+
 struct XLogReaderState
 {
     /* ----------------------------------------
@@ -120,6 +129,22 @@ struct XLogReaderState
     XLogRecPtr    EndRecPtr;        /* end+1 of last record read */
 
 
+    /* ----------------------------------------
+     * Communication with page reader
+     * readBuf is XLOG_BLCKSZ bytes, valid up to at least readLen bytes.
+     *  ----------------------------------------
+     */
+    /* parameters to page reader */
+    XLogRecPtr    loadPagePtr;    /* Pointer to the page  */
+    int            loadLen;        /* wanted length in bytes */
+    char       *readBuf;        /* buffer to store data */
+    XLogRecPtr    currRecPtr;        /* beginning of the WAL record being read */
+
+    /* return from page reader */
+    int32        readLen;        /* bytes acutually read, must be larger than
+                                 * loadLen. -1 on error. */
+    TimeLineID    readPageTLI;    /* TLI for data currently in readBuf */
+    
     /* ----------------------------------------
      * Decoded representation of current record
      *
@@ -145,17 +170,9 @@ struct XLogReaderState
      * ----------------------------------------
      */
 
-    /*
-     * Buffer for currently read page (XLOG_BLCKSZ bytes, valid up to at least
-     * readLen bytes)
-     */
-    char       *readBuf;
-    uint32        readLen;
-
-    /* last read segment, segment offset, TLI for data currently in readBuf */
+    /* last read segment and segment offset for data currently in readBuf */
     XLogSegNo    readSegNo;
     uint32        readOff;
-    TimeLineID    readPageTLI;
 
     /*
      * beginning of prior page read, and its TLI.  Doesn't necessarily
@@ -164,8 +181,6 @@ struct XLogReaderState
     XLogRecPtr    latestPagePtr;
     TimeLineID    latestPageTLI;
 
-    /* beginning of the WAL record being read. */
-    XLogRecPtr    currRecPtr;
     /* timeline to read it from, 0 if a lookup is required */
     TimeLineID    currTLI;
 
@@ -194,6 +209,13 @@ struct XLogReaderState
 
     /* Buffer to hold error message */
     char       *errormsg_buf;
+
+    /* Internal state of XLogNeedData */
+    union
+    {
+        xlnd_stateid    c;
+        void             *j;
+    } xlnd_state;
 };
 
 /* Get a new XLogReader */
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 0ab5ba62f5..5dba86b8b8 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,7 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
 extern void FreeFakeRelcacheEntry(Relation fakerel);
 
-extern int read_local_xlog_page(XLogReaderState *state,
+extern void read_local_xlog_page(XLogReaderState *state,
                      XLogRecPtr targetPagePtr, int reqLen,
                      XLogRecPtr targetRecPtr, char *cur_page,
                      TimeLineID *pageTLI);
diff --git a/src/include/replication/logicalfuncs.h b/src/include/replication/logicalfuncs.h
index 3fb7ad5d67..3d00dee067 100644
--- a/src/include/replication/logicalfuncs.h
+++ b/src/include/replication/logicalfuncs.h
@@ -11,7 +11,7 @@
 
 #include "replication/logical.h"
 
-extern int logical_read_local_xlog_page(XLogReaderState *state,
+extern void logical_read_local_xlog_page(XLogReaderState *state,
                              XLogRecPtr targetPagePtr,
                              int reqLen, XLogRecPtr targetRecPtr,
                              char *cur_page, TimeLineID *pageTLI);
-- 
2.16.3

From 7ce63df6ca10576ac0c85e64d239f98a71bf6003 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Wed, 17 Apr 2019 09:57:07 +0900
Subject: [PATCH 03/10] Change interface of XLogReadRecord

As a preparation to the second step, this patch changes the interface
of XLogReadRecord so that the function can request the callers for
reading more data.
---
 src/backend/access/transam/twophase.c          |  2 +-
 src/backend/access/transam/xlog.c              |  2 +-
 src/backend/access/transam/xlogreader.c        | 52 ++++++++++++++------------
 src/backend/replication/logical/logical.c      |  2 +-
 src/backend/replication/logical/logicalfuncs.c |  2 +-
 src/backend/replication/slotfuncs.c            |  2 +-
 src/backend/replication/walsender.c            |  2 +-
 src/bin/pg_rewind/parsexlog.c                  |  6 +--
 src/bin/pg_waldump/pg_waldump.c                |  2 +-
 src/include/access/xlogreader.h                | 11 +++++-
 10 files changed, 47 insertions(+), 36 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index a399c0052d..5dba27e5dd 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1394,7 +1394,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
                  errmsg("out of memory"),
                  errdetail("Failed while allocating a WAL reading processor.")));
 
-    record = XLogReadRecord(xlogreader, lsn, &errormsg);
+    XLogReadRecord(xlogreader, lsn, &record, &errormsg);
     if (record == NULL)
         ereport(ERROR,
                 (errcode_for_file_access(),
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 2130671d36..b6cb4111b8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4262,7 +4262,7 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode,
     {
         char       *errormsg;
 
-        record = XLogReadRecord(xlogreader, RecPtr, &errormsg);
+        XLogReadRecord(xlogreader, RecPtr, &record, &errormsg);
         ReadRecPtr = xlogreader->ReadRecPtr;
         EndRecPtr = xlogreader->EndRecPtr;
         if (record == NULL)
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f8375eceeb..7d9438f0ea 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -264,20 +264,21 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength)
  * If RecPtr is valid, try to read a record at that position.  Otherwise
  * try to read a record just after the last one previously read.
  *
- * If the read_page callback fails to read the requested data, NULL is
- * returned.  The callback is expected to have reported the error; errormsg
- * is set to NULL.
+ * If the read_page callback fails to read the requested data, *record is set
+ * to NULL and XLREAD_FAIL is returned.  The callback is expected to have
+ * reported the error; errormsg is set to NULL.
  *
- * If the reading fails for some other reason, NULL is also returned, and
- * *errormsg is set to a string with details of the failure.
+ * If the reading fails for some other reason, *record is also set to NULL and
+ * XLREAD_FAIL is returned. *errormsg is set to a string with details of the
+ * failure.
  *
  * The returned pointer (or *errormsg) points to an internal buffer that's
  * valid until the next call to XLogReadRecord.
  */
-XLogRecord *
-XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
+XLogReadRecordResult
+XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
+               XLogRecord **record, char **errormsg)
 {
-    XLogRecord *record;
     XLogRecPtr    targetPagePtr;
     bool        randAccess;
     uint32        len,
@@ -384,8 +385,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
      * cannot access any other fields until we've verified that we got the
      * whole header.
      */
-    record = (XLogRecord *) (state->readBuf + RecPtr % XLOG_BLCKSZ);
-    total_len = record->xl_tot_len;
+    *record = (XLogRecord *) (state->readBuf + RecPtr % XLOG_BLCKSZ);
+    total_len = (*record)->xl_tot_len;
 
     /*
      * If the whole record header is on this page, validate it immediately.
@@ -397,7 +398,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
      */
     if (targetRecOff <= XLOG_BLCKSZ - SizeOfXLogRecord)
     {
-        if (!ValidXLogRecordHeader(state, RecPtr, state->ReadRecPtr, record,
+        if (!ValidXLogRecordHeader(state, RecPtr, state->ReadRecPtr, *record,
                                    randAccess))
             goto err;
         gotheader = true;
@@ -519,9 +520,9 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
             /* If we just reassembled the record header, validate it. */
             if (!gotheader)
             {
-                record = (XLogRecord *) state->readRecordBuf;
+                *record = (XLogRecord *) state->readRecordBuf;
                 if (!ValidXLogRecordHeader(state, RecPtr, state->ReadRecPtr,
-                                           record, randAccess))
+                                           *record, randAccess))
                     goto err;
                 gotheader = true;
             }
@@ -529,8 +530,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
 
         Assert(gotheader);
 
-        record = (XLogRecord *) state->readRecordBuf;
-        if (!ValidXLogRecord(state, record, RecPtr))
+        *record = (XLogRecord *) state->readRecordBuf;
+        if (!ValidXLogRecord(state, *record, RecPtr))
             goto err;
 
         pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) state->readBuf);
@@ -551,7 +552,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
             goto err;
 
         /* Record does not cross a page boundary */
-        if (!ValidXLogRecord(state, record, RecPtr))
+        if (!ValidXLogRecord(state, *record, RecPtr))
             goto err;
 
         state->EndRecPtr = RecPtr + MAXALIGN(total_len);
@@ -562,18 +563,19 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, char **errormsg)
     /*
      * Special processing if it's an XLOG SWITCH record
      */
-    if (record->xl_rmid == RM_XLOG_ID &&
-        (record->xl_info & ~XLR_INFO_MASK) == XLOG_SWITCH)
+    if ((*record)->xl_rmid == RM_XLOG_ID &&
+        ((*record)->xl_info & ~XLR_INFO_MASK) == XLOG_SWITCH)
     {
         /* Pretend it extends to end of segment */
         state->EndRecPtr += state->wal_segment_size - 1;
         state->EndRecPtr -= XLogSegmentOffset(state->EndRecPtr, state->wal_segment_size);
     }
 
-    if (DecodeXLogRecord(state, record, errormsg))
-        return record;
-    else
-        return NULL;
+    if (DecodeXLogRecord(state, *record, errormsg))
+        return XLREAD_SUCCESS;
+
+    *record = NULL;
+    return XLREAD_FAIL;
 
 err:
 
@@ -586,7 +588,8 @@ err:
     if (state->errormsg_buf[0] != '\0')
         *errormsg = state->errormsg_buf;
 
-    return NULL;
+    *record = NULL;
+    return XLREAD_FAIL;
 }
 
 /*
@@ -990,6 +993,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
     XLogRecPtr    tmpRecPtr;
     XLogRecPtr    found = InvalidXLogRecPtr;
     XLogPageHeader header;
+    XLogRecord *record;
     char       *errormsg;
 
     Assert(!XLogRecPtrIsInvalid(RecPtr));
@@ -1078,7 +1082,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
      * because either we're at the first record after the beginning of a page
      * or we just jumped over the remaining data of a continuation.
      */
-    while (XLogReadRecord(state, tmpRecPtr, &errormsg) != NULL)
+    while (XLogReadRecord(state, tmpRecPtr, &record, &errormsg) == XLREAD_SUCCESS)
     {
         /* continue after the record */
         tmpRecPtr = InvalidXLogRecPtr;
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index acb4d9a106..d86feb0a0e 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -481,7 +481,7 @@ DecodingContextFindStartpoint(LogicalDecodingContext *ctx)
         char       *err = NULL;
 
         /* the read_page callback waits for new WAL */
-        record = XLogReadRecord(ctx->reader, startptr, &err);
+        XLogReadRecord(ctx->reader, startptr, &record, &err);
         if (err)
             elog(ERROR, "%s", err);
         if (!record)
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 16c6095179..6fc78d7445 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -289,7 +289,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
             XLogRecord *record;
             char       *errm = NULL;
 
-            record = XLogReadRecord(ctx->reader, startptr, &errm);
+            XLogReadRecord(ctx->reader, startptr, &record, &errm);
             if (errm)
                 elog(ERROR, "%s", errm);
 
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 182fe5bc82..d261b402eb 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -433,7 +433,7 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto)
              * Read records.  No changes are generated in fast_forward mode,
              * but snapbuilder/slot statuses are updated properly.
              */
-            record = XLogReadRecord(ctx->reader, startlsn, &errm);
+            XLogReadRecord(ctx->reader, startlsn, &record, &errm);
             if (errm)
                 elog(ERROR, "%s", errm);
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 1ef952b39a..7ca1536eaf 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2821,7 +2821,7 @@ XLogSendLogical(void)
      */
     WalSndCaughtUp = false;
 
-    record = XLogReadRecord(logical_decoding_ctx->reader, logical_startptr, &errm);
+    XLogReadRecord(logical_decoding_ctx->reader, logical_startptr, &record, &errm);
     logical_startptr = InvalidXLogRecPtr;
 
     /* xlog record was invalid */
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index c3b9e738f7..b110559e63 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -76,7 +76,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
 
     do
     {
-        record = XLogReadRecord(xlogreader, startpoint, &errormsg);
+        XLogReadRecord(xlogreader, startpoint, &record, &errormsg);
 
         if (record == NULL)
         {
@@ -128,7 +128,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
-    record = XLogReadRecord(xlogreader, ptr, &errormsg);
+    XLogReadRecord(xlogreader, ptr, &record, &errormsg);
     if (record == NULL)
     {
         if (errormsg)
@@ -191,7 +191,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
     {
         uint8        info;
 
-        record = XLogReadRecord(xlogreader, searchptr, &errormsg);
+        XLogReadRecord(xlogreader, searchptr, &record, &errormsg);
 
         if (record == NULL)
         {
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 9ad70a2f5c..280e4754ca 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1125,7 +1125,7 @@ main(int argc, char **argv)
     for (;;)
     {
         /* try to read the next record */
-        record = XLogReadRecord(xlogreader_state, first_record, &errormsg);
+        XLogReadRecord(xlogreader_state, first_record, &record, &errormsg);
         if (!record)
         {
             if (!config.follow || private.endptr_reached)
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 268252e88f..c8bd7afebe 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -66,6 +66,13 @@ typedef struct
     uint16        data_bufsz;
 } DecodedBkpBlock;
 
+/* Return code from XLogReadRecord */
+typedef enum XLogReadRecordResult
+{
+    XLREAD_SUCCESS,        /* record is successfully read */
+    XLREAD_FAIL            /* failed during reading a record */
+} XLogReadRecordResult;
+
 /* internal state of XLogNeedData() */
 typedef enum xlnd_stateid
 {
@@ -227,8 +234,8 @@ extern XLogReaderState *XLogReaderAllocate(int wal_segment_size,
 extern void XLogReaderFree(XLogReaderState *state);
 
 /* Read the next XLog record. Returns NULL on end-of-WAL or failure */
-extern struct XLogRecord *XLogReadRecord(XLogReaderState *state,
-               XLogRecPtr recptr, char **errormsg);
+extern XLogReadRecordResult XLogReadRecord(XLogReaderState *state,
+                    XLogRecPtr recptr, XLogRecord **record, char **errormsg);
 
 /* Validate a page */
 extern bool XLogReaderValidatePageHeader(XLogReaderState *state,
-- 
2.16.3

From d0993b0c0ea3a1a4e65ed2b83a51d192f81b7c39 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Wed, 17 Apr 2019 09:55:58 +0900
Subject: [PATCH 04/10] Make XLogReadRecord a state machine

This patch moves the caller sites of the callback above XLogReadRecord
by making XLogReadRecord a state machine.
---
 src/backend/access/transam/twophase.c          |   8 +-
 src/backend/access/transam/xlog.c              |  10 ++-
 src/backend/access/transam/xlogreader.c        | 109 ++++++++++++++++---------
 src/backend/replication/logical/logical.c      |  10 ++-
 src/backend/replication/logical/logicalfuncs.c |  10 ++-
 src/backend/replication/slotfuncs.c            |  10 ++-
 src/backend/replication/walsender.c            |  10 ++-
 src/bin/pg_rewind/parsexlog.c                  |  28 ++++++-
 src/bin/pg_waldump/pg_waldump.c                |  11 ++-
 src/include/access/xlogreader.h                |  20 ++++-
 10 files changed, 175 insertions(+), 51 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 5dba27e5dd..a3573ad0af 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1394,7 +1394,13 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
                  errmsg("out of memory"),
                  errdetail("Failed while allocating a WAL reading processor.")));
 
-    XLogReadRecord(xlogreader, lsn, &record, &errormsg);
+    while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
+           XLREAD_NEED_DATA)
+            xlogreader->read_page(xlogreader,
+                                  xlogreader->loadPagePtr, xlogreader->loadLen,
+                                  xlogreader->currRecPtr, xlogreader->readBuf,
+                                  &xlogreader->readPageTLI);
+
     if (record == NULL)
         ereport(ERROR,
                 (errcode_for_file_access(),
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b6cb4111b8..59fd12153a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4258,11 +4258,19 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode,
     /* This is the first attempt to read this page. */
     lastSourceFailed = false;
 
+    XLREAD_RESET(xlogreader);
+
     for (;;)
     {
         char       *errormsg;
 
-        XLogReadRecord(xlogreader, RecPtr, &record, &errormsg);
+        while (XLogReadRecord(xlogreader, RecPtr, &record, &errormsg)
+               == XLREAD_NEED_DATA)
+            xlogreader->read_page(xlogreader,
+                                  xlogreader->loadPagePtr, xlogreader->loadLen,
+                                  xlogreader->currRecPtr, xlogreader->readBuf,
+                                  &xlogreader->readPageTLI);
+
         ReadRecPtr = xlogreader->ReadRecPtr;
         EndRecPtr = xlogreader->EndRecPtr;
         if (record == NULL)
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 7d9438f0ea..05a57a1ebd 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -261,31 +261,56 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength)
 /*
  * Attempt to read an XLOG record.
  *
- * If RecPtr is valid, try to read a record at that position.  Otherwise
- * try to read a record just after the last one previously read.
+ * This function runs a state machine and may need to call several times until
+ * a record is read.
  *
- * If the read_page callback fails to read the requested data, *record is set
- * to NULL and XLREAD_FAIL is returned.  The callback is expected to have
- * reported the error; errormsg is set to NULL.
+ * At the initial state, if called with valid pRecPtr, try to read a
+ * record at that position.  Otherwise try to read a record just after
+ * the last one previously read.
  *
- * If the reading fails for some other reason, *record is also set to NULL and
- * XLREAD_FAIL is returned. *errormsg is set to a string with details of the
- * failure.
+ * When a record is successfully read, returns XLREAD_SUCCESS and the
+ * result record is stored in *record then reset to initial state.
+ *
+ * Returns XLREAD_NEED_DATA if more data is needed. The caller shall
+ * read some more XLOG data into readBuf and call this function again.
+ * In that case loadPagePtr and loadLen in state is set to inform the
+ * required WAL data. The caller shall read in the requested data into
+ * readBuf and set readLen and readPageTLI to the length of the data
+ * actually read and the TLI for the data read in respectively. In
+ * case of failure readLen shall be set to -1 to inform error and
+ * store error message in errormsg_buf.
+ *
+ * If the reading fails for some other reason, *record is also set to
+ * NULL and XLREAD_FAIL is returned. *errormsg is set to a string with
+ * details of the failure. Reset to initial state.
  *
  * The returned pointer (or *errormsg) points to an internal buffer that's
  * valid until the next call to XLogReadRecord.
  */
 XLogReadRecordResult
-XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
+XLogReadRecord(XLogReaderState *state, XLogRecPtr pRecPtr,
                XLogRecord **record, char **errormsg)
 {
-    XLogRecPtr    targetPagePtr;
-    bool        randAccess;
-    uint32        len,
-                total_len;
-    uint32        targetRecOff;
-    uint32        pageHeaderSize;
-    bool        gotheader;
+    /*
+     * This function is a state machine that can exit and reenter at any place
+     * marked as XLR_LEAVE. All internal state need to be preserved through
+     * multiple calls.
+     */
+    static XLogRecPtr    targetPagePtr;
+    static bool            randAccess;
+    static uint32        len,
+                        total_len;
+    static uint32        targetRecOff;
+    static uint32        pageHeaderSize;
+    static bool            gotheader;
+    static XLogRecPtr    RecPtr;
+
+#define XLR_STATE state->xlread_state
+#define XLR_INIT_STATE XLREAD_STATE_INIT
+
+    XLR_SWITCH();
+
+    RecPtr = pRecPtr;
 
     /*
      * randAccess indicates whether to verify the previous-record pointer of
@@ -339,10 +364,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
      */
     while (XLogNeedData(state, targetPagePtr,
                         Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ)))
-        state->read_page(state, state->loadPagePtr, state->loadLen,
-                         state->currRecPtr, state->readBuf,
-                         &state->readPageTLI);
-    
+        XLR_LEAVE(XLREAD_STATE_READ_PAGE, XLREAD_NEED_DATA);
+
     if (state->readLen < 0)
         goto err;
 
@@ -421,10 +444,10 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
     if (total_len > len)
     {
         /* Need to reassemble record */
-        char       *contdata;
-        XLogPageHeader pageHeader;
-        char       *buffer;
-        uint32        gotlen;
+        static char       *contdata;
+        static XLogPageHeader pageHeader;
+        static char       *buffer;
+        static uint32    gotlen;
 
         /*
          * Enlarge readRecordBuf as needed.
@@ -454,9 +477,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
             while (XLogNeedData(state, targetPagePtr,
                                 Min(total_len - gotlen + SizeOfXLogShortPHD,
                                     XLOG_BLCKSZ)))
-                state->read_page(state, state->loadPagePtr, state->loadLen,
-                                 state->currRecPtr, state->readBuf,
-                                 &state->readPageTLI);
+                XLR_LEAVE(XLREAD_STATE_READ_NEXT_PAGE, XLREAD_NEED_DATA);
 
             if (state->readLen < 0)
                 goto err;
@@ -493,9 +514,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
             if (state->readLen < pageHeaderSize)
             {
                 while (XLogNeedData(state, targetPagePtr, pageHeaderSize))
-                    state->read_page(state, state->loadPagePtr, state->loadLen,
-                                     state->currRecPtr, state->readBuf,
-                                     &state->readPageTLI);
+                    XLR_LEAVE(XLREAD_STATE_READ_PAGE_HADER, XLREAD_NEED_DATA);
             }
 
             Assert(pageHeaderSize <= state->readLen);
@@ -508,9 +527,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
             if (state->readLen < pageHeaderSize + len)
             {
                 if (XLogNeedData(state, targetPagePtr, pageHeaderSize + len))
-                    state->read_page(state, state->loadPagePtr, state->loadLen,
-                                     state->currRecPtr, state->readBuf,
-                                     &state->readPageTLI);
+                    XLR_LEAVE(XLREAD_STATE_READ_CONTRECORD, XLREAD_NEED_DATA);
             }
 
             memcpy(buffer, (char *) contdata, len);
@@ -544,9 +561,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
         /* Wait for the record data to become available */
         while (XLogNeedData(state, targetPagePtr,
                             Min(targetRecOff + total_len, XLOG_BLCKSZ)))
-            state->read_page(state, state->loadPagePtr, state->loadLen,
-                             state->currRecPtr, state->readBuf,
-                             &state->readPageTLI);
+            XLR_LEAVE(XLREAD_STATE_READ_RECORD, XLREAD_NEED_DATA);
 
         if (state->readLen < 0)
             goto err;
@@ -560,6 +575,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
         state->ReadRecPtr = RecPtr;
     }
 
+    XLR_SWITCH_END();
+
     /*
      * Special processing if it's an XLOG SWITCH record
      */
@@ -572,10 +589,10 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
     }
 
     if (DecodeXLogRecord(state, *record, errormsg))
-        return XLREAD_SUCCESS;
+        XLR_RETURN(XLREAD_SUCCESS);
 
     *record = NULL;
-    return XLREAD_FAIL;
+    XLR_RETURN(XLREAD_FAIL);
 
 err:
 
@@ -589,7 +606,7 @@ err:
         *errormsg = state->errormsg_buf;
 
     *record = NULL;
-    return XLREAD_FAIL;
+    XLR_RETURN(XLREAD_FAIL);
 }
 
 /*
@@ -615,6 +632,8 @@ XLogNeedData(XLogReaderState *state, XLogRecPtr pageptr, int reqLen)
     static XLogSegNo    targetSegNo;
     static XLogPageHeader hdr;
 
+#undef XLR_STATE
+#undef XLR_INIT_STATE
 #define XLR_STATE state->xlnd_state
 #define XLR_INIT_STATE XLND_STATE_INIT
 
@@ -994,6 +1013,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
     XLogRecPtr    found = InvalidXLogRecPtr;
     XLogPageHeader header;
     XLogRecord *record;
+    XLogReadRecordResult result;
     char       *errormsg;
 
     Assert(!XLogRecPtrIsInvalid(RecPtr));
@@ -1082,8 +1102,17 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
      * because either we're at the first record after the beginning of a page
      * or we just jumped over the remaining data of a continuation.
      */
-    while (XLogReadRecord(state, tmpRecPtr, &record, &errormsg) == XLREAD_SUCCESS)
+    while ((result = XLogReadRecord(state, tmpRecPtr, &record, &errormsg)) !=
+           XLREAD_FAIL)
     {
+        if (result == XLREAD_NEED_DATA)
+        {
+            state->read_page(state, state->loadPagePtr, state->loadLen,
+                             state->currRecPtr,    state->readBuf,
+                             &state->readPageTLI);
+            continue;
+        }
+
         /* continue after the record */
         tmpRecPtr = InvalidXLogRecPtr;
 
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index d86feb0a0e..1740753b76 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -481,7 +481,15 @@ DecodingContextFindStartpoint(LogicalDecodingContext *ctx)
         char       *err = NULL;
 
         /* the read_page callback waits for new WAL */
-        XLogReadRecord(ctx->reader, startptr, &record, &err);
+        while (XLogReadRecord(ctx->reader, startptr, &record, &err) ==
+               XLREAD_NEED_DATA)
+            ctx->reader->read_page(ctx->reader,
+                                   ctx->reader->loadPagePtr,
+                                   ctx->reader->loadLen,
+                                   ctx->reader->currRecPtr,
+                                   ctx->reader->readBuf,
+                                   &ctx->reader->readPageTLI);
+
         if (err)
             elog(ERROR, "%s", err);
         if (!record)
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 6fc78d7445..4d09255504 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -289,7 +289,15 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
             XLogRecord *record;
             char       *errm = NULL;
 
-            XLogReadRecord(ctx->reader, startptr, &record, &errm);
+            while (XLogReadRecord(ctx->reader, startptr, &record, &errm) ==
+                   XLREAD_NEED_DATA)
+                ctx->reader->read_page(ctx->reader,
+                                       ctx->reader->loadPagePtr,
+                                       ctx->reader->loadLen,
+                                       ctx->reader->currRecPtr,
+                                       ctx->reader->readBuf,
+                                       &ctx->reader->readPageTLI);
+
             if (errm)
                 elog(ERROR, "%s", errm);
 
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index d261b402eb..4a8952931d 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -433,7 +433,15 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto)
              * Read records.  No changes are generated in fast_forward mode,
              * but snapbuilder/slot statuses are updated properly.
              */
-            XLogReadRecord(ctx->reader, startlsn, &record, &errm);
+            while (XLogReadRecord(ctx->reader, startlsn, &record, &errm) ==
+                   XLREAD_NEED_DATA)
+                ctx->reader->read_page(ctx->reader,
+                                       ctx->reader->loadPagePtr,
+                                       ctx->reader->loadLen,
+                                       ctx->reader->currRecPtr,
+                                       ctx->reader->readBuf,
+                                       &ctx->reader->readPageTLI);
+
             if (errm)
                 elog(ERROR, "%s", errm);
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 7ca1536eaf..cb85ba3abf 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2821,7 +2821,15 @@ XLogSendLogical(void)
      */
     WalSndCaughtUp = false;
 
-    XLogReadRecord(logical_decoding_ctx->reader, logical_startptr, &record, &errm);
+    while (XLogReadRecord(logical_decoding_ctx->reader,
+                          logical_startptr, &record, &errm) == XLREAD_NEED_DATA)
+        logical_decoding_ctx->reader->read_page(logical_decoding_ctx->reader,
+                                   logical_decoding_ctx->reader->loadPagePtr,
+                                   logical_decoding_ctx->reader->loadLen,
+                                   logical_decoding_ctx->reader->currRecPtr,
+                                   logical_decoding_ctx->reader->readBuf,
+                                   &logical_decoding_ctx->reader->readPageTLI);
+
     logical_startptr = InvalidXLogRecPtr;
 
     /* xlog record was invalid */
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index b110559e63..21b638ed34 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -76,7 +76,14 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
 
     do
     {
-        XLogReadRecord(xlogreader, startpoint, &record, &errormsg);
+        while (XLogReadRecord(xlogreader, startpoint, &record, &errormsg) ==
+               XLREAD_NEED_DATA)
+            xlogreader->read_page(xlogreader,
+                                  xlogreader->loadPagePtr,
+                                  xlogreader->loadLen,
+                                  xlogreader->currRecPtr,
+                                  xlogreader->readBuf,
+                                  &xlogreader->readPageTLI);
 
         if (record == NULL)
         {
@@ -128,7 +135,15 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
-    XLogReadRecord(xlogreader, ptr, &record, &errormsg);
+    while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
+           XLREAD_NEED_DATA)
+                xlogreader->read_page(xlogreader,
+                                      xlogreader->loadPagePtr,
+                                      xlogreader->loadLen,
+                                      xlogreader->currRecPtr,
+                                      xlogreader->readBuf,
+                                      &xlogreader->readPageTLI);
+
     if (record == NULL)
     {
         if (errormsg)
@@ -191,7 +206,14 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
     {
         uint8        info;
 
-        XLogReadRecord(xlogreader, searchptr, &record, &errormsg);
+        while (XLogReadRecord(xlogreader, searchptr, &record, &errormsg) ==
+               XLREAD_NEED_DATA)
+            xlogreader->read_page(xlogreader,
+                                  xlogreader->loadPagePtr,
+                                  xlogreader->loadLen,
+                                  xlogreader->currRecPtr,
+                                  xlogreader->readBuf,
+                                  &xlogreader->readPageTLI);
 
         if (record == NULL)
         {
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 280e4754ca..acee7ae199 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1125,7 +1125,16 @@ main(int argc, char **argv)
     for (;;)
     {
         /* try to read the next record */
-        XLogReadRecord(xlogreader_state, first_record, &record, &errormsg);
+        while (XLogReadRecord(xlogreader_state,
+                              first_record, &record, &errormsg) ==
+               XLREAD_NEED_DATA)
+            xlogreader_state->read_page(xlogreader_state,
+                                        xlogreader_state->loadPagePtr,
+                                        xlogreader_state->loadLen,
+                                        xlogreader_state->currRecPtr,
+                                        xlogreader_state->readBuf,
+                                        &xlogreader_state->readPageTLI);
+
         if (!record)
         {
             if (!config.follow || private.endptr_reached)
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index c8bd7afebe..338dc2c14d 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -70,6 +70,7 @@ typedef struct
 typedef enum XLogReadRecordResult
 {
     XLREAD_SUCCESS,        /* record is successfully read */
+    XLREAD_NEED_DATA,    /* need more data. see XLogReadRecord. */
     XLREAD_FAIL            /* failed during reading a record */
 } XLogReadRecordResult;
 
@@ -82,6 +83,17 @@ typedef enum xlnd_stateid
     XLND_STATE_PAGEFULLHEAD
 } xlnd_stateid;
 
+/* internal state of XLogReadRecord() */
+typedef enum xlread_stateid
+{
+    XLREAD_STATE_INIT,
+    XLREAD_STATE_READ_PAGE,
+    XLREAD_STATE_READ_NEXT_PAGE,
+    XLREAD_STATE_READ_PAGE_HADER,
+    XLREAD_STATE_READ_CONTRECORD,
+    XLREAD_STATE_READ_RECORD
+} xlread_stateid;
+
 struct XLogReaderState
 {
     /* ----------------------------------------
@@ -217,12 +229,18 @@ struct XLogReaderState
     /* Buffer to hold error message */
     char       *errormsg_buf;
 
-    /* Internal state of XLogNeedData */
+    /* Internal state of XLogNeedData and XLogReadRecord */
     union
     {
         xlnd_stateid    c;
         void             *j;
     } xlnd_state;
+
+    union
+    {
+        xlread_stateid c;
+        void           *j;
+    } xlread_state;
 };
 
 /* Get a new XLogReader */
-- 
2.16.3

From 39538903779a919cd76aba297b4f1e2350bb4a44 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Apr 2019 13:48:25 +0900
Subject: [PATCH 05/10] Make XLogPageRead not use callback but call the
 function directly

This patch replaces the call to the callback in XLogReadRecrod with
direct call to the original function. Then invalidate the parameters
callback and private for XLogReaderAllocate.
---
 src/backend/access/transam/xlog.c | 44 +++++++++++++--------------------------
 1 file changed, 14 insertions(+), 30 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 59fd12153a..ba75ecb69c 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -804,13 +804,6 @@ static XLogSource readSource = 0;    /* XLOG_FROM_* code */
 static XLogSource currentSource = 0;    /* XLOG_FROM_* code */
 static bool lastSourceFailed = false;
 
-typedef struct XLogPageReadPrivate
-{
-    int            emode;
-    bool        fetching_ckpt;    /* are we fetching a checkpoint record? */
-    bool        randAccess;
-} XLogPageReadPrivate;
-
 /*
  * These variables track when we last obtained some WAL data to process,
  * and where we got it from.  (XLogReceiptSource is initially the same as
@@ -885,9 +878,8 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
 static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
              int source, bool notfoundOk);
 static int    XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source);
-static void XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
-             int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
-             TimeLineID *readTLI);
+static void XLogPageRead(XLogReaderState *xlogreader,
+                         bool fetching_ckpt, int emode, bool randAccess);
 static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
                             bool fetching_ckpt, XLogRecPtr tliRecPtr);
 static int    emode_for_corrupt_record(int emode, XLogRecPtr RecPtr);
@@ -4248,12 +4240,7 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode,
            bool fetching_ckpt)
 {
     XLogRecord *record;
-    XLogPageReadPrivate *private = (XLogPageReadPrivate *) xlogreader->private_data;
-
-    /* Pass through parameters to XLogPageRead */
-    private->fetching_ckpt = fetching_ckpt;
-    private->emode = emode;
-    private->randAccess = (RecPtr != InvalidXLogRecPtr);
+    bool        randAccess = (RecPtr != InvalidXLogRecPtr);
 
     /* This is the first attempt to read this page. */
     lastSourceFailed = false;
@@ -4266,10 +4253,7 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode,
 
         while (XLogReadRecord(xlogreader, RecPtr, &record, &errormsg)
                == XLREAD_NEED_DATA)
-            xlogreader->read_page(xlogreader,
-                                  xlogreader->loadPagePtr, xlogreader->loadLen,
-                                  xlogreader->currRecPtr, xlogreader->readBuf,
-                                  &xlogreader->readPageTLI);
+            XLogPageRead(xlogreader, fetching_ckpt, emode, randAccess);
 
         ReadRecPtr = xlogreader->ReadRecPtr;
         EndRecPtr = xlogreader->EndRecPtr;
@@ -6205,7 +6189,6 @@ StartupXLOG(void)
     bool        backupFromStandby = false;
     DBState        dbstate_at_startup;
     XLogReaderState *xlogreader;
-    XLogPageReadPrivate private;
     bool        fast_promoted = false;
     struct stat st;
 
@@ -6346,8 +6329,7 @@ StartupXLOG(void)
         OwnLatch(&XLogCtl->recoveryWakeupLatch);
 
     /* Set up XLOG reader facility */
-    MemSet(&private, 0, sizeof(XLogPageReadPrivate));
-    xlogreader = XLogReaderAllocate(wal_segment_size, &XLogPageRead, &private);
+    xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
     if (!xlogreader)
         ereport(ERROR,
                 (errcode(ERRCODE_OUT_OF_MEMORY),
@@ -11516,12 +11498,14 @@ CancelBackup(void)
  * sleep and retry.
  */
 static void
-XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen,
-             XLogRecPtr targetRecPtr, char *readBuf, TimeLineID *readTLI)
+XLogPageRead(XLogReaderState *xlogreader,
+             bool fetching_ckpt, int emode, bool randAccess)
 {
-    XLogPageReadPrivate *private =
-    (XLogPageReadPrivate *) xlogreader->private_data;
-    int            emode = private->emode;
+    XLogRecPtr targetPagePtr    = xlogreader->loadPagePtr;
+    int reqLen                    = xlogreader->loadLen;
+    XLogRecPtr targetRecPtr        = xlogreader->currRecPtr;
+    char *readBuf                = xlogreader->readBuf;
+    TimeLineID *readTLI            = &xlogreader->readPageTLI;
     uint32        targetPageOff;
     XLogSegNo    targetSegNo PG_USED_FOR_ASSERTS_ONLY;
     int            r;
@@ -11564,8 +11548,8 @@ retry:
          receivedUpto < targetPagePtr + reqLen))
     {
         if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen,
-                                         private->randAccess,
-                                         private->fetching_ckpt,
+                                         randAccess,
+                                         fetching_ckpt,
                                          targetRecPtr))
         {
             if (readFile >= 0)
-- 
2.16.3

From 5aa9744854c4ed044526ac752fa982d3af4e4f0a Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Apr 2019 15:02:19 +0900
Subject: [PATCH 06/10] Make XlogReadTwoPhaseData not use callback but call the
 function directly

This patch replaces the call to the callback in XlogReadTwoPhaseData
with direct call to the original function. Then invalidate the
parameters callback and private for XLogReaderAllocate.
---
 src/backend/access/transam/twophase.c          | 8 ++------
 src/backend/access/transam/xlogutils.c         | 8 +++++---
 src/backend/replication/logical/logicalfuncs.c | 8 ++++++--
 src/include/access/xlogutils.h                 | 5 +----
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index a3573ad0af..caaa09d785 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,8 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
     XLogReaderState *xlogreader;
     char       *errormsg;
 
-    xlogreader = XLogReaderAllocate(wal_segment_size, &read_local_xlog_page,
-                                    NULL);
+    xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
     if (!xlogreader)
         ereport(ERROR,
                 (errcode(ERRCODE_OUT_OF_MEMORY),
@@ -1396,10 +1395,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
 
     while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
            XLREAD_NEED_DATA)
-            xlogreader->read_page(xlogreader,
-                                  xlogreader->loadPagePtr, xlogreader->loadLen,
-                                  xlogreader->currRecPtr, xlogreader->readBuf,
-                                  &xlogreader->readPageTLI);
+        read_local_xlog_page(xlogreader);
 
     if (record == NULL)
         ereport(ERROR,
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c853e1f0e3..fd461f16fc 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -908,10 +908,12 @@ XLogReadDetermineTimeline(XLogReaderState *state, XLogRecPtr wantPage, uint32 wa
  * loop for now.
  */
 void
-read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
-                     int reqLen, XLogRecPtr targetRecPtr, char *cur_page,
-                     TimeLineID *pageTLI)
+read_local_xlog_page(XLogReaderState *state)
 {
+    XLogRecPtr    targetPagePtr = state->loadPagePtr;
+    int            reqLen          = state->loadLen;
+    char       *cur_page      = state->readBuf;
+    TimeLineID *pageTLI          = &state->readPageTLI;
     XLogRecPtr    read_upto,
                 loc;
     int            count;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 4d09255504..240a375d8f 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -118,8 +118,12 @@ void
 logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
                              int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
 {
-    read_local_xlog_page(state, targetPagePtr, reqLen,
-                         targetRecPtr, cur_page, pageTLI);
+    Assert(targetPagePtr == state->loadPagePtr &&
+           reqLen == state->loadLen &&
+           targetRecPtr == state->currRecPtr &&
+           cur_page == state->readBuf &&
+           pageTLI == &state->readPageTLI);
+    read_local_xlog_page(state);
 }
 
 /*
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 5dba86b8b8..7f119837ce 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -47,10 +47,7 @@ extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
 extern void FreeFakeRelcacheEntry(Relation fakerel);
 
-extern void read_local_xlog_page(XLogReaderState *state,
-                     XLogRecPtr targetPagePtr, int reqLen,
-                     XLogRecPtr targetRecPtr, char *cur_page,
-                     TimeLineID *pageTLI);
+extern void read_local_xlog_page(XLogReaderState *state);
 
 extern void XLogReadDetermineTimeline(XLogReaderState *state,
                           XLogRecPtr wantPage, uint32 wantLength);
-- 
2.16.3

From 2ad57f89065d769ea84a543eefed06d18543010b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Apr 2019 15:35:10 +0900
Subject: [PATCH 07/10] Make logical rep stuff not use callback but call the
 function directly

This is a bit different from the two before. This patch moves the
callback from XLogReaderState to LogicalDecodingContext. Then
invalidate the parameters callback and private for XLogReaderAllocate.
---
 src/backend/replication/logical/logical.c      | 19 +++++++++----------
 src/backend/replication/logical/logicalfuncs.c | 19 +++++--------------
 src/backend/replication/slotfuncs.c            |  9 +++------
 src/backend/replication/walsender.c            | 17 +++++++++--------
 src/include/replication/logical.h              | 13 +++++++++----
 src/include/replication/logicalfuncs.h         |  5 +----
 6 files changed, 36 insertions(+), 46 deletions(-)

diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 1740753b76..6dc87207b0 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -123,7 +123,7 @@ StartupDecodingContext(List *output_plugin_options,
                        TransactionId xmin_horizon,
                        bool need_full_snapshot,
                        bool fast_forward,
-                       XLogPageReadCB read_page,
+                       LogicalDecodingXLogReadPageCB read_page,
                        LogicalOutputPluginWriterPrepareWrite prepare_write,
                        LogicalOutputPluginWriterWrite do_write,
                        LogicalOutputPluginWriterUpdateProgress update_progress)
@@ -172,12 +172,14 @@ StartupDecodingContext(List *output_plugin_options,
 
     ctx->slot = slot;
 
-    ctx->reader = XLogReaderAllocate(wal_segment_size, read_page, ctx);
+    ctx->reader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
     if (!ctx->reader)
         ereport(ERROR,
                 (errcode(ERRCODE_OUT_OF_MEMORY),
                  errmsg("out of memory")));
 
+    ctx->read_page = read_page;
+
     ctx->reorder = ReorderBufferAllocate();
     ctx->snapshot_builder =
         AllocateSnapshotBuilder(ctx->reorder, xmin_horizon, start_lsn,
@@ -231,7 +233,7 @@ CreateInitDecodingContext(char *plugin,
                           List *output_plugin_options,
                           bool need_full_snapshot,
                           XLogRecPtr restart_lsn,
-                          XLogPageReadCB read_page,
+                          LogicalDecodingXLogReadPageCB read_page,
                           LogicalOutputPluginWriterPrepareWrite prepare_write,
                           LogicalOutputPluginWriterWrite do_write,
                           LogicalOutputPluginWriterUpdateProgress update_progress)
@@ -373,7 +375,7 @@ LogicalDecodingContext *
 CreateDecodingContext(XLogRecPtr start_lsn,
                       List *output_plugin_options,
                       bool fast_forward,
-                      XLogPageReadCB read_page,
+                      LogicalDecodingXLogReadPageCB read_page,
                       LogicalOutputPluginWriterPrepareWrite prepare_write,
                       LogicalOutputPluginWriterWrite do_write,
                       LogicalOutputPluginWriterUpdateProgress update_progress)
@@ -474,6 +476,8 @@ DecodingContextFindStartpoint(LogicalDecodingContext *ctx)
          (uint32) (slot->data.restart_lsn >> 32),
          (uint32) slot->data.restart_lsn);
 
+    XLREAD_RESET(ctx->reader);
+
     /* Wait for a consistent starting point */
     for (;;)
     {
@@ -483,12 +487,7 @@ DecodingContextFindStartpoint(LogicalDecodingContext *ctx)
         /* the read_page callback waits for new WAL */
         while (XLogReadRecord(ctx->reader, startptr, &record, &err) ==
                XLREAD_NEED_DATA)
-            ctx->reader->read_page(ctx->reader,
-                                   ctx->reader->loadPagePtr,
-                                   ctx->reader->loadLen,
-                                   ctx->reader->currRecPtr,
-                                   ctx->reader->readBuf,
-                                   &ctx->reader->readPageTLI);
+            ctx->read_page(ctx);
 
         if (err)
             elog(ERROR, "%s", err);
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 240a375d8f..0c4bc62cfa 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -115,15 +115,9 @@ check_permissions(void)
 }
 
 void
-logical_read_local_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr,
-                             int reqLen, XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
+logical_read_local_xlog_page(LogicalDecodingContext *ctx)
 {
-    Assert(targetPagePtr == state->loadPagePtr &&
-           reqLen == state->loadLen &&
-           targetRecPtr == state->currRecPtr &&
-           cur_page == state->readBuf &&
-           pageTLI == &state->readPageTLI);
-    read_local_xlog_page(state);
+    read_local_xlog_page(ctx->reader);
 }
 
 /*
@@ -286,6 +280,8 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
         /* invalidate non-timetravel entries */
         InvalidateSystemCaches();
 
+        XLREAD_RESET(ctx->reader);
+
         /* Decode until we run out of records */
         while ((startptr != InvalidXLogRecPtr && startptr < end_of_wal) ||
                (ctx->reader->EndRecPtr != InvalidXLogRecPtr && ctx->reader->EndRecPtr < end_of_wal))
@@ -295,12 +291,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 
             while (XLogReadRecord(ctx->reader, startptr, &record, &errm) ==
                    XLREAD_NEED_DATA)
-                ctx->reader->read_page(ctx->reader,
-                                       ctx->reader->loadPagePtr,
-                                       ctx->reader->loadLen,
-                                       ctx->reader->currRecPtr,
-                                       ctx->reader->readBuf,
-                                       &ctx->reader->readPageTLI);
+                ctx->read_page(ctx);
 
             if (errm)
                 elog(ERROR, "%s", errm);
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 4a8952931d..afdd3dea37 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -420,6 +420,8 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto)
         /* invalidate non-timetravel entries */
         InvalidateSystemCaches();
 
+        XLREAD_RESET(ctx->reader);
+
         /* Decode at least one record, until we run out of records */
         while ((!XLogRecPtrIsInvalid(startlsn) &&
                 startlsn < moveto) ||
@@ -435,12 +437,7 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto)
              */
             while (XLogReadRecord(ctx->reader, startlsn, &record, &errm) ==
                    XLREAD_NEED_DATA)
-                ctx->reader->read_page(ctx->reader,
-                                       ctx->reader->loadPagePtr,
-                                       ctx->reader->loadLen,
-                                       ctx->reader->currRecPtr,
-                                       ctx->reader->readBuf,
-                                       &ctx->reader->readPageTLI);
+                ctx->read_page(ctx);
 
             if (errm)
                 elog(ERROR, "%s", errm);
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index cb85ba3abf..5ac3ef4c66 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -762,9 +762,13 @@ StartReplication(StartReplicationCmd *cmd)
  * set every time WAL is flushed.
  */
 static void
-logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
-                       XLogRecPtr targetRecPtr, char *cur_page, TimeLineID *pageTLI)
+logical_read_xlog_page(LogicalDecodingContext *ctx)
 {
+    XLogReaderState *state = ctx->reader;
+    XLogRecPtr        targetPagePtr = state->loadPagePtr;
+    int                reqLen          = state->loadLen;
+    char           *cur_page      = state->readBuf;
+
     XLogRecPtr    flushptr;
     int            count;
 
@@ -2821,14 +2825,11 @@ XLogSendLogical(void)
      */
     WalSndCaughtUp = false;
 
+    XLREAD_RESET(logical_decoding_ctx->reader);
+
     while (XLogReadRecord(logical_decoding_ctx->reader,
                           logical_startptr, &record, &errm) == XLREAD_NEED_DATA)
-        logical_decoding_ctx->reader->read_page(logical_decoding_ctx->reader,
-                                   logical_decoding_ctx->reader->loadPagePtr,
-                                   logical_decoding_ctx->reader->loadLen,
-                                   logical_decoding_ctx->reader->currRecPtr,
-                                   logical_decoding_ctx->reader->readBuf,
-                                   &logical_decoding_ctx->reader->readPageTLI);
+        logical_decoding_ctx->read_page(logical_decoding_ctx);
 
     logical_startptr = InvalidXLogRecPtr;
 
diff --git a/src/include/replication/logical.h b/src/include/replication/logical.h
index 0a2a63a48c..70339eb8be 100644
--- a/src/include/replication/logical.h
+++ b/src/include/replication/logical.h
@@ -32,7 +32,11 @@ typedef void (*LogicalOutputPluginWriterUpdateProgress) (
                                                          TransactionId xid
 );
 
-typedef struct LogicalDecodingContext
+typedef struct LogicalDecodingContext LogicalDecodingContext;
+
+typedef void (*LogicalDecodingXLogReadPageCB)(LogicalDecodingContext *ctx);
+
+struct LogicalDecodingContext
 {
     /* memory context this is all allocated in */
     MemoryContext context;
@@ -42,6 +46,7 @@ typedef struct LogicalDecodingContext
 
     /* infrastructure pieces for decoding */
     XLogReaderState *reader;
+    LogicalDecodingXLogReadPageCB read_page;
     struct ReorderBuffer *reorder;
     struct SnapBuild *snapshot_builder;
 
@@ -89,7 +94,7 @@ typedef struct LogicalDecodingContext
     bool        prepared_write;
     XLogRecPtr    write_location;
     TransactionId write_xid;
-} LogicalDecodingContext;
+};
 
 
 extern void CheckLogicalDecodingRequirements(void);
@@ -98,7 +103,7 @@ extern LogicalDecodingContext *CreateInitDecodingContext(char *plugin,
                           List *output_plugin_options,
                           bool need_full_snapshot,
                           XLogRecPtr restart_lsn,
-                          XLogPageReadCB read_page,
+                          LogicalDecodingXLogReadPageCB read_page,
                           LogicalOutputPluginWriterPrepareWrite prepare_write,
                           LogicalOutputPluginWriterWrite do_write,
                           LogicalOutputPluginWriterUpdateProgress update_progress);
@@ -106,7 +111,7 @@ extern LogicalDecodingContext *CreateDecodingContext(
                       XLogRecPtr start_lsn,
                       List *output_plugin_options,
                       bool fast_forward,
-                      XLogPageReadCB read_page,
+                      LogicalDecodingXLogReadPageCB read_page,
                       LogicalOutputPluginWriterPrepareWrite prepare_write,
                       LogicalOutputPluginWriterWrite do_write,
                       LogicalOutputPluginWriterUpdateProgress update_progress);
diff --git a/src/include/replication/logicalfuncs.h b/src/include/replication/logicalfuncs.h
index 3d00dee067..04a9fe10fa 100644
--- a/src/include/replication/logicalfuncs.h
+++ b/src/include/replication/logicalfuncs.h
@@ -11,9 +11,6 @@
 
 #include "replication/logical.h"
 
-extern void logical_read_local_xlog_page(XLogReaderState *state,
-                             XLogRecPtr targetPagePtr,
-                             int reqLen, XLogRecPtr targetRecPtr,
-                             char *cur_page, TimeLineID *pageTLI);
+extern void logical_read_local_xlog_page(LogicalDecodingContext *ctx);
 
 #endif
-- 
2.16.3

From 8fcca6af6025eaca9e9857cdeaf8c4b39a851c34 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Apr 2019 15:50:51 +0900
Subject: [PATCH 08/10] Make pg_waldump not use callback but call the function
 directly

This patch does the similar thing to the change in logical rep. Moves
callback from XLogReaderState from the parameter of
XLogFindNextRecord. Then invalidate the parameters callback and
private for XLogReaderAllocate.
---
 src/backend/access/transam/xlogreader.c | 17 +++++++----------
 src/bin/pg_waldump/pg_waldump.c         | 21 +++++++++------------
 src/include/access/xlogreader.h         | 14 +++++---------
 3 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 05a57a1ebd..72b82a17d6 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1006,7 +1006,8 @@ XLogReaderValidatePageHeader(XLogReaderState *state, XLogRecPtr recptr,
  * debugging purposes.
  */
 XLogRecPtr
-XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
+XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr,
+                   XLogFindNextRecordCB read_page, void *private)
 {
     XLogReaderState saved_state = *state;
     XLogRecPtr    tmpRecPtr;
@@ -1018,6 +1019,8 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
 
     Assert(!XLogRecPtrIsInvalid(RecPtr));
 
+    XLREAD_RESET(state);
+
     /*
      * skip over potential continuation data, keeping in mind that it may span
      * multiple pages
@@ -1045,9 +1048,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
 
         /* Read the page containing the record */
         while(XLogNeedData(state, targetPagePtr, targetRecOff))
-            state->read_page(state, state->loadPagePtr, state->loadLen,
-                             state->currRecPtr, state->readBuf,
-                             &state->readPageTLI);
+            read_page(state, private);
 
         if (state->readLen < 0)
             goto err;
@@ -1058,9 +1059,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
 
         /* make sure we have enough data for the page header */
         while (XLogNeedData(state, targetPagePtr, pageHeaderSize))
-            state->read_page(state, state->loadPagePtr, state->loadLen,
-                             state->currRecPtr, state->readBuf,
-                             &state->readPageTLI);
+            read_page(state, private);
                
         if (state->readLen < 0)
             goto err;
@@ -1107,9 +1106,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
     {
         if (result == XLREAD_NEED_DATA)
         {
-            state->read_page(state, state->loadPagePtr, state->loadLen,
-                             state->currRecPtr,    state->readBuf,
-                             &state->readPageTLI);
+            read_page(state, private);
             continue;
         }
 
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index acee7ae199..1966da493f 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -422,10 +422,12 @@ XLogDumpXLogRead(const char *directory, TimeLineID timeline_id,
  * XLogReader read_page callback
  */
 static void
-XLogDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
-                 XLogRecPtr targetPtr, char *readBuff, TimeLineID *curFileTLI)
+XLogDumpReadPage(XLogReaderState *state, void *priv)
 {
-    XLogDumpPrivate *private = state->private_data;
+    XLogRecPtr    targetPagePtr = state->loadPagePtr;
+    int            reqLen          = state->loadLen;
+    char       *readBuff      = state->readBuf;
+    XLogDumpPrivate *private  = (XLogDumpPrivate *) priv;
     int            count = XLOG_BLCKSZ;
 
     if (private->endptr != InvalidXLogRecPtr)
@@ -1095,13 +1097,13 @@ main(int argc, char **argv)
     /* done with argument parsing, do the actual work */
 
     /* we have everything we need, start reading */
-    xlogreader_state = XLogReaderAllocate(WalSegSz, XLogDumpReadPage,
-                                          &private);
+    xlogreader_state = XLogReaderAllocate(WalSegSz, NULL, NULL);
     if (!xlogreader_state)
         fatal_error("out of memory");
 
     /* first find a valid recptr to start from */
-    first_record = XLogFindNextRecord(xlogreader_state, private.startptr);
+    first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
+                                      &XLogDumpReadPage, (void*) &private);
 
     if (first_record == InvalidXLogRecPtr)
         fatal_error("could not find a valid record after %X/%X",
@@ -1128,12 +1130,7 @@ main(int argc, char **argv)
         while (XLogReadRecord(xlogreader_state,
                               first_record, &record, &errormsg) ==
                XLREAD_NEED_DATA)
-            xlogreader_state->read_page(xlogreader_state,
-                                        xlogreader_state->loadPagePtr,
-                                        xlogreader_state->loadLen,
-                                        xlogreader_state->currRecPtr,
-                                        xlogreader_state->readBuf,
-                                        &xlogreader_state->readPageTLI);
+            XLogDumpReadPage(xlogreader_state, (void *) &private);
 
         if (!record)
         {
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 338dc2c14d..5f85c79424 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -29,14 +29,6 @@
 
 typedef struct XLogReaderState XLogReaderState;
 
-/* Function type definition for the read_page callback */
-typedef void (*XLogPageReadCB) (XLogReaderState *xlogreader,
-                               XLogRecPtr targetPagePtr,
-                               int reqLen,
-                               XLogRecPtr targetRecPtr,
-                               char *readBuf,
-                               TimeLineID *pageTLI);
-
 typedef struct
 {
     /* Is this block ref in use? */
@@ -263,7 +255,11 @@ extern bool XLogReaderValidatePageHeader(XLogReaderState *state,
 extern void XLogReaderInvalReadState(XLogReaderState *state);
 
 #ifdef FRONTEND
-extern XLogRecPtr XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr);
+/* Function type definition for the read_page callback */
+typedef void (*XLogFindNextRecordCB) (XLogReaderState *xlogreader,
+                                      void *private);
+extern XLogRecPtr XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr,
+                                     XLogFindNextRecordCB read_page, void *private);
 #endif                            /* FRONTEND */
 
 /* Functions for decoding an XLogRecord */
-- 
2.16.3

From 882d4dca9935233cbe5ed0f79845d78d691e6bb8 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Apr 2019 16:00:57 +0900
Subject: [PATCH 09/10] Make pg_rewind not use callback but call the function
 directly

This patch replaces the call to the callback in pg_rewind with direct
call to the original function. Then invalidate the parameters callback
and private for XLogReaderAllocate.
---
 src/bin/pg_rewind/parsexlog.c | 42 +++++++++++-------------------------------
 1 file changed, 11 insertions(+), 31 deletions(-)

diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 21b638ed34..d504c3227a 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -49,9 +49,7 @@ typedef struct XLogPageReadPrivate
 } XLogPageReadPrivate;
 
 static void SimpleXLogPageRead(XLogReaderState *xlogreader,
-                   XLogRecPtr targetPagePtr,
-                   int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
-                   TimeLineID *pageTLI);
+                               XLogPageReadPrivate *private);
 
 /*
  * Read WAL from the datadir/pg_wal, starting from 'startpoint' on timeline
@@ -69,8 +67,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
 
     private.datadir = datadir;
     private.tliIndex = tliIndex;
-    xlogreader = XLogReaderAllocate(WalSegSz, &SimpleXLogPageRead,
-                                    &private);
+    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
@@ -78,12 +75,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
     {
         while (XLogReadRecord(xlogreader, startpoint, &record, &errormsg) ==
                XLREAD_NEED_DATA)
-            xlogreader->read_page(xlogreader,
-                                  xlogreader->loadPagePtr,
-                                  xlogreader->loadLen,
-                                  xlogreader->currRecPtr,
-                                  xlogreader->readBuf,
-                                  &xlogreader->readPageTLI);
+            SimpleXLogPageRead(xlogreader, &private);
 
         if (record == NULL)
         {
@@ -130,19 +122,13 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
 
     private.datadir = datadir;
     private.tliIndex = tliIndex;
-    xlogreader = XLogReaderAllocate(WalSegSz, &SimpleXLogPageRead,
-                                    &private);
+    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
     while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
            XLREAD_NEED_DATA)
-                xlogreader->read_page(xlogreader,
-                                      xlogreader->loadPagePtr,
-                                      xlogreader->loadLen,
-                                      xlogreader->currRecPtr,
-                                      xlogreader->readBuf,
-                                      &xlogreader->readPageTLI);
+        SimpleXLogPageRead(xlogreader, &private);
 
     if (record == NULL)
     {
@@ -196,8 +182,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
 
     private.datadir = datadir;
     private.tliIndex = tliIndex;
-    xlogreader = XLogReaderAllocate(WalSegSz, &SimpleXLogPageRead,
-                                    &private);
+    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
@@ -208,12 +193,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
 
         while (XLogReadRecord(xlogreader, searchptr, &record, &errormsg) ==
                XLREAD_NEED_DATA)
-            xlogreader->read_page(xlogreader,
-                                  xlogreader->loadPagePtr,
-                                  xlogreader->loadLen,
-                                  xlogreader->currRecPtr,
-                                  xlogreader->readBuf,
-                                  &xlogreader->readPageTLI);
+            SimpleXLogPageRead(xlogreader, &private);
 
         if (record == NULL)
         {
@@ -260,11 +240,11 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
 
 /* XLogreader callback function, to read a WAL page */
 static void
-SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
-                   int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
-                   TimeLineID *pageTLI)
+SimpleXLogPageRead(XLogReaderState *xlogreader, XLogPageReadPrivate *private)
 {
-    XLogPageReadPrivate *private = (XLogPageReadPrivate *) xlogreader->private_data;
+    XLogRecPtr    targetPagePtr = xlogreader->loadPagePtr;
+    char       *readBuf          = xlogreader->readBuf;
+    TimeLineID *pageTLI          = &xlogreader->readPageTLI;
     uint32        targetPageOff;
     XLogRecPtr    targetSegEnd;
     XLogSegNo    targetSegNo;
-- 
2.16.3

From ef5b69b3faf496353a031d24cb151b3ebc37c45c Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 18 Apr 2019 16:15:18 +0900
Subject: [PATCH 10/10] Remove callback entry from XLogReaderState

This is the third (3/2?) step. Remove no-longer-useful members
read_page and private_data from XLogReaderState. Then change
XLogReaderAllocate not to take the parameters.
---
 src/backend/access/transam/twophase.c     |  2 +-
 src/backend/access/transam/xlog.c         |  4 ++--
 src/backend/access/transam/xlogreader.c   |  9 ++-------
 src/backend/replication/logical/logical.c |  2 +-
 src/bin/pg_rewind/parsexlog.c             |  6 +++---
 src/bin/pg_waldump/pg_waldump.c           |  2 +-
 src/include/access/xlogreader.h           | 32 +------------------------------
 7 files changed, 11 insertions(+), 46 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index caaa09d785..e70245e7cb 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1386,7 +1386,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
     XLogReaderState *xlogreader;
     char       *errormsg;
 
-    xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
+    xlogreader = XLogReaderAllocate(wal_segment_size);
     if (!xlogreader)
         ereport(ERROR,
                 (errcode(ERRCODE_OUT_OF_MEMORY),
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ba75ecb69c..f34a9ba806 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1188,7 +1188,7 @@ XLogInsertRecord(XLogRecData *rdata,
             appendBinaryStringInfo(&recordBuf, rdata->data, rdata->len);
 
         if (!debug_reader)
-            debug_reader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
+            debug_reader = XLogReaderAllocate(wal_segment_size);
 
         if (!debug_reader)
         {
@@ -6329,7 +6329,7 @@ StartupXLOG(void)
         OwnLatch(&XLogCtl->recoveryWakeupLatch);
 
     /* Set up XLOG reader facility */
-    xlogreader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
+    xlogreader = XLogReaderAllocate(wal_segment_size);
     if (!xlogreader)
         ereport(ERROR,
                 (errcode(ERRCODE_OUT_OF_MEMORY),
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 72b82a17d6..e2d7b9060b 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -128,8 +128,7 @@ report_invalid_record(XLogReaderState *state, const char *fmt,...)
  * Returns NULL if the xlogreader couldn't be allocated.
  */
 XLogReaderState *
-XLogReaderAllocate(int wal_segment_size, XLogPageReadCB pagereadfunc,
-                   void *private_data)
+XLogReaderAllocate(int wal_segment_size)
 {
     XLogReaderState *state;
 
@@ -157,11 +156,7 @@ XLogReaderAllocate(int wal_segment_size, XLogPageReadCB pagereadfunc,
     }
 
     state->wal_segment_size = wal_segment_size;
-    state->read_page = pagereadfunc;
-    /* system_identifier initialized to zeroes above */
-    state->private_data = private_data;
-    /* ReadRecPtr and EndRecPtr initialized to zeroes above */
-    /* readSegNo, readOff, readLen, readPageTLI initialized to zeroes above */
+    /* All members are initialized to zeroes above */
     state->errormsg_buf = palloc_extended(MAX_ERRORMSG_LEN + 1,
                                           MCXT_ALLOC_NO_OOM);
     if (!state->errormsg_buf)
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 6dc87207b0..e257dd8afc 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -172,7 +172,7 @@ StartupDecodingContext(List *output_plugin_options,
 
     ctx->slot = slot;
 
-    ctx->reader = XLogReaderAllocate(wal_segment_size, NULL, NULL);
+    ctx->reader = XLogReaderAllocate(wal_segment_size);
     if (!ctx->reader)
         ereport(ERROR,
                 (errcode(ERRCODE_OUT_OF_MEMORY),
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index d504c3227a..b46465fff3 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -67,7 +67,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
 
     private.datadir = datadir;
     private.tliIndex = tliIndex;
-    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
+    xlogreader = XLogReaderAllocate(WalSegSz);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
@@ -122,7 +122,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
 
     private.datadir = datadir;
     private.tliIndex = tliIndex;
-    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
+    xlogreader = XLogReaderAllocate(WalSegSz);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
@@ -182,7 +182,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
 
     private.datadir = datadir;
     private.tliIndex = tliIndex;
-    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
+    xlogreader = XLogReaderAllocate(WalSegSz);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 1966da493f..f70dc7bbc0 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1097,7 +1097,7 @@ main(int argc, char **argv)
     /* done with argument parsing, do the actual work */
 
     /* we have everything we need, start reading */
-    xlogreader_state = XLogReaderAllocate(WalSegSz, NULL, NULL);
+    xlogreader_state = XLogReaderAllocate(WalSegSz);
     if (!xlogreader_state)
         fatal_error("out of memory");
 
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 5f85c79424..f407746209 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -98,40 +98,12 @@ struct XLogReaderState
      */
     int            wal_segment_size;
 
-    /*
-     * Data input callback (mandatory).
-     *
-     * This callback shall read at least reqLen valid bytes of the xlog page
-     * starting at targetPagePtr, and store them in readBuf.  The callback
-     * shall return the number of bytes read (never more than XLOG_BLCKSZ), or
-     * -1 on failure.  The callback shall sleep, if necessary, to wait for the
-     * requested bytes to become available.  The callback will not be invoked
-     * again for the same page unless more than the returned number of bytes
-     * are needed.
-     *
-     * targetRecPtr is the position of the WAL record we're reading.  Usually
-     * it is equal to targetPagePtr + reqLen, but sometimes xlogreader needs
-     * to read and verify the page or segment header, before it reads the
-     * actual WAL record it's interested in.  In that case, targetRecPtr can
-     * be used to determine which timeline to read the page from.
-     *
-     * The callback shall set *pageTLI to the TLI of the file the page was
-     * read from.  It is currently used only for error reporting purposes, to
-     * reconstruct the name of the WAL file where an error occurred.
-     */
-    XLogPageReadCB read_page;
-
     /*
      * System identifier of the xlog files we're about to read.  Set to zero
      * (the default value) if unknown or unimportant.
      */
     uint64        system_identifier;
 
-    /*
-     * Opaque data for callbacks to use.  Not used by XLogReader.
-     */
-    void       *private_data;
-
     /*
      * Start and end point of last record read.  EndRecPtr is also used as the
      * position to read next, if XLogReadRecord receives an invalid recptr.
@@ -236,9 +208,7 @@ struct XLogReaderState
 };
 
 /* Get a new XLogReader */
-extern XLogReaderState *XLogReaderAllocate(int wal_segment_size,
-                   XLogPageReadCB pagereadfunc,
-                   void *private_data);
+extern XLogReaderState *XLogReaderAllocate(int wal_segment_size);
 
 /* Free an XLogReader */
 extern void XLogReaderFree(XLogReaderState *state);
-- 
2.16.3