Re: Patch: Write Amplification Reduction Method (WARM)

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.

Thanks,
Pavan