HDFS-10763. Open files can leak permanently due to inconsistent lease update. Contributed by Kihwal Lee.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/864f878d Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/864f878d Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/864f878d Branch: refs/heads/YARN-2915 Commit: 864f878d5912c82f3204f1582cfb7eb7c9f1a1da Parents: 03dea65 Author: Kihwal Lee <kih...@apache.org> Authored: Mon Aug 15 17:28:09 2016 -0500 Committer: Kihwal Lee <kih...@apache.org> Committed: Mon Aug 15 17:28:09 2016 -0500 ---------------------------------------------------------------------- .../server/namenode/FSImageFormatPBINode.java | 10 ++--- .../hdfs/server/namenode/FSNamesystem.java | 3 +- .../hdfs/server/namenode/TestLeaseManager.java | 47 ++++++++++++++++++++ 3 files changed, 53 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/864f878d/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java index 1ecd947..1456ecf 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSImageFormatPBINode.java @@ -281,18 +281,14 @@ public final class FSImageFormatPBINode { * Load the under-construction files section, and update the lease map */ void loadFilesUnderConstructionSection(InputStream in) throws IOException { + // Leases are added when the inode section is loaded. This section is + // still read in for compatibility reasons. while (true) { FileUnderConstructionEntry entry = FileUnderConstructionEntry .parseDelimitedFrom(in); if (entry == null) { break; } - // 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(), - entry.getInodeId()); } } @@ -371,6 +367,8 @@ public final class FSImageFormatPBINode { if (f.hasFileUC()) { INodeSection.FileUnderConstructionFeature uc = f.getFileUC(); file.toUnderConstruction(uc.getClientName(), uc.getClientMachine()); + // update the lease manager + fsn.leaseManager.addLease(uc.getClientName(), file.getId()); if (blocks.length > 0) { BlockInfo lastBlk = file.getLastBlock(); // replace the last block of file http://git-wip-us.apache.org/repos/asf/hadoop/blob/864f878d/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index be084c5..0621a77 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -3329,7 +3329,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, throw new IOException("Cannot finalize file " + src + " because it is not under construction"); } - leaseManager.removeLease(uc.getClientName(), pendingFile); pendingFile.recordModification(latestSnapshot); @@ -3340,6 +3339,8 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, allowCommittedBlock? numCommittedAllowed: 0, blockManager.getMinReplication()); + leaseManager.removeLease(uc.getClientName(), pendingFile); + // close file and persist block allocations for this file closeFile(src, pendingFile); http://git-wip-us.apache.org/repos/asf/hadoop/blob/864f878d/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLeaseManager.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLeaseManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLeaseManager.java index f823745..6692090 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLeaseManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestLeaseManager.java @@ -24,8 +24,14 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import com.google.common.collect.Lists; +import org.apache.hadoop.fs.FSDataOutputStream; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.fs.permission.PermissionStatus; +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.apache.hadoop.hdfs.server.blockmanagement.BlockInfo; import org.junit.Rule; import org.junit.Test; @@ -109,6 +115,47 @@ public class TestLeaseManager { assertThat(lm.countPath(), is(1L)); } + /** + * 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(), file); + + // Save a fsimage. + dfs.setSafeMode(SafeModeAction.SAFEMODE_ENTER); + cluster.getNameNodeRpc().saveNamespace(0,0); + dfs.setSafeMode(SafeModeAction.SAFEMODE_LEAVE); + + // Restart the namenode. + cluster.restartNameNode(true); + + // Check whether the lease manager has the lease + dir = cluster.getNamesystem().getFSDirectory(); + file = dir.getINode(path).asFile(); + assertTrue("Lease should exist.", + cluster.getNamesystem().leaseManager.getLease(file) != null); + } finally { + if (cluster != null) { + cluster.shutdown(); + } + } + } + private static FSNamesystem makeMockFsNameSystem() { FSDirectory dir = mock(FSDirectory.class); FSNamesystem fsn = mock(FSNamesystem.class); --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
