Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-20 Thread Robert Haas
On Wed, Sep 20, 2017 at 10:32 PM, Andres Freund wrote: > Wonder how much of that is microarchitecture, how much number of sockets. I don't know. How would we tell? power2 is a 4-socket POWER box, and cthulhu is an 8-socket x64 box, so it's not like they are the same sort of

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-20 Thread Andres Freund
On September 20, 2017 7:22:00 PM PDT, Robert Haas wrote: >On Wed, Sep 20, 2017 at 10:04 PM, Mithun Cy > wrote: >> My current tests show on scylla (2 socket machine with 28 CPU core) I >> do not see any improvement at all as similar to Jesper.

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-20 Thread Robert Haas
On Wed, Sep 20, 2017 at 10:04 PM, Mithun Cy wrote: > My current tests show on scylla (2 socket machine with 28 CPU core) I > do not see any improvement at all as similar to Jesper. But same tests > on power2 (4 sockets) and Cthulhu(8 socket machine) we can see >

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-20 Thread Mithun Cy
On Thu, Sep 21, 2017 at 7:25 AM, Robert Haas wrote: > On Wed, Sep 20, 2017 at 9:46 PM, Mithun Cy wrote: >> I think there is some confusion above results is for pgbench simple update >> (-N) tests where cached snapshot gets invalidated, I have

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-20 Thread Robert Haas
On Wed, Sep 20, 2017 at 9:46 PM, Mithun Cy wrote: > I think there is some confusion above results is for pgbench simple update > (-N) tests where cached snapshot gets invalidated, I have run this to check > if there is any regression due to frequent cache invalidation

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-20 Thread Mithun Cy
On Wed, Sep 20, 2017 at 11:45 PM, Robert Haas wrote: > On Tue, Aug 29, 2017 at 1:57 AM, Mithun Cy wrote: >> All TPS are median of 3 runs >> Clients TPS-With Patch 05 TPS-Base%Diff >> 1 752.461117

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-20 Thread Michael Paquier
On Thu, Sep 21, 2017 at 3:15 AM, Robert Haas wrote: > On Tue, Aug 29, 2017 at 1:57 AM, Mithun Cy wrote: >> All TPS are median of 3 runs >> Clients TPS-With Patch 05 TPS-Base%Diff >> 1 752.461117

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-20 Thread Robert Haas
On Tue, Aug 29, 2017 at 1:57 AM, Mithun Cy wrote: > All TPS are median of 3 runs > Clients TPS-With Patch 05 TPS-Base%Diff > 1 752.461117755.186777 -0.3% > 64 32171.296537 31202.153576

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-14 Thread Mithun Cy
On Wed, Sep 13, 2017 at 7:24 PM, Jesper Pedersen wrote: > I have done a run with this patch on a 2S/28C/56T/256Gb w/ 2 x RAID10 SSD > machine. > > Both for -M prepared, and -M prepared -S I'm not seeing any improvements (1 > to 375 clients); e.g. +-1%. My test was

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-09-13 Thread Jesper Pedersen
Hi, On 08/29/2017 05:04 AM, Mithun Cy wrote: Test Setting: = Server configuration: ./postgres -c shared_buffers=8GB -N 300 -c min_wal_size=15GB -c max_wal_size=20GB -c checkpoint_timeout=900 -c maintenance_work_mem=1GB -c checkpoint_completion_target=0.9 -c wal_buffers=256MB & pgbench

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-08-29 Thread Mithun Cy
On Thu, Aug 3, 2017 at 2:16 AM, Andres Freund wrote: >I think this patch should have a "cover letter" explaining what it's > trying to achieve, how it does so and why it's safe/correct. Cache the SnapshotData for reuse: === In one of our perf analysis

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-08-29 Thread Mithun Cy
Hi all please ignore this mail, this email is incomplete I have to add more information and Sorry accidentally I pressed the send button while replying. On Tue, Aug 29, 2017 at 11:27 AM, Mithun Cy wrote: > Cache the SnapshotData for reuse: >

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-08-28 Thread Mithun Cy
Cache the SnapshotData for reuse: === In one of our perf analysis using perf tool it showed GetSnapshotData takes a very high percentage of CPU cycles on readonly workload when there is very high number of concurrent connections >= 64. Machine : cthulhu 8 node machine.

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-08-03 Thread Mithun Cy
On 3 Aug 2017 2:16 am, "Andres Freund" wrote: Hi Andres thanks for detailed review. I agree with all of the comments. I am going for a vacation. Once I come back I will fix those comments and will submit a new patch.

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-08-02 Thread Andres Freund
Hi, I think this patch should have a "cover letter" explaining what it's trying to achieve, how it does so and why it's safe/correct. I think it'd also be good to try to show some of the worst cases of this patch (i.e. where the caching only adds overhead, but never gets used), and some of the

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-08-02 Thread Mithun Cy
On Wed, Aug 2, 2017 at 3:42 PM, Mithun Cy wrote: Sorry, there was an unnecessary header included in proc.c which should be removed adding the corrected patch. -- Thanks and Regards Mithun C Y EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-08-02 Thread Mithun Cy
I have made few more changes with the new patch. 1. Ran pgindent. 2. Instead of an atomic state variable to make only one process cache the snapshot in shared memory, I have used conditional try lwlock. With this, we have a small and reliable code. 3. Performance benchmarking Machine - cthulhu

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2017-07-09 Thread Mithun Cy
On Fri, Apr 8, 2016 at 12:13 PM, Robert Haas wrote: > I think that we really shouldn't do anything about this patch until > after the CLOG stuff is settled, which it isn't yet. So I'm going to > mark this Returned with Feedback; let's reconsider it for 9.7. I am updating

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-04-08 Thread Robert Haas
On Tue, Mar 22, 2016 at 4:42 AM, Mithun Cy wrote: > On Thu, Mar 10, 2016 at 1:36 PM, Amit Kapila wrote: > >Do you think it makes sense to check the performance by increasing CLOG > >buffers (patch for same is posted in Speed up Clog thread

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-22 Thread Mithun Cy
On Thu, Mar 10, 2016 at 1:36 PM, Amit Kapila wrote: >Do you think it makes sense to check the performance by increasing CLOG buffers (patch for same is posted in Speed up Clog thread [1]) >as that also relieves contention on CLOG as per the tests I have done? Along with

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-19 Thread Andres Freund
On 2016-03-16 13:29:22 -0400, Robert Haas wrote: > Whoa. At 64 clients, we're hardly getting any benefit, but then by 88 > clients, we're getting a huge benefit. I wonder why there's that sharp > change there. What's the specifics of the machine tested? I wonder if it either correlates with the

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-19 Thread Amit Kapila
On Wed, Mar 16, 2016 at 10:59 PM, Robert Haas wrote: > On Wed, Mar 16, 2016 at 4:40 AM, Mithun Cy > wrote: > >> >> On Thu, Mar 3, 2016 at 6:20 AM, Mithun Cy >> wrote: >> > I will continue to benchmark above tests

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-19 Thread Robert Haas
On Wed, Mar 16, 2016 at 4:40 AM, Mithun Cy wrote: > > On Thu, Mar 3, 2016 at 6:20 AM, Mithun Cy > wrote: > > I will continue to benchmark above tests with much wider range of > clients. > > Latest Benchmarking shows following results for

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-19 Thread Robert Haas
On Wed, Mar 16, 2016 at 1:33 PM, Andres Freund wrote: > On 2016-03-16 13:29:22 -0400, Robert Haas wrote: >> Whoa. At 64 clients, we're hardly getting any benefit, but then by 88 >> clients, we're getting a huge benefit. I wonder why there's that sharp >> change there. > >

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-18 Thread Mithun Cy
On Thu, Mar 17, 2016 at 9:00 AM, Amit Kapila wrote: >If you see, for the Base readings, there is a performance increase up till 64 clients and then there is a fall at 88 clients, which to me >indicates that it hits very high-contention around CLogControlLock at 88 clients

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-16 Thread Mithun Cy
On Thu, Mar 3, 2016 at 6:20 AM, Mithun Cy wrote: > I will continue to benchmark above tests with much wider range of clients. Latest Benchmarking shows following results for unlogged tables. clients BASE ONLY CLOG CHANGES % Increase CLOG CHANGES + SAVE SNAPSHOT %

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-11 Thread Mithun Cy
Thanks Amit, I did a quick pgbench write tests for unlogged tables at 88 clients as it had the peak performance from previous test. There is big jump in TPS due to clog changes. clients BASE ONLY CLOG CHANGES % Increase ONLY SAVE SNAPSHOT % Increase CLOG CHANGES + SAVE SNAPSHOT % Increase 88

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-10 Thread Amit Kapila
On Thu, Mar 10, 2016 at 1:04 PM, Mithun Cy wrote: > > > On Thu, Mar 3, 2016 at 11:50 PM, Robert Haas > wrote: > >What if you apply both this and Amit's clog control log patch(es)? Maybe > the combination of the two helps substantially more

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-09 Thread Mithun Cy
On Thu, Mar 3, 2016 at 11:50 PM, Robert Haas wrote: >What if you apply both this and Amit's clog control log patch(es)? Maybe the combination of the two helps substantially more than either >one alone. I did the above tests along with Amit's clog patch. Machine :8

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-03 Thread Robert Haas
On Thu, Mar 3, 2016 at 6:20 AM, Mithun Cy wrote: > > On Tue, Mar 1, 2016 at 12:59 PM, Amit Kapila > wrote: > >Don't we need to add this only when the xid of current transaction is > valid? Also, I think it will be better if we can explain

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-03 Thread Mithun Cy
On Tue, Mar 1, 2016 at 12:59 PM, Amit Kapila wrote: >Don't we need to add this only when the xid of current transaction is valid? Also, I think it will be better if we can explain why we need to add the our >own transaction id while caching the snapshot. I have fixed the

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-03-01 Thread Amit Kapila
On Fri, Feb 26, 2016 at 2:55 PM, Robert Haas wrote: > > On Thu, Feb 25, 2016 at 12:57 PM, Mithun Cy wrote: > > I have fixed all of the issues reported by regress test. Also now when > > backend try to cache the snapshot we also try to store the

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-02-26 Thread Robert Haas
On Thu, Feb 25, 2016 at 12:57 PM, Mithun Cy wrote: > I have fixed all of the issues reported by regress test. Also now when > backend try to cache the snapshot we also try to store the self-xid and sub > xid, so other backends can use them. > > I also did some

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-02-24 Thread Mithun Cy
On Sat, Jan 16, 2016 at 10:23 AM, Amit Kapila wrote: > >On Fri, Jan 15, 2016 at 11:23 AM, Mithun Cy > wrote: > >> On Mon, Jan 4, 2016 at 2:28 PM, Andres Freund wrote: >> >> >> I think at the very least the cache should

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-01-15 Thread Amit Kapila
On Fri, Jan 15, 2016 at 11:23 AM, Mithun Cy wrote: > On Mon, Jan 4, 2016 at 2:28 PM, Andres Freund wrote: > > > I think at the very least the cache should be protected by a separate > > lock, and that lock should be acquired with TryLock. I.e. the

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-01-15 Thread Amit Kapila
On Fri, Jan 15, 2016 at 11:23 AM, Mithun Cy wrote: > On Mon, Jan 4, 2016 at 2:28 PM, Andres Freund wrote: > > > I think at the very least the cache should be protected by a separate > > lock, and that lock should be acquired with TryLock. I.e. the

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2016-01-04 Thread Andres Freund
On 2015-12-19 22:47:30 -0800, Mithun Cy wrote: > After some analysis I saw writing to shared memory to store shared snapshot > is not protected under exclusive write lock, this leads to memory > corruptions. > I think until this is fixed measuring the performance will not be much > useful. I

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2015-12-19 Thread Mithun Cy
On Thu, Dec 17, 2015 at 3:15 AM, Mithun Cy wrote: > I have rebased the patch and tried to run pgbench. > I see memory corruptions, attaching the valgrind report for the same. Sorry forgot to add re-based patch, adding the same now. After some analysis I saw writing

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2015-11-01 Thread Jim Nasby
On 5/25/15 10:04 PM, Amit Kapila wrote: On Tue, May 26, 2015 at 12:10 AM, Andres Freund > wrote: > > On 2015-05-20 19:56:39 +0530, Amit Kapila wrote: > > I have done some tests with this patch to see the benefit with > > and it seems to me this

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2015-11-01 Thread Amit Kapila
On Sun, Nov 1, 2015 at 8:46 PM, Jim Nasby wrote: > On 5/25/15 10:04 PM, Amit Kapila wrote: > >> On Tue, May 26, 2015 at 12:10 AM, Andres Freund > > wrote: >> > >> > On 2015-05-20 19:56:39 +0530, Amit Kapila wrote: >> >

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2015-07-24 Thread Pavan Deolasee
On Mon, Feb 2, 2015 at 8:57 PM, Andres Freund and...@2ndquadrant.com wrote: Hi, I've, for a while, pondered whether we couldn't find a easier way than CSN to make snapshots cheaper as GetSnapshotData() very frequently is one of the top profile entries. Especially on bigger servers, where the

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2015-05-25 Thread Andres Freund
On 2015-05-20 19:56:39 +0530, Amit Kapila wrote: I have done some tests with this patch to see the benefit with and it seems to me this patch helps in reducing the contention around ProcArrayLock, though the increase in TPS (in tpc-b tests is around 2~4%) is not very high. pgbench (TPC-B

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2015-05-25 Thread Amit Kapila
On Tue, May 26, 2015 at 12:10 AM, Andres Freund and...@anarazel.de wrote: On 2015-05-20 19:56:39 +0530, Amit Kapila wrote: I have done some tests with this patch to see the benefit with and it seems to me this patch helps in reducing the contention around ProcArrayLock, though the increase

Re: [HACKERS] POC: Cache data in GetSnapshotData()

2015-05-20 Thread Amit Kapila
On Mon, Feb 2, 2015 at 8:57 PM, Andres Freund and...@2ndquadrant.com wrote: Hi, I've, for a while, pondered whether we couldn't find a easier way than CSN to make snapshots cheaper as GetSnapshotData() very frequently is one of the top profile entries. Especially on bigger servers, where the

[HACKERS] POC: Cache data in GetSnapshotData()

2015-02-02 Thread Andres Freund
Hi, I've, for a while, pondered whether we couldn't find a easier way than CSN to make snapshots cheaper as GetSnapshotData() very frequently is one of the top profile entries. Especially on bigger servers, where the pretty much guaranteed cachemisses are quite visibile. My idea is based on the