diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java index e71cb0a067..b592c3ff89 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java @@ -838,6 +838,12 @@ public void cleanSubtree(ReclaimContext reclaimContext, final int snapshotId, // there is snapshot data if (sf != null) { sf.cleanDirectory(reclaimContext, this, snapshotId, priorSnapshotId); + // If the inode has empty diff list and sf is not a + // DirectorySnapshottableFeature, remove the feature to save heap. + if (sf.getDiffs().isEmpty() && + !(sf instanceof DirectorySnapshottableFeature)) { + this.removeFeature(sf); + } } else { // there is no snapshot data if (priorSnapshotId == Snapshot.NO_SNAPSHOT_ID && diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java index 7b6f1e349a..ce654b789f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java @@ -744,6 +744,9 @@ public void cleanSubtree(ReclaimContext reclaimContext, sf.cleanFile(reclaimContext, this, snapshot, priorSnapshotId, getStoragePolicyID()); updateRemovedUnderConstructionFiles(reclaimContext); + if (sf.getDiffs().isEmpty()) { + this.removeFeature(sf); + } } else { if (snapshot == CURRENT_STATE_ID) { if (priorSnapshotId == NO_SNAPSHOT_ID) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java index 9cd87f1126..163b181ed1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java @@ -45,6 +45,10 @@ public final DiffList asList() { return diffs != null ? DiffList.unmodifiableList(diffs) : DiffList.emptyList(); } + + public boolean isEmpty() { + return diffs == null || diffs.isEmpty(); + } /** Clear the list. */ public void clear() { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java index db3d93fd53..b8898eb3e8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestRenameWithSnapshots.java @@ -2144,24 +2144,11 @@ public void testRenameDirAndDeleteSnapshot_6() throws Exception { INodeDirectory dir2Node = fsdir.getINode4Write(dir2.toString()) .asDirectory(); assertTrue("the diff list of " + dir2 - + " should be empty after deleting s0", dir2Node.getDiffs().asList() - .isEmpty()); + + " should be empty after deleting s0", !dir2Node.isWithSnapshot()); assertTrue(hdfs.exists(newfoo)); INode fooRefNode = fsdir.getINode4Write(newfoo.toString()); assertTrue(fooRefNode instanceof INodeReference.DstReference); - INodeDirectory fooNode = fooRefNode.asDirectory(); - // fooNode should be still INodeDirectory (With Snapshot) since we call - // recordModification before the rename - assertTrue(fooNode.isWithSnapshot()); - assertTrue(fooNode.getDiffs().asList().isEmpty()); - INodeDirectory barNode = fooNode.getChildrenList(Snapshot.CURRENT_STATE_ID) - .get(0).asDirectory(); - // bar should also be INodeDirectory (With Snapshot), and both of its diff - // list and children list are empty - assertTrue(barNode.getDiffs().asList().isEmpty()); - assertTrue(barNode.getChildrenList(Snapshot.CURRENT_STATE_ID).isEmpty()); - restartClusterAndCheckImage(true); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java index 8bd796718c..3e318b3a30 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotDeletion.java @@ -527,7 +527,7 @@ public void testDeleteEarliestSnapshot2() throws Exception { assertEquals(snapshot1.getId(), diffList.getLast().getSnapshotId()); diffList = fsdir.getINode(metaChangeDir.toString()).asDirectory() .getDiffs(); - assertEquals(0, diffList.asList().size()); + assertEquals(null, diffList); // check 2. noChangeDir and noChangeFile are still there final INodeDirectory noChangeDirNode =