Github user sachouche commented on a diff in the pull request:
https://github.com/apache/drill/pull/1237#discussion_r184747702
--- Diff:
exec/memory/base/src/main/java/org/apache/drill/exec/memory/AllocationManager.java
---
@@ -253,10 +261,12 @@ public boolean transferBalance(final BufferLedger
target) {
target.historicalLog.recordEvent("incoming(from %s)",
owningLedger.allocator.name);
}
- boolean overlimit = target.allocator.forceAllocate(size);
+ // Release first to handle the case where the current and target
allocators were part of the same
+ // parent / child tree.
allocator.releaseBytes(size);
+ boolean allocationFit = target.allocator.forceAllocate(size);
--- End diff --
- The change of order is an optimization for a parent / child relationship
as if we don't release first, then we could unnecessarily go over the memory
budget (double counting).
- The force-alloc() / free() failures should never happen on normal
conditions; when they do, the best thing to do is to exit. I still prefer not
to promote the target allocator till it is 100% successful.
---
