[
https://issues.apache.org/jira/browse/YARN-6677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16496950#comment-16496950
]
Miklos Szegedi commented on YARN-6677:
--------------------------------------
Thanks, for the update [~haibochen].
{code:java}
40 /**
41 * The timestamp when the container start request is received.
42 * @return
43 */{code}
I think the @return is not needed
{code:java}
57 boolean isC1Opportunistic = isOpportunistic(c1);
58 boolean isC2Opportunistic = isOpportunistic(c2);
59 if (isC1Opportunistic == isC2Opportunistic) {{code}
It is probable shorter to do a {{Boolean isC1Opportunistic =
isOpportunistic(c1); and do a int level0 =
isC1Opportunistic.compareTo(isC2Opportunistic)...
{code:java}
61 long order = c1.getContainerLaunchTime() - c2.getContainerLaunchTime();
62 return order > 0 ? -1 : order < 0 ? 1 : 0;{code}
Similarly this could be {{-1 *
((Long)c1.getContainerLaunchTime()).compareTo((Long)c2.getContainerLaunchTime())}}
{code:java}
97 private boolean killGuaranteedContainerIfOOM(
98 Container container, String fileName) {
99 assert(!isOpportunistic(container));{code}
I do not see any reasons to restrict to Guaranteed here. The logic is not
specific to guaranteed, I would keep the original naming. Anyone can use these
for opportunistic or any other type of containers in the future.
{code}
172 * In general we try to find a newly run container that exceeded its
limits.
173 * The justification is cost, since probably this is the one that has
174 * accumulated the least amount of uncommitted data so far.
175 * We continue the process until the OOM is resolved.
{code}
This comment is removed. I think it would be useful to have something like this
for the comparator above.
{code}
242 for (Container container : containers) {
243 boolean isOpportunistic = isOpportunistic(container);
244 if (isOpportunistic) {
{code}
I would reconsider this logic. It would be so much simpler to compare the usage
vs. request for opportunistic containers as well. If I remember well the way
[~kasha] designed oversubscription is to be backward compatible. This means
that if I run some containers as opportunistic and get them promoted
eventually, the ordering should not be different as opposed to I launch them as
guaranteed. This current logic has a conflict with this design. if o1 was
launched before o2 but o2 does not exceed it's request, it will be killed
first. If the same containers run as c1 and c2, c2 will remain and c1 will be
killed. This means that oversubscription may have a regression in cluster
utilization and fairness. This would mean that we apply the logic
killGuaranteedContainerIfOOM for opportunistic containers as well.
{code}
288 return container.getContainerTokenIdentifier() != null &&
{code}
The default should be guaranteed, not opportunistic.
{code}
58 * Test an OOM situation where there is no containers that can be
killed.
{code}
is->are
> Preempt opportunistic containers when root container cgroup goes over memory
> limit
> ----------------------------------------------------------------------------------
>
> Key: YARN-6677
> URL: https://issues.apache.org/jira/browse/YARN-6677
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager
> Affects Versions: 3.0.0-alpha3
> Reporter: Haibo Chen
> Assignee: Miklos Szegedi
> Priority: Major
> Attachments: YARN-6677.00.patch, YARN-6677.01.patch
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
