On Fri, 2009-02-13 at 10:55 +0200, Heikki Linnakangas wrote:
> >>> The logic is: if there is no lock table entry for that xid *and* it is
> >>> not in progress *and* it is not in pg_subtrans, then it must have been
> >>> an aborted subtransaction of a currently active xact or it has otherwise
> >>> completed.
> >> Right, we got it right that far. But after the subtransaction has
> >> completed, the question is: what's its parent? That's what the patch got
> >> wrong.
> >
> > We can find that out from procarray, since a subcommitted xid will still
> > be present in the subxid cache of its parent (by definition, otherwise
> > it will be marked in pg_subtrans).
>
> Unless the top transaction just committed. Looking at the other callers
> of SubTransGetParent, I think it would introduce a race condition to
> TransactionIdDidAbort and TransactionIdDidCommit.
I don't see a race condition, but we would need to add a clog recheck if
the xid was not found in procarray in the TransactionIdDid...()
functions.
SubTransGetParent() is just a way of getting the next xid to wait on. If
the xid has been removed from procarray, we know we can recheck clog to
find an authoritative answer because clog is always fully marked before
we remove from procarray.
We need three changes:
* in DidCommit/Abort
if (xidstatus == TRANSACTION_STATUS_SUB_COMMITTED){ ...other code...
parentXid = SubTransGetParent(transactionId); if (!TransactionIdIsValid(parentXid))
TransactionIdDidCommit(transactionId); return TransactionIdDidCommit(parentXid);}
* bottom of XactLockTableWait becomes
/* we may find xid has completed just before we check */xid = SubTransGetParent(xid);if (!TransactionIdIsValid(xid))
break;
* SubTransGetParent() has some extra code to check procarray if
pg_subtrans returns 0. It would be better to refactor that
-- Simon Riggs www.2ndQuadrant.comPostgreSQL Training, Services and Support