Hi,
On Wed, Sep 2, 2026 at 9:08 PM Michael Paquier <[email protected]> wrote:
>
> On Mon, Aug 17, 2026 at 04:30:00PM -0700, Bharath Rupireddy wrote:
> > Test setup: two Amazon EC2 r7i.4xlarge instances (16 vCPU, 128 GB RAM)
> > in the same AZ, one publisher and one subscriber, pg_wal on a
> > dedicated gp3 disk. Publisher runs an insert-only pgbench workload (16
> > clients) into a two-column table (bigint, text), each insert writing
> > 400 bytes, no indexes. Subscriber tails closely (lag in KB) so the WAL
> > the walsender reads is still in the 2 GB wal_buffers. wal_buffers=2GB,
> > debug_io_direct='wal'.
> >
> > Metrics: TPS is publisher pgbench insert throughput. Walsender read MB
> > is read_bytes from pg_stat_io for the walsender (bytes read through
> > WALRead()). WAL-disk reads and WAL-disk writes are peak throughput
> > from iostat on the pg_wal disk. Replication lag is
> > pg_stat_replication, sampled every 5 seconds, taken as the max over
> > the run.
>
> Worth noting something in the patch: read_local_xlog_page_guts() is
> touched, being called in read_local_xlog_page().
> read_local_xlog_page() is used in much more contexts than just the
> logical paths and pg_walinspect you are referring to at the top of
> this thread. Repack workers, 2PC code, WAL summarizer have also
> references to it in their XL_ROUTINE().
Yes, repack and 2PC are covered by the 0002 patch. But I haven't yet
used it for the WAL summarizer. Would it be okay if I do some testing
with the WAL summarizer and propose it as a follow-up patch?
> This uses pgbench for the WAL inserts. For the logical path, at
> least, could a workload based on logical WAL messages generated by
> pg_logical_emit_message() be a fancier (aka less noisy) workload to
> use to compare the modes of debug_io_direct for the scope of this
> patch?
Thanks for the suggestion. Done. I used pg_logical_emit_message() to
emit the WAL plus pg_recvlogical to let a walsender read it, and here
are the results [1].
With WAL direct IO on, the patch reduces the walsender's WAL reads
from 1.0 GB to 1.6 MB per run, removing 14.5 MB/s of physical disk
reads and improving publisher throughput by about 17% (6,393 to 7,489
TPS). The throughput gain comes from eliminating the WAL read IO on
disk, so WAL writes no longer compete with WAL reads for disk IO. With
WAL direct IO off, the same reads are eliminated at the syscall level
with no throughput change, so it never regresses.
# build WAL direct IO TPS WAL generated MB walsender
read MB WAL-disk reads WAL-disk writes replication lag
1 HEAD on 6,393 511 1,055
14.5 MB/s 9.0 MB/s 0 KB
2 PATCHED on 7,489 598 1.6
0.01 MB/s 10.6 MB/s 0 KB
3 HEAD off 7,347 587 1,229
0 10.4 MB/s 0 KB
4 PATCHED off 7,352 587 1.8
0 10.4 MB/s 0 KB
> I have to admit that I would be a bit stressed with changing the three
> code paths for logical_read_xlog_page(), XLogSendPhysical() and
> read_local_xlog_page_guts() all at once.. Checking things separately
> seems like a more safer approach, because less risky in terms of
> potential reverts of one part of the other if the buildfarm detects
> that something is wrong, particularly on slower machines where race
> condition patterns show up more easily. My 2c.
The physical walsender change was supposed to be purely mechanical,
but upon thinking more, I would rather not touch that part. Also, the
way I had it, it had a bug where it ignored the retry part. Sorry
about that.
I split the patches into two. 0001 is for logical walsender, 0002 is
for local WAL reads. 0003 adds a TAP test using an injection point for
the segment boundary issue handled in both patches (I don't intend to
get this committed unless anyone thinks otherwise). A read fully
satisfied from WAL buffers can leave the segment file open on the
wrong segment. This is handled by closing the open segment after such
a read so the next file read reopens the correct one.
PS: There is an opportunity to deduplicate with a wrapper function on
the code that 0001 and 0002 add for WALReadFromBuffers()+WALRead(). I
chose not to add that wrapper, just because we cannot reuse it in the
physical walsenders, defeating the purpose of deduplication. I am open
to thoughts on this.
Please find the attached v6 patches.
[1]
# GUCs
shared_buffers = 8GB
max_wal_size = 64GB
synchronous_commit = on
wal_buffers = 16MB
# Session 1
psql -c "SELECT pg_create_logical_replication_slot('bench','test_decoding');"
printf "SELECT pg_logical_emit_message(true, 'bench', repeat('x',
350));\n" > emit.sql
pgbench -n -f emit.sql -c 16 -j 16 -T 180 postgres
pg_recvlogical -p 5432 -d postgres --slot bench --start -f /dev/null &
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
From e8fb97ea7d9c3295ffd3291450fd67437c4770ca Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Thu, 3 Sep 2026 13:34:09 +0000
Subject: [PATCH v6 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 after a buffer-only read so
the next file read reopens the correct one.
Author: Bharath Rupireddy <[email protected]>
Reviewed-by: Jingtang Zhang <[email protected]>
Reviewed-by: Nitin Jadhav <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://www.postgresql.org/message-id/CALj2ACVfF2Uj9NoFy-5m98HNtjHpuD17EDE9twVeJng-jTAe7A%40mail.gmail.com
---
src/backend/replication/walsender.c | 48 ++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 8 deletions(-)
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index e9331de3df5..f7892d716c8 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,47 @@ 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);
+
+ targetPagePtr += rbytes;
+
+ /* now read the remaining WAL from WAL file */
+ if (rbytes < count)
+ {
+ if (!WALRead(state,
+ cur_page + rbytes,
+ targetPagePtr,
+ count - rbytes,
+ currTLI, /* Pass the current TLI because only
* WalSndSegmentOpen controls whether new TLI
* is needed. */
- &errinfo))
- WALReadRaiseError(&errinfo);
+ &errinfo))
+ {
+ WALReadRaiseError(&errinfo);
+ }
+ rbytes = count; /* All requested bytes read */
+ }
+ else if (state->seg.ws_file >= 0)
+ {
+ /*
+ * 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 after
+ * a buffer-only read so the next file read reopens the correct one.
+ */
+ state->routine.segment_close(state);
+ }
+
+ Assert(rbytes == count);
/*
* After reading into the buffer, check that what we read was valid. We do
--
2.47.3
From 41f09ff79fd9225e226aa09cbb72e7c350acdf52 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Thu, 3 Sep 2026 13:34:09 +0000
Subject: [PATCH v6 2/3] Use WALReadFromBuffers() for local WAL reads.
Commit 91f2cae7a4 introduced WALReadFromBuffers() for physical
replication walsenders, and it has since been used for logical
replication walsenders too. This commit uses it for the remaining
callers that read WAL from the local server through the shared
page-read callback, namely logical decoding driven by SQL
functions, two-phase commit, repack workers, and pg_walinspect.
When the requested WAL is still in the WAL buffers, 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 caller
follows the insertion point.
As for the logical replication walsender, a read fully satisfied
from WAL buffers can leave the segment file open on the wrong
segment. Fix this by closing the open segment after such a read
so the next file read reopens the correct one.
Author: Bharath Rupireddy <[email protected]>
Reviewed-by: Jingtang Zhang <[email protected]>
Reviewed-by: Nitin Jadhav <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://www.postgresql.org/message-id/CALj2ACVfF2Uj9NoFy-5m98HNtjHpuD17EDE9twVeJng-jTAe7A%40mail.gmail.com
---
src/backend/access/transam/xlogutils.c | 32 +++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 58b9dab6a90..f48062fdee1 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -900,6 +900,7 @@ read_local_xlog_page_guts(XLogReaderState *state, XLogRecPtr targetPagePtr,
int count;
WALReadError errinfo;
TimeLineID currTLI;
+ Size rbytes;
loc = targetPagePtr + reqLen;
@@ -1031,9 +1032,34 @@ read_local_xlog_page_guts(XLogReaderState *state, XLogRecPtr targetPagePtr,
count = read_upto - targetPagePtr;
}
- if (!WALRead(state, cur_page, targetPagePtr, count, tli,
- &errinfo))
- WALReadRaiseError(&errinfo);
+ /* 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,
+ tli,
+ &errinfo))
+ {
+ WALReadRaiseError(&errinfo);
+ }
+ rbytes = count; /* All requested bytes read */
+ }
+ else if (state->seg.ws_file >= 0)
+ {
+ /*
+ * Close the segment after a read fully satisfied from WAL buffers, so
+ * the next file read reopens the correct one. See
+ * logical_read_xlog_page() for why this is needed.
+ */
+ state->routine.segment_close(state);
+ }
+
+ Assert(rbytes == count);
/* number of valid bytes in the buffer */
return count;
--
2.47.3
From 96ae8ed4c58200b8c11f297a90cec78ce3769639 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Thu, 3 Sep 2026 15:01:09 +0000
Subject: [PATCH v6 3/3] Test reading WAL from buffers across a segment
boundary.
The previous two commits let more WAL readers use
WALReadFromBuffers() and close the open segment after a read
fully satisfied from WAL buffers that crosses into a new segment.
This commit adds a test for that.
A new injection point, wal-read-from-buffers-force-miss, forces a
buffer miss for reads that do not start at a segment boundary.
With it attached, the first page of a new segment is read from
WAL buffers while the next page falls back to the file, which
reproduces the stale open segment unless it is closed after the
buffer-only read. The test checks this for both pg_walinspect and
a logical walsender.
Author: Bharath Rupireddy <[email protected]>
Reviewed-by: Jingtang Zhang <[email protected]>
Reviewed-by: Nitin Jadhav <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://www.postgresql.org/message-id/CALj2ACVfF2Uj9NoFy-5m98HNtjHpuD17EDE9twVeJng-jTAe7A%40mail.gmail.com
---
src/backend/access/transam/xlog.c | 10 +++
src/test/subscription/Makefile | 2 +
src/test/subscription/meson.build | 1 +
.../subscription/t/099_wal_buffers_read.pl | 64 +++++++++++++++++++
4 files changed, 77 insertions(+)
create mode 100644 src/test/subscription/t/099_wal_buffers_read.pl
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 2e3f177100b..862d8966438 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1805,6 +1805,16 @@ WALReadFromBuffers(char *dstbuf, XLogRecPtr startptr, Size count,
if (RecoveryInProgress() || tli != GetWALInsertionTimeLine())
return 0;
+ /*
+ * Force a buffer miss for reads not starting at a segment boundary. See
+ * logical_read_xlog_page() for details.
+ */
+#ifdef USE_INJECTION_POINTS
+ if (XLogSegmentOffset(startptr, wal_segment_size) != 0 &&
+ IS_INJECTION_POINT_ATTACHED("wal-read-from-buffers-force-miss"))
+ return 0;
+#endif
+
Assert(XLogRecPtrIsValid(startptr));
/*
diff --git a/src/test/subscription/Makefile b/src/test/subscription/Makefile
index 1b22703dc21..01306dbd378 100644
--- a/src/test/subscription/Makefile
+++ b/src/test/subscription/Makefile
@@ -14,6 +14,8 @@ top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
EXTRA_INSTALL = contrib/hstore \
+ contrib/pg_walinspect \
+ contrib/test_decoding \
src/test/modules/injection_points
export with_icu
diff --git a/src/test/subscription/meson.build b/src/test/subscription/meson.build
index e71e95c6297..82bb3cad962 100644
--- a/src/test/subscription/meson.build
+++ b/src/test/subscription/meson.build
@@ -48,6 +48,7 @@ tests += {
't/036_sequences.pl',
't/037_except.pl',
't/038_walsnd_shutdown_timeout.pl',
+ 't/099_wal_buffers_read.pl',
't/100_bugs.pl',
],
},
diff --git a/src/test/subscription/t/099_wal_buffers_read.pl b/src/test/subscription/t/099_wal_buffers_read.pl
new file mode 100644
index 00000000000..65a01b94cb6
--- /dev/null
+++ b/src/test/subscription/t/099_wal_buffers_read.pl
@@ -0,0 +1,64 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# A WAL read served from WAL buffers must not leave a stale open segment
+# behind for a later file read. Exercised for read_local_xlog_page() (via
+# pg_walinspect) and the logical walsender (via pg_recvlogical).
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init(allows_streaming => 'logical');
+# Keep recent WAL in buffers so the new segment's first page is read from
+# buffers rather than from a file.
+$node->append_conf('postgresql.conf', 'wal_buffers = 64MB');
+$node->start;
+
+if (!$node->check_extension('injection_points'))
+{
+ plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points');
+$node->safe_psql('postgres', 'CREATE EXTENSION pg_walinspect');
+$node->safe_psql('postgres',
+ "SELECT pg_create_logical_replication_slot('slot', 'test_decoding')");
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach('wal-read-from-buffers-force-miss', 'notice')");
+
+# Emit one message per WAL page in a segment, plus a few more, so that the WAL
+# written after the switch crosses a segment boundary by a couple of pages.
+my $seg_size = $node->safe_psql('postgres',
+ "SELECT pg_size_bytes(current_setting('wal_segment_size'))");
+$node->safe_psql('postgres', 'SELECT pg_switch_wal()');
+my $start_lsn = $node->safe_psql('postgres', 'SELECT pg_current_wal_lsn()');
+$node->safe_psql('postgres',
+ "SELECT count(pg_logical_emit_message(false, 'test', repeat('x', 8192)))
+ FROM generate_series(1, $seg_size / 8192 + 16)");
+my $end_lsn = $node->safe_psql('postgres',
+ "SELECT pg_logical_emit_message(false, 'test', 'flush', true)");
+
+# read_local_xlog_page() path.
+my ($ret, $stdout, $stderr) = $node->psql('postgres',
+ "SELECT count(*) > 0 FROM pg_get_wal_records_info('$start_lsn', '$end_lsn')");
+is($ret, 0, 'pg_walinspect reads across a segment boundary');
+is($stdout, 't', 'pg_walinspect returns records across the boundary');
+
+# logical walsender path.
+my ($rc, $rout, $rerr) = $node->pg_recvlogical_upto('postgres', 'slot',
+ $end_lsn, $PostgreSQL::Test::Utils::timeout_default);
+is($rc, 0, 'walsender decodes across a segment boundary');
+unlike($rerr, qr/unexpected pageaddr/, 'walsender did not reuse a stale segment');
+
+$node->safe_psql('postgres',
+ "SELECT injection_points_detach('wal-read-from-buffers-force-miss')");
+
+done_testing();
--
2.47.3
