Eric Payne commented on YARN-2009:

- {{IntraQueuePreemptableResourceCalculator#computeIntraQueuePreemptionDemand}}:
-- Shouldn't the following be {{tq.getUsed() - tq.getActuallyToBePreempted()}}? 
{{tq.getGuaranteed()}} only returns the queue's guaranteed capacity but if apps 
in the queue are using extra resources, then you want to subtract from the 
total usage.
-- {{MaxIgnoredOverCapacity}}
        if (leafQueue.getUsedCapacity() < context
            .getMaxIgnoredOverCapacityForIntraQueue()) {
--- Shouldn't this also take into consideration used capacity of all parent 
queues as well?
--- In any case, can we change the name of the config property and its getters? 
{{CapacitySchedulerConfiguration#MAX_IGNORED_OVER_CAPACITY_FOR_INTRA_QUEUE}} / 
 / {{ProportionalCapacityPreemptionPolicy#maxIgnoredOverCapacityForIntraQueue}}
---- This is not really an "over capacity" thing. It's more of an "only start 
to preempt when queue is over this amount" thing. Maybe we could name it 
something like 

- {{FifoIntraQueuePreemptionPolicy#createTempAppForResourceCalculation}}
-- In the following code, instead of calling the {{containsKey}} or the 
{{get*}} method twice, you could just call the get method, assign its output to 
a tmp var, and then if the tmp var is not null, then assign tmp to the resource 
var. That would just be a little more efficient.
      if (app.getTotalPendingRequestsPerPartition().containsKey(partition)) {
      if (null != app.getAppAttemptResourceUsage().getUsed(partition)) {
      if (null != app.getCurrentReservation(partition)) {
-- Should {{pending}} also be cloned?
      TempAppPerPartition tmpApp = new TempAppPerPartition(app.getQueueName(),
          app.getApplicationId(), Resources.clone(used),
          Resources.clone(reserved), pending, app.getPriority().getPriority(),
          app, partitions);

- {{FifoIntraQueuePreemptionPolicy#computeAppsIdealAllocation}}:
-- Can you please change the following comment:
    // Apps ordered from highest to lower priority.
--- to be something like this?
    // Remove the app at the next highest remaining priority and process it.
-- Can you please change the word "size" to "resources"? When I first saw 
"container size", I thought it was calculating the size of each container.
      // Calculate total selected container size from current app.
-- I don't think we want to do the following:
      if (Resources.lessThanOrEqual(rc, partitionBasedResource, userHeadroom,
          Resources.none())) {
--- If {{user1}} has used the entire queue with a low priority app, {{user1}}'s 
headroom will be 0. But, if that same user starts a higher priority app, that 
higher priority app needs to preempt from the lower priority app, doesn't it?
-- I assume that you will rework the {{idealAssigned}} logic to match 
[~leftnoteasy]'s algorithm [that he provided 
 That is, the algorithm that takes into account {{user-limit-resource}}

- {{FifoIntraQueuePreemptionPolicy#getHighPriorityApps}}:
-- The comments in this method are the same as those in 
{{AbstractPreemptableResourceCalculator#getMostUnderservedQueues}}, but they 
don't apply for {{getHighPriorityApps}}.
-- {{getHighPriorityApps}} doesn't need to return 
{{ArrayList<TempAppPerPartition>}}. It will only be retrieving one app at a 
time. In {{AbstractPreemptableResourceCalculator#getMostUnderservedQueues}}, 
it's possible for 2 or more queues to be underserved by exactly the same 
amount, so all of the most underserved queues must be processed together. 
However, {{getHighPriorityApps}} is using the {{taComparator}} comparator. Even 
if apps are the same priority, one will have a lower app ID, so there will 
never be 2 apps that compare equally.

> Priority support for preemption in ProportionalCapacityPreemptionPolicy
> -----------------------------------------------------------------------
>                 Key: YARN-2009
>                 URL: https://issues.apache.org/jira/browse/YARN-2009
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: capacityscheduler
>            Reporter: Devaraj K
>            Assignee: Sunil G
>         Attachments: YARN-2009.0001.patch, YARN-2009.0002.patch
> While preempting containers based on the queue ideal assignment, we may need 
> to consider preempting the low priority application containers first.

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