[
https://issues.apache.org/jira/browse/YARN-5600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15729872#comment-15729872
]
Daniel Templeton commented on YARN-5600:
----------------------------------------
Thanks, [[email protected]] for all the updates. I'm going to stir
the pot a little to see if we can get this one closed. :)
Comments:
* The blank lines in {{ApplicationConstants.DEBUG_DELETE_DELAY}} should have
the leading asterisk
* I think the javadoc for {{YarnConfiguration.
DEBUG_NM_MAX_PER_APPLICATION_DELETE_DELAY_SEC}} is wrong.
* In {{yarn-default.xml}}, let's not reference code if we can help it: {{refer
to Environment.DEBUG_DELETE_DELAY}}. Better to just name the env var directly.
* Language: {{prevent unreliable or malicious clients keep files arbitrarily.}}
should be {{prevent unreliable or malicious clients from keeping files
arbitrarily.}}
* In {{DeletionService. getEffectiveDelaySec()}}, this code seems unnecessarily
verbose: {code} int effectiveDelay;
effectiveDelay = Math.max(debugDelayDefault, limitedDelay);
return effectiveDelay;{code} It could just be a return.
* The javadoc on this method seems off: {code} /**
* Peek the beginning of the queue.
* @return scheduled task count
*/
@VisibleForTesting
ScheduledThreadPoolExecutor getSched() {
return sched;
}{code}
* In {{serviceInit()}}, the indention looks wrong here: {code}
ThreadFactory tf = new ThreadFactoryBuilder()
.setNameFormat("DeletionService #%d")
.build();{code}
* In {{serviceInit()}}, you're missing the space before the brace on both your
_if_ statements. Also, do you think you can find a sane way to combine them or
at least reuse the error message?
* In {{ApplicationImpl. getDelayedDeletionTime()}} javadoc, "container" is
misspelled.
* I'm still hung up on {{+999 / 1000}} in {{ResourceLocalizationService}}. How
about {{Math.ceil(... / 1000.0)}}?
* In {{TestContainerManager}}, I'm worried that the new tests are pretty
heavily timing dependent. At the very least, the second wait should be for the
full timeout period. Even better would be to validate the internal state is as
expected, e.g. that the deletion thread is set to execute after the expected
amount of time.
* In {{TestDeletionService.testCustomDisableDelete()}}, do you need to set
{{DEBUG_NM_MAX_PER_APPLICATION_DELETE_DELAY_SEC}}? Also, you're missing an
assert message in that test method further down.
* {{testCustomRetentionPolicy()}} should probably also test what happens if no
container override is given, if the container override is 0, and if it's 2.
> 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, YARN-5600.003.patch, YARN-5600.004.patch,
> YARN-5600.005.patch, YARN-5600.006.patch, YARN-5600.007.patch,
> YARN-5600.008.patch, YARN-5600.009.patch, YARN-5600.010.patch,
> YARN-5600.011.patch, YARN-5600.012.patch, YARN-5600.013.patch,
> YARN-5600.014.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: [email protected]
For additional commands, e-mail: [email protected]
