drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From paul-rogers <...@git.apache.org>
Subject [GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...
Date Sun, 10 Sep 2017 02:53:47 GMT
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/938#discussion_r137939336
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java
---
    @@ -646,6 +687,46 @@ public AggOutcome doWork() {
       }
     
       /**
    +   *   Use reserved values memory (if available) to try and preemp an OOM
    +   */
    +  private void useReservedValuesMemory() {
    +    // try to preempt an OOM by using the reserved memory
    +    long reservedMemory = reserveValueBatchMemory;
    +    if ( reservedMemory > 0 ) { allocator.setLimit(allocator.getLimit() + reservedMemory);
}
    +
    +    reserveValueBatchMemory = 0;
    +  }
    +  /**
    +   *   Use reserved outgoing output memory (if available) to try and preemp an OOM
    +   */
    +  private void useReservedOutgoingMemory() {
    +    // try to preempt an OOM by using the reserved memory
    +    long reservedMemory = reserveOutgoingMemory;
    +    if ( reservedMemory > 0 ) { allocator.setLimit(allocator.getLimit() + reservedMemory);
}
    +
    +    reserveOutgoingMemory = 0;
    +  }
    +  /**
    +   *  Restore the reserve memory (both)
    +   *
    +   */
    +  private void restoreReservedMemory() {
    +    if ( 0 == reserveOutgoingMemory ) { // always restore OutputValues first (needed
for spilling)
    +      long memAvail = allocator.getLimit() - allocator.getAllocatedMemory();
    +      if ( memAvail > estOutgoingAllocSize) {
    +        allocator.setLimit(allocator.getLimit() - estOutgoingAllocSize);
    --- End diff --
    
    Done this way, it is necessary for this reviewer to mentally track the current allocator
limit. Can we ever subtract an amount twice? Add it twice?
    
    Now, it seems we should not alter the allocator. But, if we do, shouldn't it be based
on absolute amounts, not relative deltas?


---

Mime
View raw message