hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jing Zhao (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-6757) Simplify lease manager with INodeID
Date Mon, 28 Jul 2014 22:21:41 GMT

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

Jing Zhao commented on HDFS-6757:
---------------------------------

The patch looks good to me. One issue is that the current patch cannot handle the scenario
where an under construction file, which is contained in a snapshot, is deleted. The original
code still calls {{removeLeaseWithPrefixPath}} in {{removePathAndBlocks}}, thus even if {{removedINodes}}
is empty or does not contain UC files that are in snapshots the corresponding leases are still
updated. Now only updating the leaseMap based on {{removedINodes}} cannot update these leases.

Maybe we can use another list to collect all the UC files in {{cleanSubtree}}. But we need
to make sure all the files are touched by {{cleanSubtree}} and more unit tests need to be
added.

Besides, some minors and nits:
# Considering {{getFullPathName}} is a heavy recursive call, and when we call
  {{removeLease}} from {{finalizeINodeFileUnderConstruction}} we already know the
  full path, maybe here we can either pass in the full path as a parameter?
{code}
-  synchronized void removeLease(String holder, String src) {
+  synchronized void removeLease(String holder, INodeFile src) {
     Lease lease = getLease(holder);
     if (lease != null) {
-      removeLease(lease, src);
+      removeLease(lease, src.getId());
     } else {
       LOG.warn("Removing non-existent lease! holder=" + holder +
-          " src=" + src);
+          " src=" + src.getFullPathName());
     }
   }
{code}
# The following code needs some clean:
{code}
+              LOG.debug("Lease recovery for inode " + id + " is complete. " +
+                      "File" +
+                      " closed.");
{code}

{code}
-    ((Log4JLogger)DataNode.LOG).getLogger().setLevel(Level.ALL);
+    //((Log4JLogger)DataNode.LOG).getLogger().setLevel(Level.ALL);
{code}


> Simplify lease manager with INodeID
> -----------------------------------
>
>                 Key: HDFS-6757
>                 URL: https://issues.apache.org/jira/browse/HDFS-6757
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>            Reporter: Haohui Mai
>            Assignee: Haohui Mai
>         Attachments: HDFS-6757.000.patch, HDFS-6757.001.patch, HDFS-6757.002.patch, HDFS-6757.003.patch
>
>
> Currently the lease manager records leases based on path instead of inode ids. Therefore,
the lease manager needs to carefully keep track of the path of active leases during renames
and deletes. This can be a non-trivial task.
> This jira proposes to simplify the logic by tracking leases using inodeids instead of
paths.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message