[jira] [Issue Comment Deleted] (YARN-5716) Add global scheduler interface definition and update CapacityScheduler to use it.

["Jian He (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/YARN-5716?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jian He updated YARN-5716:
--------------------------
    Comment: was deleted

(was: - Fix unnecessary format changes in CapacityScheduler
- Below  null check is unnecessary
{code}
            RMContainer fromReservedContainer = null;
            if (allocation.getAllocateFromReservedContainer() != null) {
              fromReservedContainer =
                  allocation.getAllocateFromReservedContainer()
                      .getRmContainer();
            }
{code}
- This seems inconsistent with following code which will not add reserved resource back.
{code}
          // Do we have enough space on this node?
          Resource availableResource = Resources.clone(
              schedulerContainer.getSchedulerNode().getUnallocatedResource());
          if (allocation.getAllocateFromReservedContainer() != null) {
            Resources.addTo(availableResource,
                allocation.getAllocateFromReservedContainer()
                    .getRmContainer().getReservedResource());
          }
{code}
-  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.
{code}
    boolean accepted = true;
    if (!reReservation) {
      // Check parent
      accepted = getCSLeafQueue().accept(cluster, request);
    }

{code}
- should these methods be synchronized for consistency 
{code}
  public void incUnconfirmedRes(Resource res) {
    unconfirmedAllocatedMem.addAndGet(res.getMemorySize());
    unconfirmedAllocatedVcores.addAndGet(res.getVirtualCores());
  }

  public void decUnconfirmedRes(Resource res) {
    unconfirmedAllocatedMem.addAndGet(-res.getMemorySize());
    unconfirmedAllocatedVcores.addAndGet(-res.getVirtualCores());
  }
 {code}
- 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 ?
{code}
    // 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
    application.addSchedulingOpportunity(schedulerKey);
 {code}
- 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
(v6.3.4#6332)

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