FAQ
I took another look at v2 of my lazy vxid locks patch and realized
that it was pretty flaky in a couple of different ways. Here's a
version that I think is a bit more robust, but considering the extent
of the revisions, it probably needs another round of review from
someone before I commit it.

Any review appreciated; I would prefer not to have to wait until
October to get this committed, since there is quite a bit of follow-on
work that I would like to do as well. FWIW, the performance
characteristics are basically identical to the previous versions,
AFAICT.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Search Discussions

  • Jeff Davis at Aug 1, 2011 at 1:02 am

    On Wed, 2011-07-20 at 13:41 -0400, Robert Haas wrote:
    I took another look at v2 of my lazy vxid locks patch and realized
    that it was pretty flaky in a couple of different ways. Here's a
    version that I think is a bit more robust, but considering the extent
    of the revisions, it probably needs another round of review from
    someone before I commit it.

    Any review appreciated; I would prefer not to have to wait until
    October to get this committed, since there is quite a bit of follow-on
    work that I would like to do as well. FWIW, the performance
    characteristics are basically identical to the previous versions,
    AFAICT.
    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.

    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?

    Regards,
    Jeff Davis

    PS: In the recent sinval synch patch, you had a typo: "If we haven't
    catch up completely". Other than that, it looked good.
  • Robert Haas at Aug 1, 2011 at 12:15 pm

    On Sun, Jul 31, 2011 at 8:32 PM, Jeff Davis 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
  • Jeff Davis at Aug 1, 2011 at 3:21 pm

    On Mon, 2011-08-01 at 08:12 -0400, Robert Haas wrote:
    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.
    I know, this is the other purpose of fpLocalTransactionId that I was
    talking about. Is it just a guard against calling
    VirtualXactLockTableCleanup twice?

    Regards,
    Jeff Davis
  • Robert Haas at Aug 1, 2011 at 4:12 pm

    On Mon, Aug 1, 2011 at 11:21 AM, Jeff Davis wrote:
    On Mon, 2011-08-01 at 08:12 -0400, Robert Haas wrote:
    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.
    I know, this is the other purpose of fpLocalTransactionId that I was
    talking about. Is it just a guard against calling
    VirtualXactLockTableCleanup twice?
    I guess you could look at that way. It just seemed like the obvious
    way to write the code: we do LockRefindAndRelease() only if we have a
    fast-path lock that someone else has pushed into the main table.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Jeff Davis at Aug 4, 2011 at 3:36 pm

    On Mon, 2011-08-01 at 12:12 -0400, Robert Haas wrote:
    I guess you could look at that way. It just seemed like the obvious
    way to write the code: we do LockRefindAndRelease() only if we have a
    fast-path lock that someone else has pushed into the main table.
    OK, looks good to me. Marked "ready for committer".

    Regards,
    Jeff Davis
  • Robert Haas at Aug 4, 2011 at 4:44 pm

    On Thu, Aug 4, 2011 at 11:29 AM, Jeff Davis wrote:
    On Mon, 2011-08-01 at 12:12 -0400, Robert Haas wrote:
    I guess you could look at that way.  It just seemed like the obvious
    way to write the code: we do LockRefindAndRelease() only if we have a
    fast-path lock that someone else has pushed into the main table.
    OK, looks good to me. Marked "ready for committer".
    Thanks for the review!

    Committed.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedJul 20, '11 at 5:41p
activeAug 4, '11 at 4:44p
posts7
users2
websitepostgresql.org...
irc#postgresql

2 users in discussion

Robert Haas: 4 posts Jeff Davis: 3 posts

People

Translate

site design / logo © 2021 Grokbase