On Sun, Jul 31, 2011 at 8:32 PM, Jeff Davis <pgsql@j-davis.com> wrote:
> fpLocalTransactionId is redundant with the lxid, and the explanation is
> that one that they have different locking semantics. That looks
> reasonable, and it avoided the need for the careful ordering while
> starting/ending a transaction that was present in v2.
...which I became fairly convinced was in fact insufficiently careful.
> However, it also looks like you're using it for another purpose:
>
> In VirtualXactLockTableCleanup():
> /*
> * If fpVXIDLock has been cleared without touching fpLocalTransactionId,
> * that means someone transferred the lock to the main lock table.
> */
> if (!fastpath && LocalTransactionIdIsValid(lxid))
>
> Is the "&& LocalTransactionIdIsValid(lxid)" a guard against calling
> VirtualXactLockTableCleanup twice? Can that happen? Or is it just
> defensive coding to avoid making an additional assumption?
lxid there is just a local variable storing the value that we
extracted from fpLocalTransactionId while holding the lock. I named
it that way just as a mnemonic for the type of value that it was, not
intending to imply that it was copied from MyProc->lxid.
> PS: In the recent sinval synch patch, you had a typo: "If we haven't
> catch up completely". Other than that, it looked good.
Ah, thanks. Will fix.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
