Re: Remove page-read callback from XLogReaderState.

Поиск
Список
Период
Сортировка
От Kyotaro HORIGUCHI
Тема Re: Remove page-read callback from XLogReaderState.
Дата
Msg-id 20190524.115624.41405730.horiguchi.kyotaro@lab.ntt.co.jp
обсуждение исходный текст
Ответ на Re: Remove page-read callback from XLogReaderState.  (Antonin Houska <ah@cybertec.at>)
Ответы Re: Remove page-read callback from XLogReaderState.  (Antonin Houska <ah@cybertec.at>)
Re: Remove page-read callback from XLogReaderState.  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Список pgsql-hackers
Thank you for looking this, Antonin.

At Wed, 22 May 2019 13:53:23 +0200, Antonin Houska <ah@cybertec.at> wrote in <25494.1558526003@spoje.net>
> Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> 
> > Hello. Thank you for looking this.
> > ...
> > Yeah, I'll register this, maybe the week after next week.
> 
> I've checked the new version. One more thing I noticed now is that XLR_STATE.j
> is initialized to zero, either by XLogReaderAllocate() which zeroes the whole
> reader state, or later by XLREAD_RESET. This special value then needs to be
> handled here:
> 
> #define XLR_SWITCH()                    \
>     do {                        \
>         if ((XLR_STATE).j)            \
>             goto *((void *) (XLR_STATE).j);    \
>         XLR_CASE(XLR_INIT_STATE);        \
>     } while (0)
> 
> I think it's better to set the label always to (&&XLR_INIT_STATE) so that
> XLR_SWITCH can perform the jump unconditionally.

I thought the same but did not do that since label is
function-scoped so it cannot be referred outside the defined
function.

I moved the state variable from XLogReaderState into functions
static variable. It's not problem since the functions are
non-reentrant in the first place.

> Attached is also an (unrelated) comment fix proposal.

Sounds reasoable. I found another typo "acutually" there.

-    int32        readLen;        /* bytes acutually read, must be larger than
+    int32        readLen;        /* bytes acutually read, must be at least

I fixed it with other typos found.

v3-0001 : Changed macrosas suggested.

v3-0002, 0004: Fixed comments. Fixes following changes of
     macros. Renamed state symbols.

v3-0003, 0005-0010: No substantial change from v2.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 036b1a50ad79ebfde990e178bead9ba03c753d02 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
step. 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        | 180 +++++++++++++++----------
 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                |  39 ++++--
 src/include/access/xlogutils.h                 |   2 +-
 src/include/replication/logicalfuncs.h         |   2 +-
 10 files changed, 177 insertions(+), 110 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 1c7dd51b9f..d1a29fe5cd 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 baf04b9f1f..f7499aff5e 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -118,8 +118,8 @@ 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);
@@ -306,7 +306,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
@@ -358,15 +357,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)
@@ -392,8 +393,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.
@@ -470,14 +471,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;
@@ -506,20 +510,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;
@@ -550,9 +562,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 */
@@ -595,39 +611,48 @@ 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.
+ *
+ * Note: This function is not reentrant. The state is maintained internally in
+ * the function. DO NOT ereport(ERROR)-out from inside of this function.
  */
-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);
+    XLR_SWITCH (XLND_STATE_INIT);
 
     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.
      *
      * Every time we actually read the page, even if we looked at parts of it
-     * before, we need to do verification as the read_page callback might now
-     * be rereading data from a different source.
+     * before, we need to do verification as the caller might now be rereading
+     * data from a different source.
      *
      * Whenever switching to a new WAL segment, we read the first page of the
      * file and validate its header, even if that's not where the target
@@ -636,18 +661,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_SEGHEADER, 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;
     }
