Re: Logical decoding timeline following fails to handle records split across segments

 

I'm aware that this is late in the piece, btw, and that revert will be considered. I think that's probably unnecessary - in part because this code only gets called when doing logical decoding, and only does anything remotely interesting when replay from a slot hits a timeline boundary. So it should be assessed mainly based on its risk of breaking what works. That said, I also think that now that I've fixed the fundamental misunderstanding embodied by the original patch it should be pretty clear and reasonable.

The diff looks big, but it just rewrites the new XLogReadDetermineTimeline function and otherwise just removes the no-longer-needed  XlogReaderState->timelineHistory member the prior one added. It's best to just read the new XLogReadDetermineTimeline function, not the diff of that function, since diff has done confusing things with it.

The changes here are that:

* XLogReadDetermineTimeline(...) now looks up the page being fetched by ReadPageInternal. It previously used the record start pointer, but that didn't work if a continuation spilled over to the first page on a new segment where there's a timeline switch.

* The logic for determining the last timeline on a segment was way too complicated in the old patch. Instead, just look up the timeline of the LSN of the last byte on the segment. No more loops.

* XlogReaderState->timelineHistory is removed; the timeline file is now read only when needed when making a timeline decision, then discarded. It's read only very infrequently. Any time we read it we might need to re-read it if there's a newer one anyway, so it's simpler this way.

* XLogReaderState->currTLIValidUntil is now the tliSwitchPoint for currTLI and is no longer truncated to the start of the segment boundary like before. So it's actually useful now. That's a result of fixing the stupid logic I used in the old version for calculating the last timeline on a segment and whether there's a timeline change on the segment.

Álvaro is aware of this and I've added it to open items.