On 2020-Nov-23, Andres Freund wrote: > On 2020-11-23 12:30:05 -0300, Alvaro Herrera wrote:
> > PROC_IN_LOGICAL_DECODING: > > Oddly enough, I think the reset of PROC_IN_LOGICAL_DECODING in > > ReplicationSlotRelease might be the most problematic one of the lot. > > That's because a proc's xmin that had been ignored all along by > > ComputeXidHorizons, will now be included in the computation. Adding > > asserts that proc->xmin and proc->xid are InvalidXid by the time we > > reset the flag, I got hits in pg_basebackup, test_decoding and > > subscription tests. I think it's OK for ComputeXidHorizons (since it > > just means that a vacuum that reads a later will remove less rows.) But > > in GetSnapshotData it is just not correct to have the Xmin go backwards. > > I don't think there's a problem. PROC_IN_LOGICAL_DECODING can only be > set when outside a transaction block, i.e. walsender. In which case > there shouldn't be an xid/xmin, I think? Or did you gate your assert on > PROC_IN_LOGICAL_DECODING being set? Ah, you're right about this one -- I missed the significance of setting the flag only "when outside of a transaction block" at the time we call StartupDecodingContext.