On Wed, Nov 16, 2022 at 3:27 PM Andres Freund <and...@anarazel.de> wrote: > The "(also...) formulation seems a bit odd. How about "an obsolescent heap > tuple that the caller is physically removing, e.g. via HOT pruning or index > deletion." or such?
Okay, WFM. > > + * snapshotConflictHorizon format values are how all hot Standby conflicts > > are > > + * generated by REDO routines (at least wherever a granular cutoff is > > used). > > Not quite parsing for me. I meant something like: this is a cutoff that works in the same way as any other cutoff involved with recovery conflicts, in general, with the exception of those cases that have very coarse grained conflicts, such as DROP TABLESPACE. I suppose it would be better to just say the first part. Will fix. > > + /* > > + * It's quite possible that final snapshotConflictHorizon value will > > be > > + * invalid in final WAL record, indicating that we definitely don't > > need to > > + * generate a conflict > > + */ > > *the final > > Isn't this already described in the header? Sort of, but arguably it makes sense to call it out specifically. Though on second thought, yeah, lets just get rid of it. > What are "snapshotConflictHorizon format XIDs"? I guess you mean format in the > sense of having the semantics of snapshotConflictHorizon? Yes. That is the only possible way that any recovery conflict ever works on the REDO side, with the exception of a few not-very-interesting cases such as DROP TABLESPACE. GetConflictingVirtualXIDs() assigns a special meaning to InvalidTransactionId which is the *opposite* of the special meaning that snapshotConflictHorizon-based values assign to InvalidTransactionId. At one point they actually did the same definition for InvalidTransactionId, but that was changed soon after hot standby first went in (when we taught btree delete records to not use ludicrously conservative cutoffs that caused needless conflicts). Anyway, worth calling this out directly in these comments IMV. We're addressing two closely related things that assign opposite meanings to InvalidTransactionId, which is rather confusing. -- Peter Geoghegan
