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?


---

Reply via email to