[ https://issues.apache.org/jira/browse/YARN-2881?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14262397#comment-14262397 ]
Karthik Kambatla edited comment on YARN-2881 at 12/31/14 8:00 PM: ------------------------------------------------------------------ Looks mostly good, few minor comments. # FairScheduler: ## Nit: can we move PlanQueue one level up and not leave it under {{scheduler.capacity}}? ## 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 FairScheduler? # Nits: FairScheduler#PlanFollower ## The comma can go onto the first line {code} String reservationQueueName = getReservationQueueName(plan.getQueueName() , reservationId.toString()); {code} ## I would move the starting "(" to the first line or the entire RHS to the second line. {code} FSLeafQueue reservationQueue = fs.getQueueManager().getLeafQueue (reservationQueueName, false); {code} was (Author: kasha): Looks mostly good, few minor comments. # FairScheduler: ## Nit: can we move PlanQueue one level up and not leave it under {{scheduler.capacity}}? ## 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 FairScheduler? # Nits: FairScheduler#PlanFollower ## The comma can go onto the first line {code} String reservationQueueName = getReservationQueueName(plan.getQueueName() , reservationId.toString()); {code} ## I would move the starting "(" to the first line or the entire RHS to the second line. {code} FSLeafQueue reservationQueue = fs.getQueueManager().getLeafQueue (reservationQueueName, false); {code} > 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 (v6.3.4#6332)