hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Omkar Vinit Joshi (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-661) NM fails to cleanup local directories for users
Date Thu, 11 Jul 2013 02:07:49 GMT

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

Omkar Vinit Joshi commented on YARN-661:

Thanks [~zjshen] .. uploading updated patch..

bq. Not sure getCancelIfAnyDependentTaskFailedFlag is necessary. If deletion of the sub-dir
fails, should deletion of the root-dir fail automatically?

In this scenario we want to cancel this but may not be the case always. I will remove it but
that would have been an added control for user.

bq. Instead of synchronizing over HashMap, how about using ConcurrentHashMap instead?

yeah fixed it.. 
+        List<FileDeletion> dependentTaskList =
+            this.deletionTaskDependencyMap.putIfAbsent(
+                parentTask, new ArrayList<FileDeletion>());
there is a problem with this as we end up creating new objects for every call. It is present
at lot of places in yarn and need to be fixed. the containsKey check even though looks complicated
avoids extra object creation on heap.

bq. WRT the dependency, I'd like rather call task pair predecessor and successor instead of
parent and child, because it's a dag not a tree. Moreover, how about defining public void
populateFileDeletionTaskDependency(List<FileDeletion>, FileDeletion), and populateFileDeletionTaskDependency(List<FileDeletion>,
List<FileDeletion>) wraps it. In fact, the former one seems to be enough for the patch.

yeah renaming parent -> predecessor and child -> successor. I think current (List<>,
List<>) api is more flexible and we can keep it that way.

bq. How about calling it delete as well, just overloading the method?
yeah wanted to overload only but is not helping me in junit tests. MockitTo.verify is not
smart enough to distinguish between overloaded methods...hence giving different name...
    verify(delService, times(1)).deleteHelper(
      argThat(new FileDeletionInclude(user, null,
        new String[] { destinationFile })));
    verify(delService, times(1)).deleteHelper(
      argThat(new FileDeletionInclude(null, ContainerLocalizer.USERCACHE
          + "_DEL_", new String[] {})));

bq. There's one finbugs warning to fix.
Hopefully fixed it.. I think it is complaining because I am exposing final array. This too
I need it for testing. Let me see if findbug complains again then will fix it in some other

bq. Please use LocalResource.newInstance instead. 
not related to this patch ..was just reformatting..but fixed it ..trivial fix.

bq. How about refactoring the code in the following pattern, which should be more clear.
makes sense... fixed it..

> NM fails to cleanup local directories for users
> -----------------------------------------------
>                 Key: YARN-661
>                 URL: https://issues.apache.org/jira/browse/YARN-661
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: nodemanager
>    Affects Versions: 2.1.0-beta, 0.23.8
>            Reporter: Jason Lowe
>            Assignee: Omkar Vinit Joshi
>         Attachments: YARN-661-20130701.patch, YARN-661-20130708.patch
> YARN-71 added deletion of local directories on startup, but in practice it fails to delete
the directories because of permission problems.  The top-level usercache directory is owned
by the user but is in a directory that is not writable by the user.  Therefore the deletion
of the user's usercache directory, as the user, fails due to lack of permissions.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

View raw message