[jira] [Commented] (YARN-9314) Fair Scheduler: Queue Info mistake when configured same queue name at same level

["Wilfred Spiegelenburg (JIRA)" <ji...@apache.org>]

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

Wilfred Spiegelenburg commented on YARN-9314:
---------------------------------------------

Hi [~fengyongshe], thank you for filing this and providing a patch.

I have a couple comments:
* The text in the exception needs clarification:
{{queuename (" + queueName + ") repeated defining in Allocation File}}
something like this is clearer:
{{queue name (" + queueName + ") is defined multiple times, queues can only be defined once.}}
* The {{exists}} method can be simplified:
{code}
    public boolean exists(String queueName) {
      for (FSQueueType queueType : FSQueueType.values()) {
        if (configuredQueues.get(queueType).contains(queueName)) {
          return true;
        }
      }
      return false;
    }
{code}
* instead of checking the text of the message in the exception it is better to use the {{(expected = AllocationConfigurationException.class)}} on the test. If we change the text the test would still pass making maintenance easier. We already do that in a number of tests like {{testQueueAlongsideRoot}} as an example.
* the patch introduces a number of new checkstyle issues which should be fixed.

> Fair Scheduler: Queue Info mistake when configured same queue name at same level
> --------------------------------------------------------------------------------
>
>                 Key: YARN-9314
>                 URL: https://issues.apache.org/jira/browse/YARN-9314
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 3.1.0
>            Reporter: fengyongshe
>            Priority: Major
>         Attachments: Fair Scheduler Mistake when configured same queue at same level.png, YARN-9341.patch
>
>
> The Queue Info is configured in fair-scheduler.xml like below
> <queue name="root">
>       {color:#ff0000}<queue name="deva">{color}
>           <minResources>3072mb,3vcores</minResources>
>          <maxResources>4096mb,4vcores</maxResources>
>           <queue name="sample">
>                <minResources>1024mb,1vcores</minResources>
>               <maxResources>2048mb,2vcores</maxResources>
>                <aclSubmitApps>Charlie</aclSubmitApps>
>            </queue>
>        </queue>
>       {color:#ff0000}<queue name="deva">{color}
>            <minResources>1024mb,1vcores</minResources>
>            <maxResources>2048mb,2vcores</maxResources>
>        </queue>
>  </allocations>
> {color:#333333}The Queue root.deva configured last will override existing root.deva{color}{color:#333333} in root.deva.sample, like the {color}attachment 
>  
>   root.deva
> ||Used Resources:|<memory:0, vCores:0>|
> ||Min Resources:|<memory:1024, vCores:1>.  => should be <3072mb,3vcores>|
> ||Max Resources:|<memory:2048, vCores:2>.  => should be <4096mb,4vcores>|
> ||Reserved Resources:|<memory:0, vCores:0>|
> ||Steady Fair Share:|<memory:1024, vCores:2>|
> ||Instantaneous Fair Share:|<memory:0, vCores:0>|
>  
> root.deva.sample
> ||Min Resources:|<memory:1024, vCores:1>|
> ||Max Resources:|<memory:2048, vCores:2>|
> ||Reserved Resources:|<memory:0, vCores:0>|
> ||Steady Fair Share:|<memory:1024, vCores:2>|
>      
>  



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

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org