hadoop-yarn-issues mailing list archives

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

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

Zhijie Shen commented on YARN-661:
----------------------------------

I agree with the idea, and the patch looks generally good. Bellow are some comments.

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

* Instead of synchronizing over HashMap, how about using ConcurrentHashMap instead?
{code}
+        if (!this.deletionTaskDependencyMap.containsKey(parentTask)) {
+          this.deletionTaskDependencyMap.put(parentTask,
+            new ArrayList<FileDeletion>());
+        }
+        List<FileDeletion> dependentTaskList =
+            this.deletionTaskDependencyMap.get(parentTask);
{code}
can be simplified as
{code}
+        List<FileDeletion> dependentTaskList =
+            this.deletionTaskDependencyMap.putIfAbsent(
+                parentTask, new ArrayList<FileDeletion>());
{code}

* 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.
{code}
+  public void populateFileDeletionTaskDependency(List<FileDeletion> parentTasks,
+      List<FileDeletion> childDependentTasks) {
{code}

* How about calling it delete as well, just overloading the method?
{code}
+  public void deleteHelper(FileDeletion fileDeletion) {
{code}

* Please use LocalResource.newInstance instead.
{code}
+    LocalResource localResource = Records.newRecord(LocalResource.class);
{code}

* There's one finbugs warning to fix.

* In the following method
{code}
+    public boolean matches(Object o) {
{code}
How about refactoring the code in the following pattern, which should be more clear.
{code}
if (obj1 == null && obj2 != null) {
  return false;
} else if (obj1 != null && obj2 == null) {
  return false;
} else if (obj1 != null && obj2 != null) {
  // your logic
}
{code}
                
> 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

Mime
View raw message