[
https://issues.apache.org/jira/browse/YARN-6672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16528028#comment-16528028
]
Íñigo Goiri commented on YARN-6672:
-----------------------------------
[~haibochen], thanks for [^YARN-6672-YARN-1011.05.patch].
bq. I renamed it to ContainerManagerForTest, which is very generic, for the
lack of a descriptive name that I can think of. Any suggestion is appreciated.
I think this name is fine given it is in the base for the unit tests.
bq. Can you point out in which unit tests you see this? I seem to miss this
while trying to find in the node.
I was referring to tests like
{{testPreemptOpportunisticContainersUponHighMemoryUtilization()}}.
Now the comment makes it clear and the unit tests is very easy to follow.
As you said, I don't think we should add comments to the old unit tests for now.
This looks good to me.
Minor comments:
* Why is checkHighUtilization() public?
* We should add javadocs for checkUtilization(), checkLowUtilization(), and
checkHighUtilization().
* In ContainersMonitorImpl#1114 should use the {} logger style instead of
append.
* Add a javadoc comment for {{ContainerManagerForTest}} (similar to
{{ContainerMonitorForTest}}).
* Javadocs for ContainerManagerForTest#checkNodeResourceUtilization() and
ContainerManagerForTest#drainAsyncEvents().
* TestContainerSchedulerWithOverAllocation#886 has weird spacing.
* In testPreemptOpportunisticContainersUponHighMemoryUtilization, can we force
contianer 3 to be queued in the beginning and check that it's queued and not
running? Then we check utilization and let it run.
* A couple of the checkstyles can be solved.
Other than this minor comments, this looks good to go to me.
I would like somebody else to double check but +1 from my side once the nits
are solved.
In a more general scope (YARN-1011), there is one thing left that [~cheersyang]
mentioned offline: what happens when containers are starting and utilization is
still low.
We need to create a utilization policy that is conservative and does not assign
lots of load to nodes with young containers.
Anyway, this is a topic for some other JIRA.
In general, we need some documentation specifying all the options.
I think this will be there in YARN-7334; we may want to start iterating in that
JIRA.
> Add NM preemption of opportunistic containers when utilization goes high
> ------------------------------------------------------------------------
>
> Key: YARN-6672
> URL: https://issues.apache.org/jira/browse/YARN-6672
> Project: Hadoop YARN
> Issue Type: Sub-task
> Affects Versions: 3.0.0-alpha3
> Reporter: Haibo Chen
> Assignee: Haibo Chen
> Priority: Major
> Attachments: YARN-6672-YARN-1011.00.patch,
> YARN-6672-YARN-1011.01.patch, YARN-6672-YARN-1011.02.patch,
> YARN-6672-YARN-1011.03.patch, YARN-6672-YARN-1011.04.patch,
> YARN-6672-YARN-1011.05.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]
