[ 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