Re: Re: Why is RegisterPredicateLockingXid called while holding XidGenLock?

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane wrote:
>> Yeah, I was thinking that it'd be better to pull it out of
>> GetNewTransactionId and put it in a higher level.

> As long as it is always called when an xid is assigned. Since this
> function appears to be on the only path to that, it should be fine.

Well, the division of labor between GetNewTransactionId and
AssignTransactionId isn't terribly well-defined, but to the extent that
there is any bright line it is that the former does what needs to be
done while holding XidGenLock. So I'd prefer to see the call out of
there entirely. The fact that varsup.c has no other connection to the
SSI code is an additional argument that it doesn't belong there.

>> No strong preference about where in AssignTransactionId to put it.
>> Is there any chance that it would be significant whether we do it
>> before or after taking the lock on the XID (XactLockTableInsert)?

> No, but since we need to do it only on a top level assignment, we
> could save a couple cycles by putting it on an else on line 456.

Didn't particularly care for that, since this action is not the inverse
of, nor in any other way related to, pushing the XID into pg_subtrans.
After some thought I did this instead:

if (isSubXact)
SubTransSetParent(s->transactionId, s->parent->transactionId, false);

/*
* If it's a top-level transaction, the predicate locking system needs to
* be told about it too.
*/
if (!isSubXact)
RegisterPredicateLockingXid(s->transactionId);

A reasonably bright compiler will optimize that into the same thing, and
if the compiler doesn't catch it, it's an insignificant cost anyway.

regards, tom lane