Revert "HDFS-10763. Open files can leak permanently due to inconsistent lease update. Contributed by Kihwal Lee."

This reverts commit 71d0e4fca79d4305fe35e44b614703d3b9883017.

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -255,9 +255,6 @@ Release 2.7.3 - UNRELEASED
 
     HDFS-9696. Garbage snapshot records linger forever. (kihwal)
 
-    HDFS-10763. Open files can leak permanently due to inconsistent lease
-    update. (kihwal)
-
 Release 2.7.2 - 2016-01-25
 
   INCOMPATIBLE CHANGES

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java

@@ -220,13 +220,11 @@ public final class FSImageFormatPBINode {
     private final FSDirectory dir;
     private final FSNamesystem fsn;
     private final FSImageFormatProtobuf.Loader parent;
-    private final List<INodeFile> ucFiles;
 
     Loader(FSNamesystem fsn, final FSImageFormatProtobuf.Loader parent) {
       this.fsn = fsn;
       this.dir = fsn.dir;
       this.parent = parent;
-      this.ucFiles = new ArrayList<INodeFile>();
     }
 
     void loadINodeDirectorySection(InputStream in) throws IOException {
@@ -270,20 +268,17 @@ public final class FSImageFormatPBINode {
      * Load the under-construction files section, and update the lease map
      */
     void loadFilesUnderConstructionSection(InputStream in) throws IOException {
-     // This section is consumed, but not actually used for restoring leases.
       while (true) {
         FileUnderConstructionEntry entry = FileUnderConstructionEntry
             .parseDelimitedFrom(in);
         if (entry == null) {
           break;
         }
-      }
-
-      // Add a lease for each and every file under construction.
-      for (INodeFile file : ucFiles) {
+        // update the lease manager
+        INodeFile file = dir.getInode(entry.getInodeId()).asFile();
         FileUnderConstructionFeature uc = file.getFileUnderConstructionFeature();
         Preconditions.checkState(uc != null); // file must be under-construction
-        fsn.leaseManager.addLease(uc.getClientName(), file.getFullPathName());
+        fsn.leaseManager.addLease(uc.getClientName(), entry.getFullPath());
       }
     }
 
@@ -351,7 +346,6 @@ public final class FSImageFormatPBINode {
 
       // under-construction information
       if (f.hasFileUC()) {
-        ucFiles.add(file);
         INodeSection.FileUnderConstructionFeature uc = f.getFileUC();
         file.toUnderConstruction(uc.getClientName(), uc.getClientMachine());
         if (blocks.length > 0) {

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -4205,6 +4205,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
       throw new IOException("Cannot finalize file " + src
           + " because it is not under construction");
     }
+    leaseManager.removeLease(uc.getClientName(), src);
     
     pendingFile.recordModification(latestSnapshot);
 
@@ -4213,8 +4214,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
     // since we just remove the uc feature from pendingFile
     pendingFile.toCompleteFile(now());
 
-    leaseManager.removeLease(uc.getClientName(), src);
-
     waitForLoadingFSImage();
     // close file and persist block allocations for this file
     closeFile(src, pendingFile);

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLeaseManager.java

@@ -21,13 +21,9 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
-import org.apache.hadoop.fs.FSDataOutputStream;
-import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hdfs.DistributedFileSystem;
 import org.apache.hadoop.hdfs.HdfsConfiguration;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
-import org.apache.hadoop.hdfs.protocol.HdfsConstants.SafeModeAction;
 import org.junit.Test;
 import org.mockito.Mockito;
 
@@ -85,43 +81,4 @@ public class TestLeaseManager {
     //Initiate a call to checkLease. This should exit within the test timeout
     lm.checkLeases();
   }
-
-  /**
-   * Make sure the lease is restored even if only the inode has the record.
-   */
-  @Test
-  public void testLeaseRestorationOnRestart() throws Exception {
-    MiniDFSCluster cluster = null;
-    try {
-      cluster = new MiniDFSCluster.Builder(new HdfsConfiguration())
-          .numDataNodes(1).build();
-      DistributedFileSystem dfs = cluster.getFileSystem();
-
-      // Create an empty file
-      String path = "/testLeaseRestorationOnRestart";
-      FSDataOutputStream out = dfs.create(new Path(path));
-
-      // Remove the lease from the lease manager, but leave it in the inode.
-      FSDirectory dir = cluster.getNamesystem().getFSDirectory();
-      INodeFile file = dir.getINode(path).asFile();
-      cluster.getNamesystem().leaseManager.removeLease(
-          file.getFileUnderConstructionFeature().getClientName(), path);
-
-      // Save a fsimage.
-      dfs.setSafeMode(SafeModeAction.SAFEMODE_ENTER);
-      cluster.getNameNodeRpc().saveNamespace();
-      dfs.setSafeMode(SafeModeAction.SAFEMODE_LEAVE);
-
-      // Restart the namenode.
-      cluster.restartNameNode(true);
-
-      // Check whether the lease manager has the lease
-      assertNotNull("Lease should exist",
-          cluster.getNamesystem().leaseManager.getLeaseByPath(path));
-    } finally {
-      if (cluster != null) {
-        cluster.shutdown();
-      }
-    }
-  }
 }