Re: Hot Standby: subxid cache changes

Simon Riggs wrote:
> On Thu, 2009-02-12 at 14:23 +0200, Heikki Linnakangas wrote: 
>> Simon Riggs wrote:
>>> On Thu, 2009-02-12 at 09:50 +0200, Heikki Linnakangas wrote:
>>>> So far so good, but what about all the other callers of 
>>>> SubTransGetParent()? For example, XactLockTableWait will fail an 
>>>> assertion if asked to wait on a subtransaction which is then released.
>>> I agree that it could fail the assertion, though it is clear that the
>>> assertion should now be removed.
>> No, then you just get an infinite loop instead, trying to get the parent 
>> of 0 over and over again.
> 
> There is no infinite loop. Try it, or read TransactionIdIsInProgress().

I did, my CPU was pegged at 100%. Hmm, attaching with a debugger shows 
that it's not looping within XactLockTableWait as I assumed. Instead, 
XactLockTableWait returns without waiting on the parent, so we get into 
an busy loop in _bt_do_insert, trying to wait on the transaction over 
and over again.

>>> 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.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com