Prev: pg_ctl stop -m immediate on the primary server inflatessequences
Next: [HACKERS] non-reproducible failure of random test on HEAD
From: Mark Kirkwood on 20 Apr 2010 23:09 Robert Haas wrote: > > So, does anyone have a few cycles to test this out? We are down to > handful of remaining open items, so getting this tested and committed > sooner = beta sooner. > > > I did some testing of this patch (v2). Unfortunately I don't have access to hardware capable of doing tests at the same scale as Erik used. However I was still able to show a consistent difference (I think) between standby performance with and without the patch applied. Setup: host: 2.7 Ghz dual core amd64 with 4G ram and 1 sata drive, code: cvs head from 2010-04-14. pgbench: scale=100, 4 clients, 10000 (select) transactions each. Results: Master performance (with and without patch applied ): tps = 10903.612340 - 14070.109951 (including connections establishing) Standby performance without patch (: tps = 8288.119913 - 9722.245178 (including connections establishing) Standby performance with patch applied: tps = 11592.922637 - 14065.553214 (including connections establishing) I performed 8 runs of each, and results would start at the low range and climb up to the high one, where they would stabilize. In between runs I cleared the os buffer cache and (partially) reloaded it by selecting counts from the pgbench tables (i.e I was trying to ensure each run had the same or similar os cache contents). Overall looks like the patch gets standby read only performance close to the master - at least in the case where there are minimal master transactions being tracked by the standby (I had to leave the master idle whilst running the standby case, as they shared the machine). Hope this info is useful. regards Mark -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Simon Riggs on 21 Apr 2010 02:39 On Wed, 2010-04-21 at 15:09 +1200, Mark Kirkwood wrote: > I did some testing of this patch (v2). Unfortunately I don't have access > to hardware capable of doing tests at the same scale as Erik used. > However I was still able to show a consistent difference (I think) > between standby performance with and without the patch applied. .... > Overall looks like the patch gets standby read only performance close to > the master - at least in the case where there are minimal master > transactions being tracked by the standby (I had to leave the master > idle whilst running the standby case, as they shared the machine). Hope > this info is useful. Thanks very much for the report; always good to get confirmation. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 20 Apr 2010 23:46 On Tue, Apr 20, 2010 at 11:09 PM, Mark Kirkwood <mark.kirkwood(a)catalyst.net.nz> wrote: > Robert Haas wrote: >> >> So, does anyone have a few cycles to test this out? We are down to >> handful of remaining open items, so getting this tested and committed >> sooner = beta sooner. >> >> >> > > I did some testing of this patch (v2). Unfortunately I don't have access to > hardware capable of doing tests at the same scale as Erik used. However I > was still able to show a consistent difference (I think) between standby > performance with and without the patch applied. > > Setup: > > host: 2.7 Ghz dual core amd64 with 4G ram and 1 sata drive, > code: cvs head from 2010-04-14. > pgbench: scale=100, 4 clients, 10000 (select) transactions each. > > Results: > > Master performance (with and without patch applied ): > tps = 10903.612340 - 14070.109951 (including connections establishing) > > Standby performance without patch (: > tps = 8288.119913 - 9722.245178 (including connections establishing) > > Standby performance with patch applied: > tps = 11592.922637 - 14065.553214 (including connections establishing) > > I performed 8 runs of each, and results would start at the low range and > climb up to the high one, where they would stabilize. In between runs I > cleared the os buffer cache and (partially) reloaded it by selecting counts > from the pgbench tables (i.e I was trying to ensure each run had the same or > similar os cache contents). > > Overall looks like the patch gets standby read only performance close to the > master - at least in the case where there are minimal master transactions > being tracked by the standby (I had to leave the master idle whilst running > the standby case, as they shared the machine). Hope this info is useful. Thanks, that sounds promising. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Heikki Linnakangas on 21 Apr 2010 08:20 Simon Riggs wrote: > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: >> On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: >>> Simon Riggs <simon(a)2ndQuadrant.com> writes: >>>> What I'm not clear on is why you've used a spinlock everywhere when only >>>> weak-memory thang CPUs are a problem. Why not have a weak-memory-protect >>>> macro that does does nada when the hardware already protects us? (i.e. a >>>> spinlock only for the hardware that needs it). >>> Well, we could certainly consider that, if we had enough places where >>> there was a demonstrable benefit from it. I couldn't measure any real >>> slowdown from adding a spinlock in that sinval code, so I didn't propose >>> doing so at the time --- and I'm pretty dubious that this code is >>> sufficiently performance-critical to justify the work, either. >> OK, I'll put a spinlock around access to the head of the array. > > v2 patch attached The locking seems overly complex to me. Looking at KnownAssignedXidsAdd(), isn't it possible for two processes to call it concurrently in exclusive_lock==false mode and get the same 'head' value, and step on each others toes? I guess KnownAssignedXidsAdd() is only called by the startup process, but it seems like an accident waiting to happen. Spinlocks are fast, if you have to add an if-branch to decide whether to acquire it, I suspect you've lost any performance gain to be had already. Let's keep it simple. And acquiring ProcArrayLock in exclusive mode while adding entries to the array seems OK to me as well. It only needs to be held very briefly, and getting this correct and keeping it simple is much more important at this point than squeezing out every possible CPU cycle from the system. Just require that the caller holds ProcArrayLock in exclusive mode when calling an operation that modifies the array, and in shared mode when reading it. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Heikki Linnakangas on 21 Apr 2010 08:27
Simon Riggs wrote: > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: >> On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: >>> Simon Riggs <simon(a)2ndQuadrant.com> writes: >>>> What I'm not clear on is why you've used a spinlock everywhere when only >>>> weak-memory thang CPUs are a problem. Why not have a weak-memory-protect >>>> macro that does does nada when the hardware already protects us? (i.e. a >>>> spinlock only for the hardware that needs it). >>> Well, we could certainly consider that, if we had enough places where >>> there was a demonstrable benefit from it. I couldn't measure any real >>> slowdown from adding a spinlock in that sinval code, so I didn't propose >>> doing so at the time --- and I'm pretty dubious that this code is >>> sufficiently performance-critical to justify the work, either. >> OK, I'll put a spinlock around access to the head of the array. > > v2 patch attached Given the discussion about the cyclic nature of XIDs, it would be good to add an assertion that when a new XID is added to the array, it is a) larger than the biggest value already in the array (TransactionIdFollows(new, head)), and b) not too far from the smallest value in the array to confuse binary search (TransactionIdFollows(new, tail)). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |