On Wed, Sep 25, 2013 at 10:53 AM, Andres Freund <andres@xxxxxxxxxxxxxxx> wrote:
On 2013-09-25 00:06:06 -0700, Jeff Janes wrote:That should be gone in master, we don't use SnapshotNow anymore which
> > On 09/20/2013 03:01 PM, Jeff Janes wrote:> 3) Even worse, asking if a
> > given transaction has finished yet can be a
> > > serious point of system-wide contention, because it takes the
> > > ProcArrayLock, once per row which needs to be checked. So you have 20
> > > processes all fighting over the ProcArrayLock, each doing so 1000
> > times per
> > > query.
had those TransactionIdIsInProgress() calls you're probably referring
to. The lookups discussed in this thread now use the statement's
snapshot. And all those have their own copy of the currently running
transactions.
See HeapTupleSatisfiesMVCC, near line 943 of tqual.c:
else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
return false;
else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
HeapTupleHeaderGetXmin(tuple));
If we guarded that check by moving up line 961 to before 943:
if (XidInMVCCSnapshot(HeapTupleHeaderGetXmin(tuple), snapshot))
return false; /* treat as still in progress */
Then we could avoid the contention, as that check only refers to local memory.
As far as I can tell, the only downside of doing that is that, since hint bits might be set later, it is possible some dirty pages will get written unhinted and then re-dirtied by the hint bit setting, when more aggressive setting would have only one combined dirty write instead. But that seems rather hypothetical, and if it really is a problem we should probably tackle it directly rather than by barring other optimizations.
Cheers,
Jeff
