Karthik Kambatla commented on YARN-2881:

Looks mostly good, few minor comments.
# FairScheduler:
## Nit: can we move PlanQueue one level up and not leave it under 
## Nit: resolveReservationQueueName - {{!(allocConf.isReservable())}}, you 
don't need the parentheses after {{!}}
## Do removeQueue and setEntitlement race with only each other or with any 
other methods? If it is only the two methods, can we not synchronize on 
# Nits: FairScheduler#PlanFollower
## The comma can go onto the first line
    String reservationQueueName = getReservationQueueName(plan.getQueueName()
        , reservationId.toString());
## I would move the starting "(" to the first line or the entire RHS to the 
second line.
    FSLeafQueue reservationQueue = fs.getQueueManager().getLeafQueue
        (reservationQueueName, false);

> Implement PlanFollower for FairScheduler
> ----------------------------------------
>                 Key: YARN-2881
>                 URL: https://issues.apache.org/jira/browse/YARN-2881
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: fairscheduler
>            Reporter: Anubhav Dhoot
>            Assignee: Anubhav Dhoot
>         Attachments: YARN-2881.001.patch, YARN-2881.002.patch, 
> YARN-2881.002.patch, YARN-2881.003.patch, YARN-2881.004.patch, 
> YARN-2881.005.patch, YARN-2881.prelim.patch

This message was sent by Atlassian JIRA

Reply via email to