hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Wangda Tan (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-2113) Add cross-user preemption within CapacityScheduler's leaf-queue
Date Fri, 31 Mar 2017 16:41:42 GMT

    [ https://issues.apache.org/jira/browse/YARN-2113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15951244#comment-15951244
] 

Wangda Tan commented on YARN-2113:
----------------------------------

Thanks [~sunilg],

1) validateOutSameAppPriorityFromDemand:

Major:
1.1)
{code}
              Resource deltaULQuota = Resources
                  .subtract(tmpUser.getUsedDeductAM(), tmpUser.getUserLimit());
{code}
Is this correct? Should we use "{{used - user_already_selected_to_be_preempt - user_limit
> 0}}"?

1.2) 
{code}
            if (Resources.lessThanOrEqual(rc, cluster,
                apps[lPriority].toBePreempted,
                apps[lPriority].getActuallyToBePreempted())) {
              continue;
            }

            // For user-limit preemption, we may not preempt whole of
            // apps[lPriority].toBePreempted. So use below flag to continue.
            if (Resources.equals(apps[hPriority].getToBePreemptFromOther(),
                Resources.none()) || skipAppBasedOnULQuota) {
              continue;
            }
{code}
The two statement don't take any effect, should you move them to front or just remove them?

Minor: 
- delta => preemptableFromLowerPriorityApp, and add a comment
- "we need to do below hack" => It's not a hack to me! :-p
- skipAppBasedOnULQuota is not necessary which can be replaced by a "continue".

2) calculateIdealAssignedResourcePerApp:
- {{partitionBasedResource}} is not used, is it a mistake?
- {{clusterResource}}, should we remove it and use {{partitionBasedResource}} instead? 
- TempApp can be added to orderedApps at the begining of the while loop after {{orderedByPriority.remove()}}

3) Other minor comments:
- TempUserPerPartition#isPreemptionQuotaForULDeltaDone/isUserLimitReached => is not necessary,
it's only used once, it's better to directly use formula check.
- TempApp#skipFromPreemptionCandidateSelection, it is not necessary as well. Better to use
formula directly.
- TempApp#isBelowUserLimit, it's also not necessary and can be get from TempUser.
{code}
            && (apps[lPriority].isBelowUserLimit() == false)) {
          TempUserPerPartition tmpUser = usersPerPartition
              .get(apps[lPriority].getUser());
{code} 
- CapacitySchedulerPreemptionUtils#deductPreemptableResourcePerApp, partition is not necessary,
and same for TempApp#deductActuallyToBePreempted
- Not caused by this patch, I suggest to remove {{queueReassignableResource}} entirely from
different methods/class, since it can be replaced by formula.

> Add cross-user preemption within CapacityScheduler's leaf-queue
> ---------------------------------------------------------------
>
>                 Key: YARN-2113
>                 URL: https://issues.apache.org/jira/browse/YARN-2113
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: scheduler
>            Reporter: Vinod Kumar Vavilapalli
>            Assignee: Sunil G
>         Attachments: TestNoIntraQueuePreemptionIfBelowUserLimitAndDifferentPrioritiesWithExtraUsers.txt,
YARN-2113.0001.patch, YARN-2113.0002.patch, YARN-2113.0003.patch, YARN-2113.0004.patch, YARN-2113.v0.patch
>
>
> Preemption today only works across queues and moves around resources across queues per
demand and usage. We should also have user-level preemption within a queue, to balance capacity
across users in a predictable manner.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

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


Mime
View raw message