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?
+        if (!this.deletionTaskDependencyMap.containsKey(parentTask)) {
+          this.deletionTaskDependencyMap.put(parentTask,
+            new ArrayList<FileDeletion>());
+        }
+        List<FileDeletion> dependentTaskList =
+            this.deletionTaskDependencyMap.get(parentTask);
can be simplified as
+        List<FileDeletion> dependentTaskList =
+            this.deletionTaskDependencyMap.putIfAbsent(
+                parentTask, new ArrayList<FileDeletion>());

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

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

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

* There's one finbugs warning to fix.

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