Re: Do not fail WAL buffers read after on-disk xlog removal

On Sat, Oct 24, 2026 at 10:19:42PM +0500, Kirill Reshke wrote:
> XLogSendPhysical checks for on-disk xlog removal even when the entire requested
> range was copied from WAL buffers. So this rejects data already available in the
> output buffer and terminates streaming unnecessarily.
>
> Attached checks for xlog files presence only in WAL buffers read failed.

- /* See logical_read_xlog_page(). */
- XLByteToSeg(startptr, segno, xlogreader->segcxt.ws_segsize);
- CheckXLogRemoved(segno, xlogreader->seg.ws_tli);
+ /*
+ * Check that the segment read from disk was not recycled while we were
+ * reading it. See logical_read_xlog_page(). WALReadFromBuffers() validates
+ * its own reads, which need not have corresponding files in pg_wal.
+ */
+ if (nbytes > 0)
+ {
+ XLByteToSeg(startptr, segno, xlogreader->segcxt.ws_segsize);
+ CheckXLogRemoved(segno, xlogreader->seg.ws_tli);
+ }

I have a hard time believing that this should be treated as a bug.
The current check is just not optimal as it is not required in the
case where we have no remaining data to read from the on-disk WAL
files, having read everything from the WAL buffers. I'd leave the
stable branches out of that, even if it means that a WAL sender can be
interrupted more frequently if a segment has been removed concurrently
of a physical read.

It seems to me that it would make the code more readable to move the
XLByteToSeg() and CheckXLogRemoved() calls to be in the same "if"
block as the WALRead(). No need for a comment if you move that, and
as far as I can see we don't need the XLByteToSeg() at all in the case
where we have read all the WAL from the buffers.
--
Michael