Thread: lazy vxid locks, v2
Here is an updated version of the "lazy vxid locks" patch [1], which applies over the latest "reduce the overhead of frequent table locks"[2] patch. [1] https://commitfest.postgresql.org/action/patch_view?id=585 [2] https://commitfest.postgresql.org/action/patch_view?id=572 -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Jul 5, 2011 at 1:13 PM, Robert Haas <robertmhaas@gmail.com> wrote: > Here is an updated version of the "lazy vxid locks" patch [1], which > applies over the latest "reduce the overhead of frequent table > locks"[2] patch. > > [1] https://commitfest.postgresql.org/action/patch_view?id=585 > [2] https://commitfest.postgresql.org/action/patch_view?id=572 And then I forgot the attachment. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
On Tue, 2011-07-05 at 13:15 -0400, Robert Haas wrote: > On Tue, Jul 5, 2011 at 1:13 PM, Robert Haas <robertmhaas@gmail.com> wrote: > > Here is an updated version of the "lazy vxid locks" patch [1], which > > applies over the latest "reduce the overhead of frequent table > > locks"[2] patch. > > > > [1] https://commitfest.postgresql.org/action/patch_view?id=585 > > [2] https://commitfest.postgresql.org/action/patch_view?id=572 > > And then I forgot the attachment. The patch looks good, and I like the concept. My only real comment is one that you already made: the BackendIdGetProc() mechanism is not "awesome". However, that seems like material for a separate patch, if at all. Big disclaimer: I did not do any performance review, despite the fact that this is a performance patch. I see that there are some active performance concerns around this patch, specifically that it may cause an increase in spinlock contention: http://archives.postgresql.org/message-id/BANLkTikp4EGbfw9xDx9bQ_vK8DQa11WbPg@mail.gmail.com Fortunately, there's a subsequent discussion that shows a lot of promise: http://archives.postgresql.org/pgsql-hackers/2011-07/msg00293.php I'll mark this "waiting on author" pending the results of that discussion. I like the approach you're taking with this series of patches, so perhaps we shouldn't set the bar so high that you have to remove all of the bottlenecks before making any progress. Then again, maybe there's not a huge cost to leaving these patches on the shelf until we're sure that they lead somewhere. Regards,Jeff Davis
Robert, I should be able to do some performance testing on this, but not today. Questions: (1) can you re-link me to the pgbench and sysbench setup you used to test this originally? I'd like to implement those. (2) the max machine I can test these on is 16 cores. Is that adequate, or do we need more cores for real testing? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On Fri, Jul 15, 2011 at 4:41 PM, Josh Berkus <josh@agliodbs.com> wrote: > (1) can you re-link me to the pgbench and sysbench setup you used to > test this originally? I'd like to implement those. I didn't use sysbench. The pgbench command I used was something like: pgbench -n -S -T $TIME -c $CLIENTS -j $CLIENTS ...with varying times and client counts. Three minutes or so seems to be enough to get a reasonably good measurement. We have a report from another tester that -M prepared exacerbates the effect of the main fastlock patch (which I just committed) so that might be worth trying here, too. > (2) the max machine I can test these on is 16 cores. Is that adequate, > or do we need more cores for real testing? I think you should be able to see a small benefit on a machine of that size. My memory is a bit fuzzy because it's 1 AM and I haven't looked at this in over a week, but I believe that this patch produces only a small improvement in performance by itself, apparently because most of the gain that we get from reducing lock manager contention gets eaten up by additional spinlock contention. However, I think the change is worthwhile anyway, because the benefit of any changes we might make to reduce spinlock contention is going to be limited by the rate at which we can shove traffic through the lock manager. In other words, aside from whatever benefit this patch may have on its own, I believe it's an enabler for future performance work. In mulling over this patch, it's occurred to me that there are some stylistic things about it that can stand to be improved. So I'm going to do that for the next version. However, if you want to test on this version, feel free, because I think those changes are going to have little if any effect on performance. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company