On Mon, Jul 13, 2026 at 2:14 PM Thom Brown <[email protected]> wrote:
> 0001 clamps read_upto to the switch point when reading a historic
> timeline, matching the page-read callback and the documented contract.

IMHO, this is a great example of why AI-generated bug reports and/or
fixes need to be double-checked by knowledgeable humans. claude would
have benefited from reading the comments for SummarizeWAL itself,
which say:

 * 'maximum_lsn' identifies the point beyond which we can't count on being
 * able to read any more WAL. It should be the switch point when reading a
 * historic timeline, or the most-recently-measured end of WAL when reading
 * the current timeline.

Which means that the clamping in 0001 shouldn't be necessary, because
the caller should already have done it.

But the question is: how exactly does this scenario arise in the first
place? SummarizeWAL checks before reading each record that the record
it's reading starts before the switch point, and then checks again
after reading it that it ends before the switch point. So if, for
example, you have a primary archiving files on TLI 1 and you promote a
standby and it archives files on TLI 2, nothing will actually go
wrong, I think. The standby trying to follow the timeline switch from
TLI 1 to TLI 2 might read one record past the switchpoint, but then it
will realize what's happened and sort itself out. The problem only
occurs if trying to read one record past the switchpoint results in an
error. In the original scenario and in claude's analysis, that seems
to happen because the tail end of the WAL segment is all zeros... but
how did such a file get archived in the first place?

The only obvious way I can see that happening is if somebody renames
the .partial file to remove that suffix and then causes the resulting
file to get archived. I don't think that's a thing that you're really
supposed to do. That's not to say I don't think we should fix this:
WalSummarizerMain is calling SummarizeWAL with a maximum_lsn that is
not computed in the way that SummarizeWAL says it should be computed,
which is bad, and the result is that this code is less robust than I
would like it to be, which is also bad. But I *think* you have to be
doing something unusual for it to become a problem in practice, which
might be why Fabrice had difficulty reproducing it.

I attach a patch. I don't think we need anything like the 0002 in your
proposal from claude. The read horizon used by the WAL summarizer
*has* to be valid; if we can't achieve that, we're doomed.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Attachment: v2-0001-walsummarizer-Guard-against-WAL-files-whose-tail-.patch
Description: Binary data

Reply via email to