From 0f8b4da1d9f0cc767ca178e6051c63ad6b4c345f Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Sat, 5 Sep 2026 23:22:33 +0000
Subject: [PATCH v7 1/3] Use WALReadFromBuffers() for logical replication
 walsenders.

Commit 91f2cae7a4 introduced WALReadFromBuffers() but used it
only for physical replication walsenders. This commit uses it for
logical replication walsenders as well, so that logical decoding
can also read WAL from the WAL buffers instead of always going to
a file.

When a logical replication consumer keeps up with WAL generation,
the requested WAL is often still in the WAL buffers, so it can be
read from there instead of from a file. The gain is largest with
WAL direct I/O, where a file read is a physical disk read.
Without direct I/O it still saves a syscall and does not regress.
The benefit depends on the workload and how closely the consumer
follows the insertion point.

A read fully satisfied from WAL buffers skips the file read path,
which is also where the reader closes and reopens its segment
file as it crosses a segment boundary. So a buffer-only read
never notices a segment change. The segment file stays open on
the old segment while the reader's segment number advances to the
new one. For example, when the first page of segment 2 comes from
buffers, the reader's segment number becomes 2 but its file is
still open on segment 1. A later read of segment 2 that falls
back to the file reuses that stale open file and returns segment
1's data, seen during decoding as an "unexpected pageaddr" error.
Fix this by closing the open segment when a buffer-only read is
not in the open segment, so the next file read reopens the
correct one. Reads that stay within the open segment leave it
alone, as the reader's segment number does not change.

Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Jingtang Zhang <mrdrivingduck@gmail.com>
Reviewed-by: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/CALj2ACVfF2Uj9NoFy-5m98HNtjHpuD17EDE9twVeJng-jTAe7A%40mail.gmail.com
---
 src/backend/replication/walsender.c | 45 ++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 8 deletions(-)

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index e9331de3df5..9f9b3b3f729 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1098,6 +1098,7 @@ logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int req
 	WALReadError errinfo;
 	XLogSegNo	segno;
 	TimeLineID	currTLI;
+	Size		rbytes;
 
 	/*
 	 * Make sure we have enough WAL available before retrieving the current
@@ -1157,16 +1158,44 @@ logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int req
 	else
 		count = flushptr - targetPagePtr;	/* part of the page available */
 
-	/* now actually read the data, we know it's there */
-	if (!WALRead(state,
-				 cur_page,
-				 targetPagePtr,
-				 count,
-				 currTLI,		/* Pass the current TLI because only
+	/* attempt to read WAL from WAL buffers first */
+	rbytes = WALReadFromBuffers(cur_page, targetPagePtr, count, currTLI);
+
+	/* now read the remaining WAL from WAL file */
+	if (rbytes < count)
+	{
+		if (!WALRead(state,
+					 cur_page + rbytes,
+					 targetPagePtr + rbytes,
+					 count - rbytes,
+					 currTLI,	/* Pass the current TLI because only
 								 * WalSndSegmentOpen controls whether new TLI
 								 * is needed. */
-				 &errinfo))
-		WALReadRaiseError(&errinfo);
+					 &errinfo))
+			WALReadRaiseError(&errinfo);
+	}
+	else if (state->seg.ws_file >= 0 &&
+			 !XLByteInSeg(targetPagePtr, state->seg.ws_segno,
+						  state->segcxt.ws_segsize))
+	{
+		/*
+		 * A read fully satisfied from WAL buffers skips WALRead(), which is
+		 * where ws_file is closed and reopened as the reader crosses
+		 * segments. So a buffer-only read never notices the segment change.
+		 * ws_file stays open on the old segment while ReadPageInternal()
+		 * advances ws_segno. For example, when the first page of segment 2
+		 * comes from buffers, ws_segno becomes 2 but ws_file is still open on
+		 * segment 1. A later read of segment 2 that falls back to the file
+		 * reuses the stale descriptor, since WALRead() decides whether to
+		 * reopen from ws_segno (already 2) rather than the open file. It
+		 * reads segment 1 and returns the wrong segment's WAL, seen during
+		 * decoding as an "unexpected pageaddr" error. Close the segment when
+		 * the page just read from buffers is not in the open segment, so the
+		 * next file read reopens the correct one. Reads staying within the
+		 * open segment leave it alone, because ws_segno does not change.
+		 */
+		state->routine.segment_close(state);
+	}
 
 	/*
 	 * After reading into the buffer, check that what we read was valid. We do
-- 
2.47.3

