Github user sachouche commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1237#discussion_r183898352
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
 ---
    @@ -182,13 +184,18 @@ public IterOutcome next() {
             return IterOutcome.OUT_OF_MEMORY;
           }
     
    +      // Transfer the ownership of this raw-batch to this operator for 
proper memory statistics reporting
    +      batch = batch.transferBodyOwnership(oContext.getAllocator());
    --- End diff --
    
    @HanumathRao,
    
    - This will not affect the outcome since the unordered-receiver is a child 
of the context allocator (the minor fragment)
    - Doing the check earlier seemed cleaner as there is no point of changing 
ownership when there is already an out-of-memory condition
    - I think the code was written this way since implicitly, the network 
handlers are receiving the batch and then performing a change of ownership 
(from RPC to context allocator); this step could lead to an out-of-memory 
condition at the fragment level
    - If your point is about reporting, that is attributing the OOM condition 
to the child operator, then I guess I don't have a problem with that.
    @parthchandra what do you think?
     


---

Reply via email to