[ 
https://issues.apache.org/jira/browse/DRILL-6543?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16532108#comment-16532108
 ] 

ASF GitHub Bot commented on DRILL-6543:
---------------------------------------

Ben-Zvi commented on a change in pull request #1351: DRILL-6543: Disable Hash 
Join fallback, add percent_reserved_allowance_from_direct
URL: https://github.com/apache/drill/pull/1351#discussion_r199985944
 
 

 ##########
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/util/MemoryAllocationUtilities.java
 ##########
 @@ -138,16 +139,36 @@ public static long computeOperatorMemory(OptionSet 
optionManager, long maxAllocP
   @VisibleForTesting
   public static long computeQueryMemory(DrillConfig config, OptionSet 
optionManager, long directMemory) {
 
+    // Get the options
+    double percent_per_query = 
optionManager.getOption(ExecConstants.PERCENT_MEMORY_PER_QUERY);
+    long max_query_per_node = 
optionManager.getOption(ExecConstants.MAX_QUERY_MEMORY_PER_NODE);
+    double percent_allowance = 
optionManager.getOption(ExecConstants.PERCENT_RESERVED_ALLOWANCE_FROM_DIRECT);
+
+    // verify that the allowance is kept
+    if ( percent_per_query + percent_allowance > 1.0 ) {
 
 Review comment:
   The code in the PR "enforces" keeping the allowance (out of the direct 
memory); any user settings that violates this limit returns an error 
(unfortunately can only be done when a query (using buffered ops) is executed).
     This mainly serves as a reminder (for the common user that does not read 
the documentation). It does not help with concurrent query execution (though 
users using concurrent are usually more sophisticated, so may know about the 
allowance). 
        The example suggested above applies the *allowance* onto the final 
computed memory, which is not the intention. For example, Direct Memory of 
20GB, and Mem Per Query of 4GB -- then the code about would subtract 25% and 
the Mem Per Query would be only 3GB (and confuse the user).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> Option for memory mgmt: Reserve allowance for non-buffered
> ----------------------------------------------------------
>
>                 Key: DRILL-6543
>                 URL: https://issues.apache.org/jira/browse/DRILL-6543
>             Project: Apache Drill
>          Issue Type: Improvement
>          Components: Execution - Relational Operators
>    Affects Versions: 1.13.0
>            Reporter: Boaz Ben-Zvi
>            Assignee: Boaz Ben-Zvi
>            Priority: Major
>             Fix For: 1.15.0
>
>
> Introduce a new option to enforce/remind users to reserve some allowance when 
> budgeting their memory:
> The problem: When the "planner.memory.max_query_memory_per_node" (MQMPN) 
> option is set equal (or "nearly equal") to the allocated *Direct Memory*, an 
> OOM is still possible. The reason is that the memory used by the 
> "non-buffered" operators is not taken into account.
> For example, MQMPN == Direct-Memory == 100 MB. Run a query with 5 buffered 
> operators (e.g., 5 instances of a Hash-Join), so each gets "promised" 20 MB. 
> When other non-buffered operators (e.g., a Scanner, or a Sender) also grab 
> some of the Direct Memory, then less than 100 MB is left available. And if 
> all those 5 Hash-Joins are pushing their limits, then one HJ may have only 
> allocated 12MB so far, but on the next 1MB allocation it will hit an OOM 
> (from the JVM, as all the 100MB Direct memory is already used).
> A solution -- a new option to _*reserve*_ some of the Direct Memory for those 
> non-buffered operators (e.g., default %25). This *allowance* may prevent many 
> of the cases like the example above. The new option would return an error 
> (when a query initiates) if the MQMPN is set too high. Note that this option 
> +can not+ address concurrent queries.
> This should also apply to the alternative for the MQMPN - the 
> {{"planner.memory.percent_per_query"}} option (PPQ). The PPQ does not 
> _*reserve*_ such memory (e.g., can set it to %100); only its documentation 
> clearly explains this issue (that doc suggests reserving %50 allowance, as it 
> was written when the Hash-Join was non-buffered; i.e., before spill was 
> implemented).
> The memory given to the buffered operators is the highest calculated between 
> the MQMPN and the PPQ. The new reserve option would verify that this figure 
> allows the allowance.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to