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

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

sachouche commented on a change in pull request #1329: DRILL-6513: Max query 
memory per node set to Drillbit's max direrct memory
URL: https://github.com/apache/drill/pull/1329#discussion_r196630782
 
 

 ##########
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
 ##########
 @@ -440,7 +441,7 @@ private ExecConstants() {
    * DEFAULT: 2048 MB
    */
   public static final String MAX_QUERY_MEMORY_PER_NODE_KEY = 
"planner.memory.max_query_memory_per_node";
-  public static final LongValidator MAX_QUERY_MEMORY_PER_NODE = new 
RangeLongValidator(MAX_QUERY_MEMORY_PER_NODE_KEY, 1024 * 1024, Long.MAX_VALUE);
+  public static final LongValidator MAX_QUERY_MEMORY_PER_NODE = new 
RangeLongValidator(MAX_QUERY_MEMORY_PER_NODE_KEY, 1024 * 1024, 
DrillConfig.getMaxDirectMemory());
 
 Review comment:
   @vrozov 
   
   - The current code was not performing any validation (the maximum value was 
Long.MAX_VALUE)
   - Now we added validation with the caveat that we are assuming the Drillbit 
are configured homogeneously
   - Our code assumes the same configuration (cores-per-node, memory-per-node, 
and so on)
   
   I suggest the following:
   - Let's proceed with this new validation since it is an improvement (albeit 
not perfect); from the get go I emphasized the scope of this JIRA is limited
   - Create a new JIRA to validate that all Drillbits are homogeneous; this 
will address your current feedback and many other validation deficiencies
   
   

----------------------------------------------------------------
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]


> Drill should only allow valid values when users set 
> planner.memory.max_query_memory_per_node
> --------------------------------------------------------------------------------------------
>
>                 Key: DRILL-6513
>                 URL: https://issues.apache.org/jira/browse/DRILL-6513
>             Project: Apache Drill
>          Issue Type: Bug
>          Components: Query Planning & Optimization
>            Reporter: salim achouche
>            Assignee: salim achouche
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.14.0
>
>
> The "planner.memory.max_query_memory_per_node" configuration can be currently 
> set to values higher than the Drillbit Direct Memory configuration. The goal 
> of this Jira is to fail queries with such an erroneous configuration to avoid 
> runtime OOM.
> NOTE - The current semantic of the maximum query memory per node 
> configuration is that the end user has computed valid values especially 
> knowing the current Drill limitations. Such values have to account for 
> Netty's overhead (memory pools), shared pools (e.g., network exchanges), and 
> concurrent query execution. This Jira should not be used to also cover such 
> use-cases. The Drill Resource Management feature has the means to automate 
> query quotas and the associated validation. We should create another Jira 
> requesting the enhanced validations contracts under the umbrella of the 
> Resource Management feature.   
>  
>  



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

Reply via email to