Re: Patch: Write Amplification Reduction Method (WARM)

From: Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
To: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Jaime Casanova <jaime(dot)casanova(at)2ndquadrant(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch: Write Amplification Reduction Method (WARM)
Date: 2017-03-31 21:58:37
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

On Sat, Apr 1, 2017 at 12:39 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:

> On Thu, Mar 30, 2017 at 4:13 AM, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>
> wrote:
>> On Thu, Mar 30, 2017 at 3:29 PM, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
>> wrote:
>>> On Wed, Mar 29, 2017 at 11:51 AM, Pavan Deolasee
>>> <pavan(dot)deolasee(at)gmail(dot)com> wrote:
>>> > Thanks. I think your patch of tracking interesting attributes seems ok
>>> too
>>> > after the performance issue was addressed. Even though we can still
>>> improve
>>> > that further, at least Mithun confirmed that there is no significant
>>> > regression anymore and in fact for one artificial case, patch does
>>> better
>>> > than even master.
>>> I was trying to compile these patches on latest
>>> head(f90d23d0c51895e0d7db7910538e85d3d38691f0) for some testing but I
>>> was not able to compile it.
>>> make[3]: *** [postgres.bki] Error 1
>> Looks like OID conflict to me.. Please try rebased set.
> broken again on oid conflicts for 3373 to 3375 from the monitoring
> permissions commi 25fff40798fc4.
Hi Jeff,

Thanks for trying. Much appreciated,

> After bumping those, I get these compiler warnings:
> heapam.c: In function 'heap_delete':
> heapam.c:3298: warning: 'root_offnum' may be used uninitialized in this
> function
> heapam.c: In function 'heap_update':
> heapam.c:4311: warning: 'root_offnum' may be used uninitialized in this
> function
> heapam.c:4311: note: 'root_offnum' was declared here
> heapam.c:3784: warning: 'root_offnum' may be used uninitialized in this
> function
> heapam.c: In function 'heap_lock_tuple':
> heapam.c:5087: warning: 'root_offnum' may be used uninitialized in this
> function
Thanks. I don't see them on my LLVM compiler even at -O2. Anyways, I
inspected. They all looked non-problematic, but fixed in the attached
version v24, along with some others I could see on another linux machine.

> And I get a regression test failure, attached.
Thanks again. Seems like my last changes to disallow WARM updates if more
than 50% indexes are updated caused this regression. Having various
features in different branches and merging them right before sending out
the patchset was probably not the smartest thing to do. I've fixed the
regression simply by adding another index on that table and making changes
to the expected output.

BTW I still need 2 regression failures, but I see them on the master too,
so not related to the patch. Attached here.


Attachment Content-Type Size
0004-Provide-control-knobs-to-decide-when-to-do-heap-_v24.patch application/octet-stream 69.9 KB
0001-Track-root-line-pointer-v23_v24.patch application/octet-stream 39.4 KB
0002-Free-3-bits-in-ip_posid-field-of-the-ItemPointer_v24.patch application/octet-stream 4.2 KB
0003-Main-WARM-patch_v24.patch application/octet-stream 285.6 KB
regression.diffs application/octet-stream 2.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Sharma 2017-03-31 22:09:22 Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
Previous Message Robert Haas 2017-03-31 21:44:39 Re: Partitioning vs ON CONFLICT