Simon Riggs <simon.riggs@enterprisedb.com> writes:
> On Tue, 15 Nov 2022 at 21:03, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The subxidStatus.overflowed check quoted above has a similar sort
>> of myopia: it's checking whether our current transaction has
>> already suboverflowed. But (a) that doesn't prove it won't suboverflow
>> later, and (b) the relevant logic in XidInMVCCSnapshot needs to run
>> SubTransGetTopmostTransaction if *any* proc in the snapshot has
>> suboverflowed.
> Not the way it is coded now.
> First, we search the subxid cache in snapshot->subxip.
> Then, and only if the snapshot overflowed (i.e. ANY xact overflowed),
> do we check subtrans.
No, that's not what XidInMVCCSnapshot does. If snapshot->suboverflowed
is set (ie, somebody somewhere/somewhen overflowed), then it does
SubTransGetTopmostTransaction and searches only the xips with the result.
This behavior requires that all live subxids be correctly mapped by
SubTransGetTopmostTransaction, or we'll draw false conclusions.
We could perhaps make it do what you suggest, but that would require
a complete performance analysis to make sure we're not giving up
more than we would gain.
Also, both GetSnapshotData and CopySnapshot assume that the subxips
array is not used if suboverflowed is set, and don't bother
(continuing to) populate it. So we would need code changes and
additional cycles in those areas too.
I'm not sure about your other claims, but I'm pretty sure this one
point is enough to kill the patch.
regards, tom lane