Repository: hadoop Updated Branches: refs/heads/branch-2.6 8ebf8965d -> a1272d5fc
HDFS-10763. Open files can leak permanently due to inconsistent lease update. Contributed by Kihwal Lee. (cherry picked from commit 61e1348fe479872b0eb1b53c50b248b33b37d77b) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/a1272d5f Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/a1272d5f Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/a1272d5f Branch: refs/heads/branch-2.6 Commit: a1272d5fc019608d3ddca2c8ed2e03f2ddc7a0d7 Parents: 8ebf896 Author: Kihwal Lee <kih...@apache.org> Authored: Thu Aug 18 16:38:25 2016 -0500 Committer: Sangjin Lee <sj...@apache.org> Committed: Wed Sep 14 14:30:41 2016 -0700 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../server/namenode/FSImageFormatPBINode.java | 17 ++++++-- .../hdfs/server/namenode/FSNamesystem.java | 3 +- .../hdfs/server/namenode/TestLeaseManager.java | 43 ++++++++++++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/a1272d5f/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index f508e84..1cc15fc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -47,6 +47,9 @@ Release 2.6.5 - UNRELEASED HDFS-9696. Garbage snapshot records linger forever. (kihwal) + HDFS-10763. Open files can leak permanently due to inconsistent + lease update (kihwal) + Release 2.6.4 - 2016-02-11 INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/hadoop/blob/a1272d5f/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 8442202..525f488 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 @@ -179,11 +179,13 @@ 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 { @@ -227,17 +229,25 @@ 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; } - // update the lease manager - INodeFile file = dir.getInode(entry.getInodeId()).asFile(); + } + + // Add a lease for each and every file under construction. + for (INodeFile file : ucFiles) { FileUnderConstructionFeature uc = file.getFileUnderConstructionFeature(); Preconditions.checkState(uc != null); // file must be under-construction - fsn.leaseManager.addLease(uc.getClientName(), entry.getFullPath()); + String path = file.getFullPathName(); + // Skip the deleted files in snapshot. This leaks UC inodes that are + // deleted from the current view. + if (path.startsWith("/")) { + fsn.leaseManager.addLease(uc.getClientName(), path); + } } } @@ -304,6 +314,7 @@ 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) { http://git-wip-us.apache.org/repos/asf/hadoop/blob/a1272d5f/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 4a70ac0..86dfd3e 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 @@ -4699,7 +4699,6 @@ public class FSNamesystem implements Namesystem, FSClusterStats, throw new IOException("Cannot finalize file " + src + " because it is not under construction"); } - leaseManager.removeLease(uc.getClientName(), src); pendingFile.recordModification(latestSnapshot); @@ -4708,6 +4707,8 @@ public class FSNamesystem implements Namesystem, FSClusterStats, // since we just remove the uc feature from pendingFile final INodeFile newFile = pendingFile.toCompleteFile(now()); + leaseManager.removeLease(uc.getClientName(), src); + waitForLoadingFSImage(); // close file and persist block allocations for this file closeFile(src, newFile); http://git-wip-us.apache.org/repos/asf/hadoop/blob/a1272d5f/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 9b454ea..31911bd 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 @@ -21,9 +21,13 @@ 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; @@ -79,4 +83,43 @@ 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(); + } + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
