On Mon, 13 Jul 2026 at 22:43, Robert Haas <[email protected]> wrote: > > 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.
Ha, yes, this is certainly a clear example of the confidence AI puts in its own conclusions not being indicative of reliability. > 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. So really, this doesn't sound like this is solved, not that these changes aren't still necessary. > 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. Nick, you said that you saw something "similar" (suggesting that it's not identical), but you didn't explain what that was. Is there potentially a separate bug that needs reporting? Thom
