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

Daniel Templeton commented on YARN-5600:
----------------------------------------

Thanks for the patch, [~miklos.szeg...@cloudera.com].  Some comments:

* It seems to me that you're doing extra work to keep the delete time as a 
{{Date}}, not to mention adding potential time zone concerns.  Millis since the 
epoch may be simpler.
* Ignoring the {{IOException}} in 
{{ResourceLocalizationService.submitDirForDeletion()}} seems bad.  While you're 
in there, it might be good to do something more useful.
* In your javadoc, the param text should start with a lower case letter, e.g. 
{{DeletionService#deleteWithDelay()}}
* The {{DeletionService.scheduleFileDeletionTask()}} methods can and probably 
should be private.
* In your tests, instead of sleeping and asserting, sleep for short periods in 
a loop to minimize the test time.
* In {{TestContainerManager}} you have {code}-    for (File f : new File[] { 
containerDir, containerSysDir }) {
+    for (File f : new File[] {containerDir, containerSysDir }) {{code}  You 
may as well remove the trailing space as well.
* In {{TestContainerManager.verifyContainerDir()}}, your 
if-if-else-else-if-else would be cleaner as if-elseif-elseif-else.  Also, the 
messages could be a little more descriptive so that someone reading it without 
the source code has some clue what's happening.  And I don't think we need the 
exclamation points. :)

Otherwise, the general approach looks fine.

> Add a parameter to ContainerLaunchContext to emulate 
> yarn.nodemanager.delete.debug-delay-sec on a per-application basis
> -----------------------------------------------------------------------------------------------------------------------
>
>                 Key: YARN-5600
>                 URL: https://issues.apache.org/jira/browse/YARN-5600
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: nodemanager
>    Affects Versions: 3.0.0-alpha1
>            Reporter: Daniel Templeton
>            Assignee: Miklos Szegedi
>              Labels: oct16-medium
>         Attachments: YARN-5600.000.patch, YARN-5600.001.patch, 
> YARN-5600.002.patch
>
>
> To make debugging application launch failures simpler, I'd like to add a 
> parameter to the CLC to allow an application owner to request delayed 
> deletion of the application's launch artifacts.
> This JIRA solves largely the same problem as YARN-5599, but for cases where 
> ATS is not in use, e.g. branch-2.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to