[ 
https://issues.apache.org/jira/browse/PIG-4012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14532906#comment-14532906
 ] 

Rohini Palaniswamy commented on PIG-4012:
-----------------------------------------

Saw this exception in a job recently and spent some time looking into it. The 
job does not fail and the SpillableMemoryManager runs fine when JVM calls the 
handleNotification next time. Agree that this can be fixed so that the 
SpillableMemoryManager runs the first time and frees up memory early.

The actual problem is with the below piece of code in the Comparator.  

{code}
                 if (o1 == null && o2 == null) {
                        return 0;
                    }
                   if (o1 == null) {
                        return 1;
                    }
                    if (o2 == null) {
                        return -1;
                    }
{code}

References: 
http://stackoverflow.com/questions/6626437/why-does-my-compare-method-throw-exception-comparison-method-violates-its-gen
http://dertompson.com/2012/11/23/sort-algorithm-changes-in-java-7/
https://docs.oracle.com/javase/7/docs/api/java/lang/Comparable.html

Half way through sorting if o1 or o2 become null due to garbage collection, 
o1.compareTo(o2) and o2.compareTo(o1) return different results and violates the 
Comparable contract. [~ddreyfus] idea of taking the Spillable out of 
WeakReference and copying it to a new list and sorting the new list 
(https://issues.apache.org/jira/secure/attachment/12674030/PIG-3979-3.patch) 
would work. But that patch still has same null checks for Spillable in the 
comparator and would run into same exception mentioned in this jira. The 
comparator will have to change to only do size check (SpillablePtr has the size 
as well) and no null checks at all.  A new LinkedList might add to memory 
pressure in case there are lot of bag of bags, but avoiding the getMemorySize() 
call multiple times should offset that.

[~ddreyfus],
     Do you want to take it up as changes to the Comparator to only compare 
sizes will be on top of your old patch?
     
This jira is marked as Closed which should not be the case. I somehow can't 
find a way to Re-open this one even though I changed the Fix Version to 0.16 
from 0.13. 

> java.lang.IllegalArgumentException: Comparison method violates its general 
> contract! SpillableMemoryManager
> -----------------------------------------------------------------------------------------------------------
>
>                 Key: PIG-4012
>                 URL: https://issues.apache.org/jira/browse/PIG-4012
>             Project: Pig
>          Issue Type: Bug
>          Components: impl
>    Affects Versions: 0.12.0
>         Environment: java version "1.7.0_60-ea"
> Java(TM) SE Runtime Environment (build 1.7.0_60-ea-b02)
> Java HotSpot(TM) 64-Bit Server VM (build 24.60-b04, mixed mode)
>            Reporter: David Dreyfus
>            Assignee: David Dreyfus
>             Fix For: 0.16.0
>
>
> java.lang.IllegalArgumentException: Comparison method violates its general 
> contract!
>         at java.util.TimSort.mergeHi(TimSort.java:868)
>         at java.util.TimSort.mergeAt(TimSort.java:485)
>         at java.util.TimSort.mergeForceCollapse(TimSort.java:426)
>         at java.util.TimSort.sort(TimSort.java:223)
>         at java.util.TimSort.sort(TimSort.java:173)
>         at java.util.Arrays.sort(Arrays.java:659)
>         at java.util.Collections.sort(Collections.java:217)
>         at 
> org.apache.pig.impl.util.SpillableMemoryManager.handleNotification(SpillableMemoryManager.java:199)
>         at 
> sun.management.NotificationEmitterSupport.sendNotification(NotificationEmitterSupport.java:156)
>         at sun.management.MemoryImpl.createNotification(MemoryImpl.java:168)
>         at 
> sun.management.MemoryPoolImpl$PoolSensor.triggerAction(MemoryPoolImpl.java:301)
>         at sun.management.Sensor.trigger(Sensor.java:137)
> From SpillableMemoryManager.java:
>                 /**
>                  * We don't lock anything, so this sort may not be stable if 
> a WeakReference suddenly
>                  * becomes null, but it will be close enough.
>                  * Also between the time we sort and we use these spillables, 
> they
>                  * may actually change in size - so this is just best effort
>                  */
> Issue may be due to Java 7 and reporting vs ignoring the exception.
> Trying      
> -Djava.util.Arrays.useLegacyMergeSort=true
> http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6804124
> suggests the newer MergeSort is much faster.
> Someone may want to make the sorting stable in SpillableMemoryManager so that 
> the new merge sort can be used without failure.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to