[
https://issues.apache.org/jira/browse/YARN-9041?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16704014#comment-16704014
]
Yufei Gu commented on YARN-9041:
--------------------------------
Hi [~jiwq], the patch v5 looks good in terms of logic. Some nits:
# Can you rename two tests or write comments to clarify their intentions? I
suppose the goals of two methods are: One can find less AM containers solution
in the relax locations and the other can't.
# It is a good practice to put the callee methods under the caller methods.
# Can you refactor to create a new method like this? Please remember to
reorganize the method java doc. And we probably don't need
the comment "// Don't preempt AM containers just to satisfy local requests if
relax // locality is enabled." in that case.
{code}
/**
* Iterate through matching
* nodes and identify containers to preempt all on one node, also
* * optimizing for least number of AM container preemptions. Only nodes
* * that match the locality level specified in the {@link ResourceRequest}
* * are considered. However, if this would lead to AM preemption, and
locality
* * relaxation is allowed, then the search space is expanded to the
remaining
* * nodes.
*
* @param rr
* @param potentialNodes
* @return
*/
private PreemptableContainers getBestPreemptableContainers(ResourceRequest
rr, List<FSSchedulerNode> potentialNodes) {
PreemptableContainers bestContainers =
identifyContainersToPreemptForOneContainer(potentialNodes, rr);
if (rr.getRelaxLocality()
&& !ResourceRequest.isAnyLocation(rr.getResourceName())
&& bestContainers != null
&& bestContainers.numAMContainers > 0) {
List<FSSchedulerNode> remainingNodes =
scheduler.getNodeTracker().getAllNodes();
remainingNodes.removeAll(potentialNodes);
PreemptableContainers spareContainers =
identifyContainersToPreemptForOneContainer(remainingNodes, rr);
if (spareContainers != null && spareContainers.numAMContainers <
bestContainers.numAMContainers) {
bestContainers = spareContainers;
}
}
return bestContainers;
}
{code}
> Optimize FSPreemptionThread#identifyContainersToPreempt method
> --------------------------------------------------------------
>
> Key: YARN-9041
> URL: https://issues.apache.org/jira/browse/YARN-9041
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: scheduler preemption
> Reporter: Wanqiang Ji
> Assignee: Wanqiang Ji
> Priority: Major
> Attachments: YARN-9041.001.patch, YARN-9041.002.patch,
> YARN-9041.003.patch, YARN-9041.004.patch, YARN-9041.005.patch
>
>
> In FSPreemptionThread#identifyContainersToPreempt method, I suggest if AM
> preemption, and locality relaxation is allowed, then the search space is
> expanded to all nodes changed to the remaining nodes. The remaining nodes are
> equal to all nodes minus the potential nodes.
> Judging condition changed to:
> # rr.getRelaxLocality()
> # !ResourceRequest.isAnyLocation(rr.getResourceName())
> # bestContainers != null
> # bestContainers.numAMContainers > 0
> If I understand the deviation, please criticize me. thx~
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]