hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Varun Vasudev (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-6366) Refactor the NodeManager DeletionService to support additional DeletionTask types.
Date Wed, 10 May 2017 09:29:04 GMT

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

Varun Vasudev commented on YARN-6366:
-------------------------------------

Thanks for the patch [~shanekumpf@gmail.com]! Some minor things to clean up -

1)
Should we add the 'user' field to the DeletionTask base class instead of keeping it in the
FileDeletionTask class?

2)
{code}
if (proto.getTaskType() != null) {
{code}
Can you add a check for {code} proto.hasTaskType() {code} and then check if it's null?

3)
{code}
int taskId = proto.getId(); 
{code}
Shouldn't this be in the base converter? It seems to be a common piece of code that every
task type will have to call.

4)
You don't need the ProtoToFileDeletionTaskConverter and DelegatingProtoToDeletionTaskConverter
classes. Please move the convert functions to the ProtoUtils class as static functions.

5)
{code}
+    FileDeletionTask dependentDeletionTask = new FileDeletionTask(del, null,
+        userDirPath, new ArrayList<>());
{code}
Why create a new ArrayList here? You've used "null" in other places.

6)
The new tests you've added need to be renamed to match the naming convention. Invoked test
functions need to be renamed to start with "test" (like testGetUser)

Thanks!

> Refactor the NodeManager DeletionService to support additional DeletionTask types.
> ----------------------------------------------------------------------------------
>
>                 Key: YARN-6366
>                 URL: https://issues.apache.org/jira/browse/YARN-6366
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: nodemanager, yarn
>            Reporter: Shane Kumpf
>            Assignee: Shane Kumpf
>         Attachments: YARN-6366.001.patch, YARN-6366.002.patch, YARN-6366.003.patch, YARN-6366.004.patch,
YARN-6366.005.patch, YARN-6366.006.patch
>
>
> The NodeManager's DeletionService only supports file based DeletionTask's. This makes
sense as files (and directories) have been the primary concern for clean up to date. With
the addition of the Docker container runtime, addition types of DeletionTask are likely to
be required, such as deletion of docker container and images. See YARN-5366 and YARN-5670.
This issue is to refactor the DeletionService to support additional DeletionTask's.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
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