Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-31 Thread Alvaro Herrera
Andres Freund wrote: > On 2017-01-31 14:10:01 -0300, Alvaro Herrera wrote: > > Hmm, I was thinking we would get rid of CatalogUpdateIndexes altogether. > > Two of the callers are in the new routines (which I propose to rename to > > CatalogTupleInsert and CatalogTupleUpdate); the only remaining

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-31 Thread Andres Freund
On 2017-01-31 14:10:01 -0300, Alvaro Herrera wrote: > Pavan Deolasee wrote: > > > Two new APIs added. > > > > - CatalogInsertHeapAndIndex which does a simple_heap_insert followed by > > catalog updates > > - CatalogUpdateHeapAndIndex which does a simple_heap_update followed by > > catalog

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-31 Thread Alvaro Herrera
Alvaro Herrera wrote: > Unless there are objections I will push this later this afternoon. Done. Let's get on with the show -- please post a rebased WARM. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services --

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-31 Thread Alvaro Herrera
Pavan Deolasee wrote: > Two new APIs added. > > - CatalogInsertHeapAndIndex which does a simple_heap_insert followed by > catalog updates > - CatalogUpdateHeapAndIndex which does a simple_heap_update followed by > catalog updates > > There are only a handful callers remain for

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-31 Thread Pavan Deolasee
On Tue, Jan 31, 2017 at 7:37 PM, Alvaro Herrera wrote: > Pavan Deolasee wrote: > > > > > Sounds good. Should I submit that as a separate patch on current master? > > Yes, please. > > Attached. Two new APIs added. - CatalogInsertHeapAndIndex which does a

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-31 Thread Alvaro Herrera
Pavan Deolasee wrote: > On Tue, Jan 31, 2017 at 7:21 PM, Alvaro Herrera > wrote: > > CatalogUpdateIndexes was just added as a convenience function on top of > > a very common pattern. If we now have a reason to create a second one > > because there are now two very

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-31 Thread Pavan Deolasee
On Tue, Jan 31, 2017 at 7:21 PM, Alvaro Herrera wrote: > Pavan Deolasee wrote: > > On Thu, Jan 26, 2017 at 2:38 AM, Alvaro Herrera < > alvhe...@2ndquadrant.com> > > wrote: > > > > The simple_heap_update + CatalogUpdateIndexes pattern is getting > > > obnoxious. How

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-31 Thread Alvaro Herrera
Pavan Deolasee wrote: > On Thu, Jan 26, 2017 at 2:38 AM, Alvaro Herrera > wrote: > > The simple_heap_update + CatalogUpdateIndexes pattern is getting > > obnoxious. How about creating something like catalog_heap_update which > > does both things at once, and stop

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-30 Thread Alvaro Herrera
Pavan Deolasee wrote: > On Wed, Jan 25, 2017 at 10:06 PM, Alvaro Herrera > wrote: > > > +( \ > > > + ((tup)->t_infomask2 & HEAP_LATEST_TUPLE) != 0 \ > > > +) > > > > > > +#define HeapTupleHeaderGetRootOffset(tup) \ > > > +( \ > > > +

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-30 Thread Pavan Deolasee
On Thu, Jan 26, 2017 at 2:38 AM, Alvaro Herrera wrote: > Looking at your 0002 patch now. It no longer applies, but the conflicts > are trivial to fix. Please rebase and resubmit. > > Thanks. > > Maybe I will spend some time trying to > convert it to Perl using

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-30 Thread Pavan Deolasee
On Wed, Jan 25, 2017 at 10:06 PM, Alvaro Herrera wrote: > Reading 0001_track_root_lp_v9.patch again: > > Thanks for the review. > > +/* > > + * We use the same HEAP_LATEST_TUPLE flag to check if the tuple's > t_ctid field > > + * contains the root line pointer. We

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-26 Thread Alvaro Herrera
Robert Haas wrote: > On Wed, Jan 25, 2017 at 4:08 PM, Alvaro Herrera > wrote: > > I think the way WARM works has been pretty well hammered by now, other > > than the CREATE INDEX CONCURRENTLY issues, so I'm looking at the code > > from a maintainability point of view

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-26 Thread Robert Haas
On Wed, Jan 25, 2017 at 4:08 PM, Alvaro Herrera wrote: > I think the way WARM works has been pretty well hammered by now, other > than the CREATE INDEX CONCURRENTLY issues, so I'm looking at the code > from a maintainability point of view only. Which senior hackers have

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-26 Thread Alvaro Herrera
Alvaro Herrera wrote: > I wonder if heap_hot_search_buffer() and heap_hot_search() should return > a tri-valued enum instead of boolean; that idea looks reasonable in > theory but callers have to do more work afterwards, so maybe not. > > I think heap_hot_search() sometimes leaving the buffer

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-25 Thread Alvaro Herrera
Looking at your 0002 patch now. It no longer applies, but the conflicts are trivial to fix. Please rebase and resubmit. I think the way WARM works has been pretty well hammered by now, other than the CREATE INDEX CONCURRENTLY issues, so I'm looking at the code from a maintainability point of

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-25 Thread Alvaro Herrera
Reading 0001_track_root_lp_v9.patch again: > +/* > + * We use the same HEAP_LATEST_TUPLE flag to check if the tuple's t_ctid > field > + * contains the root line pointer. We can't use the same > + * HeapTupleHeaderIsHeapLatest macro because that also checks for > TID-equality > + * to decide

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-19 Thread Pavan Deolasee
Hi Alvaro, On Tue, Jan 17, 2017 at 8:41 PM, Alvaro Herrera wrote: > Reading through the track_root_lp patch now. > > Thanks for the review. > > + /* > > + * For HOT (or WARM) updated tuples, we store the offset > of the root > > +

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2017-01-17 Thread Alvaro Herrera
Reading through the track_root_lp patch now. > + /* > + * For HOT (or WARM) updated tuples, we store the offset of the > root > + * line pointer of this chain in the ip_posid field of the new > tuple. > + * Usually this information will be

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-12-27 Thread Pavan Deolasee
On Mon, Dec 26, 2016 at 11:49 AM, Jaime Casanova < jaime.casan...@2ndquadrant.com> wrote: > On 2 December 2016 at 07:36, Pavan Deolasee > wrote: > > > > I've updated the patches after fixing the issue. Multiple rounds of > > regression passes for me without any issue.

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-12-27 Thread Pavan Deolasee
On Sat, Dec 24, 2016 at 1:18 AM, Alvaro Herrera wrote: > Alvaro Herrera wrote: > > > With your WARM and my indirect indexes, plus the additions for for-key > > locks, plus identity columns, there is no longer a real expectation that > > we can exit early from the

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-12-26 Thread Alvaro Herrera
Alvaro Herrera wrote: > Jaime Casanova wrote: > > > * The isolation test for partial_index fails (attached the regression.diffs) > > Hmm, I had a very similar (if not identical) failure with indirect > indexes; in my case it was a bug in RelationGetIndexAttrBitmap() -- I > was missing to have

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-12-26 Thread Alvaro Herrera
Jaime Casanova wrote: > * The isolation test for partial_index fails (attached the regression.diffs) Hmm, I had a very similar (if not identical) failure with indirect indexes; in my case it was a bug in RelationGetIndexAttrBitmap() -- I was missing to have HOT considerate the columns in index

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-12-25 Thread Jaime Casanova
On 2 December 2016 at 07:36, Pavan Deolasee wrote: > > I've updated the patches after fixing the issue. Multiple rounds of > regression passes for me without any issue. Please let me know if it works > for you. > Hi Pavan, Today i was playing with your patch and

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-12-23 Thread Alvaro Herrera
Alvaro Herrera wrote: > With your WARM and my indirect indexes, plus the additions for for-key > locks, plus identity columns, there is no longer a real expectation that > we can exit early from the function. In your patch, as well as mine, > there is a semblance of optimization that tries to

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-12-23 Thread Alvaro Herrera
I noticed that this patch changes HeapSatisfiesHOTAndKeyUpdate() by adding one more set of attributes to check, and one more output boolean flag. My patch to add indirect indexes also modifies that routine to add the same set of things. I think after committing both these patches, the API is

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-12-01 Thread Haribabu Kommi
On Tue, Nov 15, 2016 at 5:58 PM, Haribabu Kommi wrote: > > > On Sat, Nov 12, 2016 at 10:12 PM, Pavan Deolasee > wrote: > >> >> >> On Tue, Nov 8, 2016 at 9:13 AM, Haribabu Kommi >> wrote: >> >>> >>> Thanks for the

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-11-14 Thread Haribabu Kommi
On Sat, Nov 12, 2016 at 10:12 PM, Pavan Deolasee wrote: > > > On Tue, Nov 8, 2016 at 9:13 AM, Haribabu Kommi > wrote: > >> >> Thanks for the patch. This shows a very good performance improvement. >> >> > Thank you. Can you please share the

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-11-07 Thread Haribabu Kommi
Thanks for the patch. This shows a very good performance improvement. I started reviewing the patch, during this process and I ran the regression test on the WARM patch. I observed a failure in create_index test. This may be a bug in code or expected that needs to be corrected. Regards, Hari

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-10-07 Thread Tomas Vondra
On 10/06/2016 07:36 AM, Pavan Deolasee wrote: On Wed, Oct 5, 2016 at 1:43 PM, Tomas Vondra > wrote: ... I can confirm the significant speedup, often by more than 75% (depending on number of indexes, whether the data

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-10-05 Thread Pavan Deolasee
On Wed, Oct 5, 2016 at 1:43 PM, Tomas Vondra wrote: > > > I've been looking at the patch over the past few days, running a bunch of > benchmarks etc. Thanks for doing that. > I can confirm the significant speedup, often by more than 75% (depending > on number of

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-10-05 Thread Tomas Vondra
On 09/05/2016 06:53 AM, Pavan Deolasee wrote: On Thu, Sep 1, 2016 at 9:44 PM, Bruce Momjian > wrote: On Thu, Sep 1, 2016 at 02:37:40PM +0530, Pavan Deolasee wrote: > I like the simplified approach, as long as it doesn't block further

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-10-02 Thread Michael Paquier
On Mon, Sep 5, 2016 at 1:53 PM, Pavan Deolasee wrote: > 0001_track_root_lp_v4.patch: This patch uses a free t_infomask2 bit to track > latest tuple in an update chain. The t_ctid.ip_posid is used to track the > root line pointer of the update chain. We do this only in

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-09-01 Thread Bruce Momjian
On Thu, Sep 1, 2016 at 02:37:40PM +0530, Pavan Deolasee wrote: > I like the simplified approach, as long as it doesn't block further > improvements. > > > > Yes, the proposed approach is simple yet does not stop us from improving > things > further. Moreover it has shown good

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-09-01 Thread Pavan Deolasee
On Thu, Sep 1, 2016 at 1:33 AM, Bruce Momjian wrote: > On Wed, Aug 31, 2016 at 10:15:33PM +0530, Pavan Deolasee wrote: > > Instead, what I would like to propose and the patch currently implements > is to > > restrict WARM update to once per chain. So the first non-HOT update to

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-08-31 Thread Bruce Momjian
On Wed, Aug 31, 2016 at 04:03:29PM -0400, Bruce Momjian wrote: > Why not just remember the tid of chains converted from WARM to HOT, then > use "amrecheck" on an index entry matching that tid to see if the index > matches one of the entries in the chain. (It will match all of them or > none of

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-08-31 Thread Bruce Momjian
On Wed, Aug 31, 2016 at 10:15:33PM +0530, Pavan Deolasee wrote: > Instead, what I would like to propose and the patch currently implements is to > restrict WARM update to once per chain. So the first non-HOT update to a tuple > or a HOT chain can be a WARM update. The chain can further be HOT

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-08-31 Thread Pavan Deolasee
On Wed, Aug 31, 2016 at 10:38 PM, Claudio Freire wrote: > > On Wed, Aug 31, 2016 at 1:45 PM, Pavan Deolasee > wrote: > >> We discussed a few ideas to address the "Duplicate Scan" problem. For >> example, we can teach Index AMs to discard any

Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)

2016-08-31 Thread Claudio Freire
On Wed, Aug 31, 2016 at 1:45 PM, Pavan Deolasee wrote: > We discussed a few ideas to address the "Duplicate Scan" problem. For > example, we can teach Index AMs to discard any duplicate (key, CTID) insert > requests. Or we could guarantee uniqueness by either only

<    1   2   3