@@ -656,48 +680,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_PAGEHEADER, 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_PAGELONGHEADER, 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);
 }
 
 /*
@@ -996,7 +1025,6 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
         XLogRecPtr    targetPagePtr;
         int            targetRecOff;
         uint32        pageHeaderSize;
-        int            readLen;
 
         /*
          * Compute targetRecOff. It should typically be equal or greater than
@@ -1004,7 +1032,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;
@@ -1013,8 +1041,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;
@@ -1022,8 +1054,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 3f31368022..082c98b2ec 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 287af60c4e..0bfbea8f09 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -47,7 +47,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);
@@ -236,7 +236,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)
@@ -291,7 +291,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;
         }
     }
 
@@ -304,7 +305,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;
     }
 
 
@@ -317,13 +319,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 c40014d483..c748820a5f 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 04228e2a87..d3b3e4b7ba 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_SEGHEADER,
+    XLND_STATE_PAGEHEADER,
+    XLND_STATE_PAGELONGHEADER
+} 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 actually read, must be at least
+                                 * 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;
 
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 4105b59904..be0c79d18c 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 a9c178a9e6..b0306c54ab 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 94bf31fb10ce24284c27b49b6dcc206ebf9344cc Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Fri, 24 May 2019 09:22:44 +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                | 13 ++++++-
 10 files changed, 49 insertions(+), 36 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 5196d6181d..334abe7ba8 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 d1a29fe5cd..ee73576d5b 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 f7499aff5e..ee11642759 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -285,20 +285,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,
@@ -405,8 +406,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.
@@ -418,7 +419,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;
@@ -540,9 +541,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;
             }
@@ -550,8 +551,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);
@@ -572,7 +573,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);
@@ -583,18 +584,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:
 
@@ -607,7 +609,8 @@ err:
     if (state->errormsg_buf[0] != '\0')
         *errormsg = state->errormsg_buf;
 
-    return NULL;
+    *record = NULL;
+    return XLREAD_FAIL;
 }
 
 /*
@@ -1011,6 +1014,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
     XLogRecPtr    tmpRecPtr;
     XLogRecPtr    found = InvalidXLogRecPtr;
     XLogPageHeader header;
+    XLogRecord *record;
     char       *errormsg;
 
     Assert(!XLogRecPtrIsInvalid(RecPtr));
@@ -1099,7 +1103,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 bbd38c06d1..79a3ec09ff 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 808a6f5b83..7db8e0d044 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 082c98b2ec..d136a48805 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2826,7 +2826,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 0bfbea8f09..512005de1c 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -75,7 +75,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
 
     do
     {
-        record = XLogReadRecord(xlogreader, startpoint, &errormsg);
+        XLogReadRecord(xlogreader, startpoint, &record, &errormsg);
 
         if (record == NULL)
         {
@@ -127,7 +127,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)
@@ -190,7 +190,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 c748820a5f..3198627242 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 d3b3e4b7ba..0e734d27f1 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
 {
@@ -220,8 +227,10 @@ 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 6d3694958c65e1fb930bc858d75f0499af788770 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Fri, 24 May 2019 09:33:29 +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              |   8 +-
 src/backend/access/transam/xlogreader.c        | 108 ++++++++++++++++---------
 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                |  12 +++
 10 files changed, 165 insertions(+), 50 deletions(-)

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 334abe7ba8..ad0d505e54 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 ee73576d5b..5b9b1893a2 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4262,7 +4262,13 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode,
     {
         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 ee11642759..ce901c358a 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -282,31 +282,57 @@ 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.  If invalid pRecPtr is given 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 with result
+ * record being stored in *record then the state machine is reset to initial
+ * state.
+ *
+ * Returns XLREAD_NEED_DATA if needs more data fed.  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 the caller shall call the
+ * function setting readLen to -1 and storing error message in errormsg_buf to
+ * inform error.
+ *
+ * If the reading fails for some reasons including caller-side error mentioned
+ * above, returns XLREAD_FAIL with *record being set to NULL. *errormsg is set
+ * to a string with details of the failure. The state machine is reset to
+ * initial state.
  *
  * The returned pointer (or *errormsg) points to an internal buffer that's
  * valid until the next call to XLogReadRecord.
+ *
+ * Note: This function is not reentrant. The state is maintained internally in
+ * the function. DO NOT non-local exit (ereport) from inside of this function.
  */
 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 needs 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;
+
+    XLR_SWITCH(XLREAD_STATE_INIT);
+
+    RecPtr = pRecPtr;
 
     /*
      * randAccess indicates whether to verify the previous-record pointer of
@@ -360,10 +386,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_PAGE, XLREAD_NEED_DATA);
+
     if (state->readLen < 0)
         goto err;
 
@@ -442,10 +466,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.
@@ -475,9 +499,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_CONTPAGE, XLREAD_NEED_DATA);
 
             if (state->readLen < 0)
                 goto err;
@@ -514,9 +536,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_CONTPAGE_HEADER, XLREAD_NEED_DATA);
             }
 
             Assert(pageHeaderSize <= state->readLen);
@@ -529,9 +549,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_CONTRECORD, XLREAD_NEED_DATA);
             }
 
             memcpy(buffer, (char *) contdata, len);
@@ -565,9 +583,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_RECORD, XLREAD_NEED_DATA);
 
         if (state->readLen < 0)
             goto err;
@@ -581,6 +597,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr,
         state->ReadRecPtr = RecPtr;
     }
 
+    XLR_SWITCH_END();
+
     /*
      * Special processing if it's an XLOG SWITCH record
      */
@@ -593,10 +611,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:
 
@@ -610,7 +628,7 @@ err:
         *errormsg = state->errormsg_buf;
 
     *record = NULL;
