Re: Patch: Write Amplification Reduction Method (WARM)

Pavan Deolasee <pavan.deolasee@gmail.com>

From: Pavan Deolasee <pavan.deolasee@gmail.com>
To: Jeff Janes <jeff.janes@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Amit Kapila <amit.kapila16@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Bruce Momjian <bruce@momjian.us>, Jaime Casanova <jaime.casanova@2ndquadrant.com>, Haribabu Kommi <kommi.haribabu@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-03-31T21:58:37Z
Lists: pgsql-hackers

Attachments

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

> On Thu, Mar 30, 2017 at 4:13 AM, Pavan Deolasee <pavan.deolasee@gmail.com>
> wrote:
>
>>
>>
>> On Thu, Mar 30, 2017 at 3:29 PM, Dilip Kumar <dilipbalaut@gmail.com>
>> wrote:
>>
>>> On Wed, Mar 29, 2017 at 11:51 AM, Pavan Deolasee
>>> <pavan.deolasee@gmail.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.

Thanks,
Pavan

Commits

  1. Implement SortSupport for macaddr data type

  2. Simplify check of modified attributes in heap_update

  3. Remove direct uses of ItemPointer.{ip_blkid,ip_posid}

  4. Fix CatalogTupleInsert/Update abstraction for case of shared indstate.

  5. Provide CatalogTupleDelete() as a wrapper around simple_heap_delete().

  6. Band-aid fix for incorrect use of view options as StdRdOptions.

  7. Update visibility map in the second phase of vacuum.

  8. Avoid having two copies of the HOT-chain search logic.

  9. Postgres95 1.01 Distribution - Virgin Sources