[
https://issues.apache.org/jira/browse/DRILL-3811?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14902929#comment-14902929
]
ASF GitHub Bot commented on DRILL-3811:
---------------------------------------
Github user cwestin commented on a diff in the pull request:
https://github.com/apache/drill/pull/163#discussion_r40109454
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/memory/AtomicRemainder.java
---
@@ -76,16 +74,17 @@ public void setLimit(long limit) {
}
/**
- * Automatically allocate memory. This is used when an actual allocation
happened to be larger than requested. This
- * memory has already been used up so it must be accurately accounted
for in future allocations.
+ * Automatically allocate memory. This is used when an actual allocation
happened to be larger than requested, or when
+ * a buffer has it's ownership passed to another allocator.<br>
+ * This memory has already been used up so it must be accurately
accounted for in future allocations.
*
- * @param size
+ * @param size extra allocated memory that needs to be accounted for
*/
public boolean forceGet(long size) {
if (get(size, this.applyFragmentLimit)) {
return true;
} else {
- availableShared.addAndGet(size);
+ availableShared.addAndGet(-size);
--- End diff --
This call is used when transferring ownership from the RPC layer to a
receiving fragment -- and you can't refuse it -- the RPC layer has to hand it
off. The problem is that the accounting for buffer ownership transfer and
sharing is questionable in the original allocator. This is probably the best
that can be done for now.
> AtomicRemainder incorrectly accounts for transferred allocations
> ----------------------------------------------------------------
>
> Key: DRILL-3811
> URL: https://issues.apache.org/jira/browse/DRILL-3811
> Project: Apache Drill
> Issue Type: Bug
> Components: Execution - Relational Operators
> Reporter: Deneche A. Hakim
> Assignee: Chris Westin
> Attachments: DRILL-3811.1.patch.txt
>
>
> when an allocator takes ownership of a buffer, AtomicRemainder.forceGet(int)
> is called to account for the extra memory of the buffer, but when the
> allocator exceeds it's maximum allocated memory it accounts for it
> incorrectly. In the following code, {{availableShared.andAndGet(size)}}
> should actually receive {{-size}}:
> {code}
> public boolean forceGet(long size) {
> if (get(size, this.applyFragmentLimit)) {
> return true;
> } else {
> availableShared.addAndGet(size);
> if (parent != null) {
> parent.forceGet(size);
> }
> return false;
> }
> }
> {code}
> I was able to reproduce the issue in a simple unit test
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
