Re: [HACKERS] lazy vxid locks, v3
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] lazy vxid locks, v3
On Thu, Aug 4, 2011 at 11:29 AM, Jeff Davis pg...@j-davis.com 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] lazy vxid locks, v3
On Sun, Jul 31, 2011 at 8:32 PM, Jeff Davis pg...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] lazy vxid locks, v3
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] lazy vxid locks, v3
On Mon, Aug 1, 2011 at 11:21 AM, Jeff Davis pg...@j-davis.com 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] lazy vxid locks, v3
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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] lazy vxid locks, v3
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 lazyvxid-v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
