Re: incremental backup issue

On Mon, 13 Jul 2026 at 22:43, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Mon, Jul 13, 2026 at 2:14 PM Thom Brown <thom(at)linux(dot)com> 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