On 19 August 2015 at 15:08, Andres Freund <and...@anarazel.de> wrote:
> On 2015-08-18 20:36:13 -0400, Tom Lane wrote: > > I wrote: > > > Just thinking about this ... I wonder why we need to call > > > TransactionIdIsInProgress() at all rather than believing the answer > from > > > the snapshot? Under what circumstances could > TransactionIdIsInProgress() > > > return true where XidInMVCCSnapshot() had not? > > > > I experimented with the attached patch, which replaces > > HeapTupleSatisfiesMVCC's calls of TransactionIdIsInProgress with > > XidInMVCCSnapshot, and then as a cross-check has all the "return false" > > exits from XidInMVCCSnapshot assert !TransactionIdIsInProgress(). > > I'm not sure about it, but it might be worthwhile to add a > TransactionIdIsKnownCompleted() check before the more expensive parts of > XidInMVCCSnapshot(). Neither the array search nor, much more so, the > subtrans lookups are free. > That's true, but they are of the same order as the ProcArray search, just without the contention, which is a pretty important thing to avoid. We only consult subtrans is the snapshot has overflowed and we do that in both XidInMVCCSnapshot() and in TransactionIdIsInProgress(), so there's no much difference there. I thought about adding TransactionIdIsKnownCompleted() also, but the cachedFetchXid will hardly ever be set correctly. If the xid is in the snapshot then with this new mechanism we don't ever check transaction completion. It's possible that we are using multiple snapshots alternately, with an xid completed in one but not the other, but that seems like a slim possibility. Hmm, I notice that XidInMVCCSnapshot() doesn't set cachedFetchXid. Perhaps we should record the last fetched xid for a snapshot, so we can use the same technique to record the outcome of repeated lookups in XidInMVCCSnapshot(). -- Simon Riggs http://www.2ndQuadrant.com/ <http://www.2ndquadrant.com/> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
