Re: Use WALReadFromBuffers in more places

Hi,

On Wed, Sep 2, 2026 at 9:08 PM Michael Paquier <michael(at)paquier(dot)xyz> 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