On Tue, 21 Jan 2020 at 18:46, Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > Hello. > > At Mon, 20 Jan 2020 17:24:07 +0900 (JST), Kyotaro Horiguchi > <horikyota....@gmail.com> wrote in > > Separating XLogBeginRead seems reasonable. The annoying recptr trick > > is no longer needed. In particular some logical-decoding stuff become > > far cleaner by the patch. > > > > fetching_ckpt of ReadRecord is a bit annoying but that coundn't be > > moved outside to, say, ReadCheckpointRecord in a clean way. (Anyway > > XLogBeginRead is not the place, of course). > > > > As I looked through it, it looks good to me but give me some more time > > to look closer. > > It seems to me that it works perfectly, and everything looks good
I seem to remember some considerable pain in this area when it came to timeline switches. Especially with logical decoding and xlog records that split across a segment boundary. My first attempts at logical decoding timeline following appeared fine and passed tests until they were put under extended real world workloads, at which point they exploded when they tripped corner cases like this. I landed up writing ridiculous regression tests to trigger some of these behaviours. I don't recall how many of them made it into the final patch to core but it's worth a look in the TAP test suite. -- Craig Ringer http://www.2ndQuadrant.com/ 2ndQuadrant - PostgreSQL Solutions for the Enterprise