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

Sunil G commented on YARN-6081:
-------------------------------

Thanks [~leftnoteasy]. Good catch!

We decrement pending resources only if container is allocated (not reserved). 
So ideally we have to deduct reserved memory from pending resource if any. 
Ideally makes sense for me.

Few comments:
1. {{getTotalPendingResourcesConsideringUserLimit}}, Not part of this patch. 
Could have a java doc comment there as well. So it ll be make javadoc also more 
 better?
2. 
{code}
                Resource pending = app.getAppAttemptResourceUsage().getPending(
                    partition);
                if (deductReservedFromPending) {
                  pending = Resources.subtract(pending,
                      app.getAppAttemptResourceUsage().getReserved(partition));
                }
{code}

I have one doubt here. {{pending}} holds a reference of pending resource of 
appAttemptResource usage. Inside {{if(deductReservedFromPending)}} block, that 
reference is getting updated. Is that intentional?

3.  
{code}
                pending = Resources.max(resourceCalculator, lastClusterResource,
                    pending, Resources.none());
{code}
A quick doubt. Why are we using lastClusterResource here?

4. {{testPreemptionNotHappenForSingleReservedQueue}}, comment near verify block 
is confusing.
5. In {{testPendingResourcesConsideringUserLimit}}, could we also try to assert 
the app's pending and reserved too?


> LeafQueue#getTotalPendingResourcesConsideringUserLimit should deduct reserved 
> from pending to avoid unnecessary preemption of reserved container
> ------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: YARN-6081
>                 URL: https://issues.apache.org/jira/browse/YARN-6081
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Wangda Tan
>            Assignee: Wangda Tan
>         Attachments: YARN-6081.001.patch
>
>
> While doing YARN-5864 tests, found an issue when a queue's reserved > 
> pending. PreemptionResourceCalculator will preempt reserved container even if 
> there's only one active queue in the cluster. 
> To fix the problem, we need to deduct reserved from pending when getting 
> total-pending resource for LeafQueue.



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