Wangda Tan commented on YARN-2933:

In addition to previously comment, I think we put incorrect #container for each 
application when setLabelContainer=true. The "usedResource" or "current" in 
TestProportionalPreemptionPolicy actually means "used resource of nodes without 
label". So if we want to have labeled container in an application, we should 
make it stay outside of "usedResource".

So in the patch, before:
     for (int i = 0; i < used; i += gran) {
       if(setAMContainer && i == 0){
         cLive.add(mockContainer(appAttId, cAlloc, unit, 0));
-      }else{
+      }else if(setLabelContainer && i ==1){
+        cLive.add(mockContainer(appAttId, cAlloc, unit, 2));
+      }
We should add 
+    if (setLabelContainer) {
+      used++;
+    }
To make it correct. 

And {{testSkipLabeledContainer}} is fully covered by 
{{testIdealAllocationForLabels}}. Since we have already checked #container 
preempted in each application in {{testIdealAllocationForLabels}}, which 
implies labeled containers are ignored.

A minor suggest is rename {{setLabelContainer}} to {{setLabeledContainer}}


> Capacity Scheduler preemption policy should only consider capacity without 
> labels temporarily
> ---------------------------------------------------------------------------------------------
>                 Key: YARN-2933
>                 URL: https://issues.apache.org/jira/browse/YARN-2933
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: capacityscheduler
>            Reporter: Wangda Tan
>            Assignee: Mayank Bansal
>         Attachments: YARN-2933-1.patch, YARN-2933-2.patch, YARN-2933-3.patch, 
> YARN-2933-4.patch, YARN-2933-5.patch
> Currently, we have capacity enforcement on each queue for each label in 
> CapacityScheduler, but we don't have preemption policy to support that. 
> YARN-2498 is targeting to support preemption respect node labels, but we have 
> some gaps in code base, like queues/FiCaScheduler should be able to get 
> usedResource/pendingResource, etc. by label. These items potentially need to 
> refactor CS which we need spend some time carefully think about.
> For now, what immediately we can do is allow calculate ideal_allocation and 
> preempt containers only for resources on nodes without labels, to avoid 
> regression like: A cluster has some nodes with labels and some not, assume 
> queueA isn't satisfied for resource without label, but for now, preemption 
> policy may preempt resource from nodes with labels for queueA, that is not 
> correct.
> Again, it is just a short-term enhancement, YARN-2498 will consider 
> preemption respecting node-labels for Capacity Scheduler which is our final 
> target. 

This message was sent by Atlassian JIRA

Reply via email to