[
https://issues.apache.org/jira/browse/YARN-5641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15830718#comment-15830718
]
Jason Lowe commented on YARN-5641:
----------------------------------
Thanks for updating the patch!
I think getThread and the description is a little vague. It would be more
clear if the method were called something like getWaitingThread to indicate
this is the thread waiting for the shell command to return, and the Javadoc
should be more clear that it is the thread in the runCommand method or null if
no thread is waiting for a shell command to complete.
runCommand needs to set the waiting thread field to null after the subprocess
completes.
removeShell does not seem to be appropriate to be public or even necessary.
Shell instances should automatically remove themselves when the subprocess
exits, and if the subprocess hasn't exited then they shouldn't be removed
otherwise. I don't think it's appropriate for the ContainerLocalizer to be
removing shells, or am I missing something?
FSDownloadWrapper should be package-private.
HashSet is not thread safe, and multiple threads can be accessing it at the
same time (e.g.: one thread in the executor pool may be trying to add itself to
the set just as another is trying to call the set's contains method). This
needs to either be a synchronized set or a concurrent set.
Why doesn't the ContainerLocalizerWrapper constructor initialize the spylfs
member? It's odd to call the constructor then always have to poke fields in it.
To help reduce the changes in the patch, I'd recommend creating {{spylfs}},
{{random}}, and {{nmProxy}} local variables to eliminate all the changes that
are simply adding the 'wrapper.' prefix.
Nit: Please use a much smaller check interval on the waitFor calls in the unit
test.
Style Nit: Typically there is no space between the generic class name and the
type specifier (e.g.: {{Map <>}} should be {{Map<>}})
Style Nit: Please do not use wildcard imports
> Localizer leaves behind tarballs after container is complete
> ------------------------------------------------------------
>
> Key: YARN-5641
> URL: https://issues.apache.org/jira/browse/YARN-5641
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: Eric Badger
> Assignee: Eric Badger
> Attachments: YARN-5641.001.patch, YARN-5641.002.patch,
> YARN-5641.003.patch, YARN-5641.004.patch, YARN-5641.005.patch,
> YARN-5641.006.patch, YARN-5641.007.patch
>
>
> The localizer sometimes fails to clean up extracted tarballs leaving large
> footprints that persist on the nodes indefinitely.
--
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
