Re: Use WALReadFromBuffers in more places

Hi,

On Thu, Sep 3, 2026 at 10:41 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> + }
> + 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);
> + }
>
> Cannot that become wasteful for the logical path when reading pages
> from the same segment repeatedly causing opening and closing of the
> same file? That sounds relevant to me if we are still attempting
> to read from the same segment, depending on wal_buffers whose default
> is at 4MB. Something like an extra check based on XLByteInSeg() may
> be adapted, using the targetPagePtr, where we could close the segment
> only if we target a page not on the same segment?
>
> The same argument applies to both v6-0001 and v6-0002, both
> unconditionally closing a segment after completing a read from buffer
> or even not completing a read from buffers and completing the read
> with an extra WALRead().

Ah, yes, that is not acceptable. Nice catch! Fixed it by closing the
old segment only when the first WAL page of the new segment is fully
read from buffers.

> + Assert(rbytes == count);
>
> This assert feels redundant due to the other checks done above.
> Applies to patches 0001 and 0002.

Removed.

Please find the attached v7 patches.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com