-    return XLREAD_FAIL;
+    XLR_RETURN(XLREAD_FAIL);
 }
 
 /*
@@ -1015,6 +1033,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr)
     XLogRecPtr    found = InvalidXLogRecPtr;
     XLogPageHeader header;
     XLogRecord *record;
+    XLogReadRecordResult result;
     char       *errormsg;
 
     Assert(!XLogRecPtrIsInvalid(RecPtr));
@@ -1103,8 +1122,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 79a3ec09ff..108dd6f9b2 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 7db8e0d044..f4f4a907ad 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 d136a48805..e1c721bc9e 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2826,7 +2826,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 512005de1c..e26127206c 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -75,7 +75,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)
         {
@@ -127,7 +134,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)
@@ -190,7 +205,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 3198627242..54717c9320 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 0e734d27f1..bc0c642906 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_PAGELONGHEADER
 } xlnd_stateid;
 
+/* internal state of XLogReadRecord() */
+typedef enum xlread_stateid
+{
+    XLREAD_STATE_INIT,
+    XLREAD_STATE_PAGE,
+    XLREAD_STATE_CONTPAGE,
+    XLREAD_STATE_CONTPAGE_HEADER,
+    XLREAD_STATE_CONTRECORD,
+    XLREAD_STATE_RECORD
+} xlread_stateid;
+
 struct XLogReaderState
 {
     /* ----------------------------------------
-- 
2.16.3

From fce5b4fa23442e10d96dce648ce7c477476cbbe8 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 XLogReadRecord 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 5b9b1893a2..9f184eefbe 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;
@@ -4264,10 +4251,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;
@@ -6203,7 +6187,6 @@ StartupXLOG(void)
     bool        backupFromStandby = false;
     DBState        dbstate_at_startup;
     XLogReaderState *xlogreader;
-    XLogPageReadPrivate private;
     bool        fast_promoted = false;
     struct stat st;
 
@@ -6344,8 +6327,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),
@@ -11514,12 +11496,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;
@@ -11562,8 +11546,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 b9e2755f2e52f981255f1123f24922cc29e308e8 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 ad0d505e54..23424886dc 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 be0c79d18c..9724ce20b8 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 39151062558d909df02dfedd4f5183f4f62c02ed 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      | 17 +++++++----------
 src/backend/replication/logical/logicalfuncs.c | 17 +++--------------
 src/backend/replication/slotfuncs.c            |  7 +------
 src/backend/replication/walsender.c            | 15 +++++++--------
 src/include/replication/logical.h              | 13 +++++++++----
 src/include/replication/logicalfuncs.h         |  5 +----
 6 files changed, 28 insertions(+), 46 deletions(-)

diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 108dd6f9b2..767d73f476 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)
@@ -483,12 +485,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..2a3f6d3cde 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);
 }
 
 /*
@@ -295,12 +289,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 f4f4a907ad..8675c9203e 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -435,12 +435,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 e1c721bc9e..fde94a561f 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;
 
@@ -2828,12 +2832,7 @@ XLogSendLogical(void)
 
     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 f76b9eb4df..f0bec63241 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 b0306c54ab..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 19765979c323afc0f45b507fea6b3187274c6dc8 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 | 15 +++++----------
 src/bin/pg_waldump/pg_waldump.c         | 21 +++++++++------------
 src/include/access/xlogreader.h         | 14 +++++---------
 3 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index ce901c358a..0bf8dac408 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1026,7 +1026,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;
@@ -1065,9 +1066,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;
@@ -1078,9 +1077,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;
@@ -1127,9 +1124,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 54717c9320..3125633327 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 bc0c642906..b4ace71a75 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? */
@@ -252,7 +244,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 b6392972840935be4faaa09df103ee6f77a8b9a8 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 | 78 ++++++++++++-------------------------------
 1 file changed, 22 insertions(+), 56 deletions(-)

diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index e26127206c..26446027ab 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -41,16 +41,8 @@ static int    xlogreadfd = -1;
 static XLogSegNo xlogreadsegno = -1;
 static char xlogfpath[MAXPGPATH];
 
-typedef struct XLogPageReadPrivate
-{
-    const char *datadir;
-    int            tliIndex;
-} XLogPageReadPrivate;
-
-static void    SimpleXLogPageRead(XLogReaderState *xlogreader,
-                               XLogRecPtr targetPagePtr,
-                               int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
-                               TimeLineID *pageTLI);
+static void SimpleXLogPageRead(XLogReaderState *xlogreader,
+                               const char *datadir, int *tliIndex);
 
 /*
  * Read WAL from the datadir/pg_wal, starting from 'startpoint' on timeline
@@ -64,12 +56,8 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
     XLogRecord *record;
     XLogReaderState *xlogreader;
     char       *errormsg;
-    XLogPageReadPrivate private;
 
-    private.datadir = datadir;
-    private.tliIndex = tliIndex;
-    xlogreader = XLogReaderAllocate(WalSegSz, &SimpleXLogPageRead,
-                                    &private);
+    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
@@ -77,12 +65,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, datadir, &tliIndex);
 
         if (record == NULL)
         {
@@ -124,24 +107,15 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
     XLogRecord *record;
     XLogReaderState *xlogreader;
     char       *errormsg;
-    XLogPageReadPrivate private;
     XLogRecPtr    endptr;
 
-    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, datadir, &tliIndex);
 
     if (record == NULL)
     {
@@ -177,7 +151,6 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
     XLogRecPtr    searchptr;
     XLogReaderState *xlogreader;
     char       *errormsg;
-    XLogPageReadPrivate private;
 
     /*
      * The given fork pointer points to the end of the last common record,
@@ -193,10 +166,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
             forkptr += SizeOfXLogShortPHD;
     }
 
-    private.datadir = datadir;
-    private.tliIndex = tliIndex;
-    xlogreader = XLogReaderAllocate(WalSegSz, &SimpleXLogPageRead,
-                                    &private);
+    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
@@ -207,12 +177,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, datadir, &tliIndex);
 
         if (record == NULL)
         {
@@ -259,11 +224,12 @@ 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,
+                   const char*datadir, int *tliIndex)
 {
-    XLogPageReadPrivate *private = (XLogPageReadPrivate *) xlogreader->private_data;
+    XLogRecPtr    targetPagePtr = xlogreader->loadPagePtr;
+    char       *readBuf          = xlogreader->readBuf;
+    TimeLineID *pageTLI          = &xlogreader->readPageTLI;
     uint32        targetPageOff;
     XLogRecPtr    targetSegEnd;
     XLogSegNo    targetSegNo;
@@ -296,17 +262,17 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
          * be done both forward and backward, consider also switching timeline
          * accordingly.
          */
-        while (private->tliIndex < targetNentries - 1 &&
-               targetHistory[private->tliIndex].end < targetSegEnd)
-            private->tliIndex++;
-        while (private->tliIndex > 0 &&
-               targetHistory[private->tliIndex].begin >= targetSegEnd)
-            private->tliIndex--;
+        while (*tliIndex < targetNentries - 1 &&
+               targetHistory[*tliIndex].end < targetSegEnd)
+            (*tliIndex)++;
+        while (*tliIndex > 0 &&
+               targetHistory[*tliIndex].begin >= targetSegEnd)
+            (*tliIndex)--;
 
-        XLogFileName(xlogfname, targetHistory[private->tliIndex].tli,
+        XLogFileName(xlogfname, targetHistory[*tliIndex].tli,
                      xlogreadsegno, WalSegSz);
 
-        snprintf(xlogfpath, MAXPGPATH, "%s/" XLOGDIR "/%s", private->datadir, xlogfname);
+        snprintf(xlogfpath, MAXPGPATH, "%s/" XLOGDIR "/%s", datadir, xlogfname);
 
         xlogreadfd = open(xlogfpath, O_RDONLY | PG_BINARY, 0);
 
@@ -347,7 +313,7 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
 
     Assert(targetSegNo == xlogreadsegno);
 
-    *pageTLI = targetHistory[private->tliIndex].tli;
+    *pageTLI = targetHistory[*tliIndex].tli;
 
     xlogreader->readLen = XLOG_BLCKSZ;
     return;
-- 
2.16.3

From fd8bb17b85cd0a35e76a3cf29c114d765d05fbbd 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 23424886dc..ec68f770bd 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 9f184eefbe..73b343dca6 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)
         {
@@ -6327,7 +6327,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 0bf8dac408..bf61bdd230 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -149,8 +149,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;
 
@@ -178,11 +177,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 767d73f476..17d1b7ae1d 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 26446027ab..65420d0e4a 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -57,7 +57,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
     XLogReaderState *xlogreader;
     char       *errormsg;
 
-    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
+    xlogreader = XLogReaderAllocate(WalSegSz);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
@@ -109,7 +109,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
     char       *errormsg;
     XLogRecPtr    endptr;
 
-    xlogreader = XLogReaderAllocate(WalSegSz, NULL, NULL);
+    xlogreader = XLogReaderAllocate(WalSegSz);
     if (xlogreader == NULL)
         pg_fatal("out of memory");
 
@@ -166,7 +166,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
             forkptr += SizeOfXLogShortPHD;
     }
 
-    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 3125633327..cec0a401ee 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 b4ace71a75..e83fc4da0e 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.
@@ -223,9 +195,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


В списке pgsql-hackers по дате отправления:

Предыдущее
От: David Rowley
Дата:
Сообщение: Re: Top-N sorts in EXPLAIN, row count estimates, and parallelism
Следующее
От: David Rowley
Дата:
Сообщение: Re: Should we warn against using too many partitions?