[ https://issues.apache.org/jira/browse/YARN-1707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14115999#comment-14115999 ]
Jian He commented on YARN-1707: ------------------------------- Hi Carlo, thanks for your work ! I looked at the patch, some comments and questions: - to simplify, we can use getNumApplications() method {code} disposableLeafQueue.getApplications().size() > 0 || disposableLeafQueue.pendingApplications.size() > 0 {code} - PlanQueue.java 80 column limit - why “newQueue.changeCapacity(sesConf.getCapacity());” is inside the check and “queue.setMaxCapacity(sesConf.getMaxCapacity());” is outside the check - CapacityScheduler#getReservationQueueNames seems getting the child reservation queues of the given plan queue. We can use the planQueue#childQueues directly - DynamicQueueConf, how about calling it QueueEntitlement to be consistent ? - CapacityScheduler#parseQueue method, I think we can simplify the condition for isReservableQueue flag something like this: {code} boolean isReservableQueue = conf.isReservableQueue(fullQueueName); if (isReservableQueue) { ParentQueue parentQueue = new PlanQueue(csContext, queueName, parent, oldQueues.get(queueName)); queue = hook.hook(parentQueue); } else if ((childQueueNames == null || childQueueNames.length == 0)) {code} - just to simplify, this log msg may be put after previous “qiter.remove();” to avoid the removed boolean flag. {code} if (LOG.isDebugEnabled()) { LOG.debug("updateChildQueues (action: remove queue): " + removed + " " + getChildQueuesToPrint()); } {code} - we can add a new reinitialize in ReservationQueue which does all these initializations. {code} CSQueueUtils.updateQueueStatistics( schedulerContext.getResourceCalculator(), ses, this, schedulerContext.getClusterResource(), schedulerContext.getMinimumResourceCapability()); ses.reinitialize(ses, clusterResource); ((ReservationQueue) ses).setMaxApplications(this .getMaxApplicationsForReservations()); ((ReservationQueue) ses).setMaxApplicationsPerUser(this .getMaxApplicationsPerUserForReservation()); {code} - IIUC, right now, queueName here is for the planQueue(inherits parentQueue), and the reservationID is for the reservationQueue(inherits from leafQueue). I think if we can get the proper reservationQueueName(leafQueue) upfront and pass it as the queueName parameter into this method, we can avoid some if/else condition changes inside this method and the method signature. {code} private synchronized void addApplication(ApplicationId applicationId, String queueName, String user, boolean isAppRecovering, ReservationId reservationID) {code} > Making the CapacityScheduler more dynamic > ----------------------------------------- > > Key: YARN-1707 > URL: https://issues.apache.org/jira/browse/YARN-1707 > Project: Hadoop YARN > Issue Type: Sub-task > Components: capacityscheduler > Reporter: Carlo Curino > Assignee: Carlo Curino > Labels: capacity-scheduler > Attachments: YARN-1707.2.patch, YARN-1707.3.patch, YARN-1707.4.patch, > YARN-1707.patch > > > The CapacityScheduler is a rather static at the moment, and refreshqueue > provides a rather heavy-handed way to reconfigure it. Moving towards > long-running services (tracked in YARN-896) and to enable more advanced > admission control and resource parcelling we need to make the > CapacityScheduler more dynamic. This is instrumental to the umbrella jira > YARN-1051. > Concretely this require the following changes: > * create queues dynamically > * destroy queues dynamically > * dynamically change queue parameters (e.g., capacity) > * modify refreshqueue validation to enforce sum(child.getCapacity())<= 100% > instead of ==100% > We limit this to LeafQueues. -- This message was sent by Atlassian JIRA (v6.2#6252)