[
https://issues.apache.org/jira/browse/YARN-1392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13821764#comment-13821764
]
Alejandro Abdelnur commented on YARN-1392:
------------------------------------------
Overall looks good.
* Javadoc warning still surviving:
{code}
[WARNING] Javadoc Warnings
[WARNING]
/home/jenkins/jenkins-slave/workspace/PreCommit-YARN-Build/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/QueuePlacementRule.java:75:
warning - @param argument "ugi" is not a parameter name.
{code}
* FairScheduler.apt.vm
** first chunk: s/applicaton/application/
** second chunk: s/subelements/sub-elements/g
** second chunk: not really clear: "which are specified as subelements of the
rule element, for which the argument name is the subelement and the argument
value is its contents."
** second chunk: "The currently existing rules are detailed below:", just say
"Valid rules are:"
** we should spell out the default behavior if create is not specified
* XML syntax for rules, 'type' seems more appropriate than the 'name'
attribute. Said this, wouldn't the following syntax be cleaner:
{code}
<queuePlacementPolicy>
<specifiedQueue create="false"/>
<userPrimaryGroup create="false"/>
<userName create="false"/>
<defaultQueue/>
</queuePlacementPolicy>
{code}
* FairScheduler.java
** Exception message "Application by queue placement policy" not really clear,
how about "Queue placement policy could not resolve to a valid queue"
** {{LOG.error("Error assigning app to queue, placing in default", ex);}}
message is misleading, from what can I tell it will return NULL and it will be
rejected.
* QueuePlacementPolicy.java
** Any special reason for using Maps.newHashMap() instead of JDK HashMap?
** The first constructor is not used, do we need it?
** If the constructor fails to process the policies it throws an exception.
This will abort the complete scheduler config reloading, right? If so, it's OK.
** The parseArgsAndInitialize() method should delegate to each rule class for
parsing its sub-elements (if any), even the create should be handled by the
rule parser.
* {{QueuePlacementRule.java}}
** it seems that if we have a rule with 'create=true' we shouldn't allow follow
up rules as they are never used. The QueryManager should take care of this.
> Allow sophisticated app-to-queue placement policies in the Fair Scheduler
> -------------------------------------------------------------------------
>
> Key: YARN-1392
> URL: https://issues.apache.org/jira/browse/YARN-1392
> Project: Hadoop YARN
> Issue Type: New Feature
> Components: scheduler
> Affects Versions: 2.2.0
> Reporter: Sandy Ryza
> Assignee: Sandy Ryza
> Attachments: YARN-1392-1.patch, YARN-1392-1.patch, YARN-1392.patch
>
>
> Currently the Fair Scheduler supports app-to-queue placement by username. It
> would be beneficial to allow more sophisticated policies that rely on primary
> and secondary groups and fallbacks.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
