[
https://issues.apache.org/jira/browse/YARN-999?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16811171#comment-16811171
]
Giovanni Matteo Fumarola commented on YARN-999:
-----------------------------------------------
Thanks [~elgoiri] for the hard work to add this important feature.
The patch is fully tested with good unit tests.`
Few comments on the last patch:
* Few times you used containers launched. It should be launched containers.
* Line 253 LOG.debug("{} is overcommitted ({}), preempt/kill containers",
should be LOG.info. I would like to search in the code what the situation.
* You can use the word Overcommitted over "over committed".
* In testGetRunningContainersToKill some comments have the numeration missing.
e.g AM instead of AM0.
* Line 1069 false, ExecutionType.GUARANTEED, "GUARANTEED0"); it should
be GUARANTEED1
* I am not a fan of Static imports. when should you use static import? Very
sparingly! Only use it when you'd otherwise be tempted to declare local copies
of constants, or to abuse inheritance (the Constant Interface Antipattern). ...
If you overuse the static import feature, it can make your program unreadable
and unmaintainable, polluting its namespace with all the static members you
import. Readers of your code (including you, a few months after you wrote it)
will not know which class a static member comes from. Importing all of the
static members from a class can be particularly harmful to readability; if you
need only one or two members, import them individually.
([https://docs.oracle.com/javase/8/docs/technotes/guides/language/static-import.html)
]I would remove those from the new tests classes.
> In case of long running tasks, reduce node resource should balloon out
> resource quickly by calling preemption API and suspending running task.
> -----------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: YARN-999
> URL: https://issues.apache.org/jira/browse/YARN-999
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: graceful, nodemanager, scheduler
> Reporter: Junping Du
> Assignee: Íñigo Goiri
> Priority: Major
> Attachments: YARN-291.000.patch, YARN-999.001.patch,
> YARN-999.002.patch, YARN-999.003.patch, YARN-999.004.patch,
> YARN-999.005.patch, YARN-999.006.patch, YARN-999.007.patch,
> YARN-999.008.patch, YARN-999.009.patch
>
>
> In current design and implementation, when we decrease resource on node to
> less than resource consumption of current running tasks, tasks can still be
> running until the end. But just no new task get assigned on this node
> (because AvailableResource < 0) until some tasks are finished and
> AvailableResource > 0 again. This is good for most cases but in case of long
> running task, it could be too slow for resource setting to actually work so
> preemption could be used here.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
