hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Giovanni Matteo Fumarola (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (YARN-999) In case of long running tasks, reduce node resource should balloon out resource quickly by calling preemption API and suspending running task.
Date Fri, 05 Apr 2019 19:07:00 GMT

    [ https://issues.apache.org/jira/browse/YARN-999?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16811171#comment-16811171
] 

Giovanni Matteo Fumarola edited comment on YARN-999 at 4/5/19 7:06 PM:
-----------------------------------------------------------------------

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.

[Static import documentation|https://docs.oracle.com/javase/8/docs/technotes/guides/language/static-import.html)].
I would remove those from the new tests classes.


was (Author: giovanni.fumarola):
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: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message