On Wed, Nov 16, 2022 at 3:27 PM Andres Freund <andres@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