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

Miklos Szegedi commented on YARN-5600:
--------------------------------------

Thank you for the review [~templedf]!

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.
  I do not necessarily agree with using a long of milliseconds here. The value 
is a time stamp and Date is the natural representation. I just use the standard 
Date functions.
  Date does not bring in timezone concerns, it is implemented with the same 
long inside. It is supposed to be timezone independent. A custom implementation 
may add review overhead.
  I could try MonotonicClock, but it uses System.nanoTime(). It is not safe to 
use System.nanoTime() across threads and getDelayedDeletionTime() may be called 
from multiple threads.

The DeletionService.scheduleFileDeletionTask() methods can and probably should 
be private.
   Unfortunately ResourceLocalizationService#cleanUpFilesPerUserDir uses it, so 
I have to keep it public.
   I agree that this needs to be refactored. Feel free to file a JIRA.

In your tests, instead of sleeping and asserting, sleep for short periods in a 
loop to minimize the test time.
  testDelayedDeletionContainerOnly(), testDelayedDeletion(): I need to wait as 
long as the specified retention time but I added the suggested pattern once 
that has elapsed.
  testDelayedKeep(): the condition is that nothing happens for X amount of 
time, so the wait is expected. I am happy to accept suggestions on the wait 
time, here.
    I think 200ms should be enough to catch any pending deletes.

> 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