[ https://issues.apache.org/jira/browse/YARN-9892?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17008878#comment-17008878 ]
Peter Bacsko edited comment on YARN-9892 at 1/6/20 1:59 PM: ------------------------------------------------------------ Thanks for the patch [~maniraj...@gmail.com]. I just have a few thoughts: 1. Do we need a {{CompoundComparator}} in the constructor? Can't we just pass {{DominantResourceFairnessComparator}} instance directly to {{ConcurrentSkipListSet}}? 2. Quite a few methods are empty, I guess on purpose. However, it would be good to indicate that it's intentional. So I'd add a short comment like "// nop" to each empty method body. 3. Probably the biggest concern: I think the "dominant resource" is the one which has the largest {{resource[N]/clusterResource[N]}} value, where "N" is the index of a particular resource in the available resource vector. So I think you need the cluster resource there, not the queue effective resource. So you should somehow retrieve the cluster resource and pass it to the policy implementation. See Fair Scheduler implementation: https://github.com/apache/hadoop/blob/0921b706f7f80c40e061d2c0f8c8b2e4910071e5/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/policies/DominantResourceFairnessPolicy.java#L283-L298 was (Author: pbacsko): Thanks for the patch [~maniraj...@gmail.com]. I just have a few thoughts: 1. Do we need a {{CompoundComparator}} in the constructor? Can't we just pass {{DominantResourceFairnessComparator}} instance directly to {{ConcurrentSkipListSet}}?2. Quite a few methods are empty, I guess on purpose. However, it would be good to indicate that it's intentional. So I'd add a short comment like "// nop" to each empty method body. 3. Probably the biggest concern: I think the "dominant resource" is the one which has the largest {{resource[N]/clusterResource[N]}} value, where "N" is the index of a particular resource in the available resource vector. So I think you need the cluster resource there, not the queue effective resource. So you should somehow retrieve the cluster resource and pass it to the policy implementation. See Fair Scheduler implementation: https://github.com/apache/hadoop/blob/0921b706f7f80c40e061d2c0f8c8b2e4910071e5/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/policies/DominantResourceFairnessPolicy.java#L283-L298 > Capacity scheduler: support DRF ordering policy on queue level > -------------------------------------------------------------- > > Key: YARN-9892 > URL: https://issues.apache.org/jira/browse/YARN-9892 > Project: Hadoop YARN > Issue Type: Sub-task > Components: capacity scheduler > Reporter: Peter Bacsko > Assignee: Manikandan R > Priority: Major > Attachments: YARN-9892.001.patch > > > Capacity scheduler does not support DRF (Dominant Resource Fairness) ordering > policy on queue level. Only "fifo" and "fair" are accepted for > {{yarn.scheduler.capacity.<queue-path>.ordering-policy}}. > DRF can only be used globally if > {{yarn.scheduler.capacity.resource-calculator}} is set to > DominantResourceCalculator. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org