Jian He commented on YARN-5716:

- Fix unnecessary format changes in CapacityScheduler
- Below  null check is unnecessary
            RMContainer fromReservedContainer = null;
            if (allocation.getAllocateFromReservedContainer() != null) {
              fromReservedContainer =
- This seems inconsistent with following code which will not add reserved 
resource back.
          // Do we have enough space on this node?
          Resource availableResource = Resources.clone(
          if (allocation.getAllocateFromReservedContainer() != null) {
-  may be we can check {{anythingAllocatedOrReserved}} and 
{{getAllocateFromReservedContainer}} outside of accept so that they do need to 
be checked for every queue in the path.
    boolean accepted = true;
    if (!reReservation) {
      // Check parent
      accepted = getCSLeafQueue().accept(cluster, request);

- should these methods be synchronized for consistency 
  public void incUnconfirmedRes(Resource res) {

  public void decUnconfirmedRes(Resource res) {
- Too many data structures which wrapps container related info with similar 
names: CSAssignment, ContainerAllocation, ContainerAllocationContext, 
SchedulerContainer.. could you consolidate these ?
- SchedulingPlacementSet has a bunch unused methods and introduced unused class 
ResourceRequestUpdateResult, can we add them when needed ?
- rename allocationResult.updatedContainer properly ? or add comments to what 
this field is used for
- Add comments to what preCheckForPlacementSet does
- preCheckForSingleNode -> checkIfNodeBlackListed 
- change getFinishedStatus to isCompleted
- I think this needs to be done in accept ?
    // Inform the application it is about to get a scheduling opportunity
    // TODO, we may need to revisit here to see if we should add scheduling
    // opportunity here
- FiCaSchedulerApp#toBeRemovedIncRequests, why is this field moved to this 
class ? now the toBeRemovedIncRequests will be removed on allocate API only 

> Add global scheduler interface definition and update CapacityScheduler to use 
> it.
> ---------------------------------------------------------------------------------
>                 Key: YARN-5716
>                 URL: https://issues.apache.org/jira/browse/YARN-5716
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Wangda Tan
>            Assignee: Wangda Tan
>         Attachments: YARN-5716.001.patch, YARN-5716.002.patch
> Target of this JIRA:
> - Definition of interfaces / objects which will be used by global scheduling, 
> this will be shared by different schedulers.
> - Modify CapacityScheduler to use it.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to