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

Sandy Ryza commented on MAPREDUCE-5651:
---------------------------------------

[~malaskat], the patch is looking good.  A few comments:

{code}
+
 public class TestFairScheduler extends TestCase {
{code}
Unnecessary new line

{code}
+
+
+    placementPolicyConfig.set("user.name", "user1");
{code}
Only need one line here

{code}
+    scheduler.getPoolManager().placementPolicy = new QueuePlacementPolicy(
+        rules, queues, conf);
{code}
Can we save the pool manager in a variable instead of calling 
scheduler.getPoolManager() each time we use it in the test.

{code}
+  static {
+    configuredQueues.add("someuser");
+  }
{code}
Put this in the BeforeClass method

{code}
   public static final String EXPLICIT_POOL_PROPERTY = 
"mapred.fairscheduler.pool";
+  
{code}
Unnecessary newline

{code}
+    try {
+      return placementPolicy.assignAppToQueue(queueName, user);
+    } catch (IOException ioe) {
+      return Pool.DEFAULT_POOL_NAME;
+    }
{code}
To mirror the YARN behavior, we should log an error and reject the app if we 
hit an IOException.

{code}
+
+      System.out.println("queue:" + queue + " " + rule);
+
{code}
Take this out.

{code}
+  public static final String  USER_AS_DEFAULT_QUEUE = 
+    "mapred.fairscheduler.user.as.default.queue";
+  public static final boolean DEFAULT_USER_AS_DEFAULT_QUEUE = true;
{code}
No need to add this property, which only exists in MR2, in MR1.  Instead, if a 
pool placement policy isn't specified, we should fall back to the MR1 default, 
which is determining the pool from mapred.fairscheduler.poolnameproperty.

MR1 refers to "pools" instead of "queues".  We should change QueuePlacementRule 
to PoolPlacementRule, assignAppToQueue to assignAppToPool, the 
queuePlacementPolicy XML element to poolPlacementPolicy, etc.

> Backport Fair Scheduler queue placement policies to branch-1
> ------------------------------------------------------------
>
>                 Key: MAPREDUCE-5651
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-5651
>             Project: Hadoop Map/Reduce
>          Issue Type: Improvement
>          Components: scheduler
>            Reporter: Sandy Ryza
>         Attachments: MAPREDUCE-5651.patch
>
>
> YARN-1392 introduced general policies for assigning applications to queues in 
> the YARN fair scheduler.  This functionality would be useful and minimally 
> invasive in MR1 as well.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to