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

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.
{code}
        tq.setUnAllocated(Resources.subtract(tq.getGuaranteed(),
            tq.getActuallyToBePreempted()));
{code}
-- {{MaxIgnoredOverCapacity}}
{code}
        if (leafQueue.getUsedCapacity() < context
            .getMaxIgnoredOverCapacityForIntraQueue()) {
          continue;
        }
{code}
--- 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}} / 
{{yarn.resourcemanager.monitor.capacity.preemption.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 
{{yarn.resourcemanager.monitor.capacity.preemption.ignore_below_percent_of_queue}}

-----
- {{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.
{code}
      if (app.getTotalPendingRequestsPerPartition().containsKey(partition)) {
...
      if (null != app.getAppAttemptResourceUsage().getUsed(partition)) {
...
      if (null != app.getCurrentReservation(partition)) {
{code}
-- Should {{pending}} also be cloned?
{code}
      TempAppPerPartition tmpApp = new TempAppPerPartition(app.getQueueName(),
          app.getApplicationId(), Resources.clone(used),
          Resources.clone(reserved), pending, app.getPriority().getPriority(),
          app, partitions);
{code}

-----
- {{FifoIntraQueuePreemptionPolicy#computeAppsIdealAllocation}}:
-- Can you please change the following comment:
{code}
    // Apps ordered from highest to lower priority.
{code}
--- to be something like this?
{code}
    // Remove the app at the next highest remaining priority and process it.
{code}
-- 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.
{code}
      // Calculate total selected container size from current app.
{code}
-- I don't think we want to do the following:
{code}
      if (Resources.lessThanOrEqual(rc, partitionBasedResource, userHeadroom,
          Resources.none())) {
        continue;
      }
{code}
--- 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 
above|https://issues.apache.org/jira/browse/YARN-2009?focusedCommentId=15504978&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15504978].
 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
(v6.3.4#6332)

---------------------------------------------------------------------
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