Hi, On Mon, Sep 7, 2026 at 11:27 PM Kirill Reshke <[email protected]> wrote: > > Hi! > I noticed this patch did a small benchmarking on v6/v7, > primary-standby on single vm. Seems like XLByteInSeg is really needful > here, but I didn't find any v6 perf regression in close() syscall > spam. I can share my bench scripts if needed. > > reads/bytes in benches measured using pg_stat_io > > The patch virtually eliminates walsender file reads (407 MB -> 818 KB). > > ## Results: lagging subscriber > > With a lagging subscriber, the patch reduces file reads by 44% and > improves TPS (probably noise). > > v7 is neutral for buffered io (no regression/only 1-2% noise) and > saves a lot of IO for direct IO.
Thanks a lot for benchmarking it. The subscriber (or for that matter any logical decoding consumer) that closely follows the publisher WAL gets the most out of it by avoiding disk read syscalls and IO. In other words, all the WAL is available in the WAL buffers for the readers before the walwriter's opportunistic page initialization zeros them out. > Code itself looks fine to me, don't see any major issues. Thanks for reviewing it. > In 0001 this comment looks unnecessarily big for me: > > Isn't this just a very detailed way to say "close WAL segment that you > ought to close?". I think 0002 comment > > + /* > + * Close the segment when a read fully satisfied from WAL buffers is > + * not in the open segment, so the next file read reopens the correct > + * one. See logical_read_xlog_page() for why this is needed. > + */ > > Is ok. Yes, that works for me. Having it without getting into too much of what the code does is good for readers. So, I used the same wording for both 0001 and 0002, without 0002 referring to logical_read_xlog_page(). > Also here [1] & [2], you (and Andres) suggest sending WAL before it has been > locally written out and flushed to improve synchronous replication > ... > So, can you share a thread with this work or start one? Thanks. I have been thinking about this and have it on my list for PG20, but I do not have anything concrete to share yet. I will post patches when I have something ready. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
From 7605ba076e5a7663d7f5a2f5e6fa638631208752 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sat, 5 Sep 2026 23:22:33 +0000 Subject: [PATCH v8 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 <[email protected]> Reviewed-by: Jingtang Zhang <[email protected]> Reviewed-by: Nitin Jadhav <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Kirill Reshke <[email protected]> Discussion: https://www.postgresql.org/message-id/CALj2ACVfF2Uj9NoFy-5m98HNtjHpuD17EDE9twVeJng-jTAe7A%40mail.gmail.com --- src/backend/replication/walsender.c | 34 ++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index e9331de3df5..c3b02ae34a3 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,33 @@ 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)) + { + /* + * Close the segment when a read fully satisfied from WAL buffers is + * not in the open segment, so the next file read reopens the correct + * one. + */ + state->routine.segment_close(state); + } /* * After reading into the buffer, check that what we read was valid. We do -- 2.47.3
From 93f617e5010e7b5cf174cc19cce81ed59d0b2b8f Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sat, 5 Sep 2026 23:22:33 +0000 Subject: [PATCH v8 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. The timeline passed to WALReadFromBuffers() is the one the read targets, the same one passed to WALRead(), so that a read on a historical timeline never comes from the WAL buffers. 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 when such a read is not in the open segment, 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]> Reviewed-by: Kirill Reshke <[email protected]> Discussion: https://www.postgresql.org/message-id/CALj2ACVfF2Uj9NoFy-5m98HNtjHpuD17EDE9twVeJng-jTAe7A%40mail.gmail.com --- src/backend/access/transam/xlogutils.c | 29 +++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index 58b9dab6a90..8225150c593 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,31 @@ 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, tli); + + /* 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); + } + else if (state->seg.ws_file >= 0 && + !XLByteInSeg(targetPagePtr, state->seg.ws_segno, + state->segcxt.ws_segsize)) + { + /* + * Close the segment when a read fully satisfied from WAL buffers is + * not in the open segment, so the next file read reopens the correct + * one. + */ + state->routine.segment_close(state); + } /* number of valid bytes in the buffer */ return count; -- 2.47.3
From ea7e5929e1dcf5011dc29505effc9d875900da9d Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Sat, 5 Sep 2026 23:22:33 +0000 Subject: [PATCH v8 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: Kirill Reshke <[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 | 67 +++++++++++++++++++ 4 files changed, 80 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 3203f2fd4ee..7ed317ada57 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..9ffe6ec9cc0 --- /dev/null +++ b/src/test/subscription/t/099_wal_buffers_read.pl @@ -0,0 +1,67 @@ +# 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, and keep the WAL writer asleep so that its +# opportunistic buffer pre-initialization does not evict that page meanwhile. +$node->append_conf( + 'postgresql.conf', qq(wal_buffers = 64MB +wal_writer_delay = 10s)); +$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
