Revert "HDFS-14492. Snapshot memory leak. Contributed by Wei-Chiu Chuang. (#1370)"
This reverts commit 6ef6594c7ee09b561e42c16ce4e91c0479908ad8.
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 b592c3f..e71cb0a 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,12 +838,6 @@
// 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 ce654b7..7b6f1e3 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,9 +744,6 @@
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 163b181..9cd87f1 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,10 +45,6 @@
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 b8898eb..db3d93f 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,11 +2144,24 @@
INodeDirectory dir2Node = fsdir.getINode4Write(dir2.toString())
.asDirectory();
assertTrue("the diff list of " + dir2
- + " should be empty after deleting s0", !dir2Node.isWithSnapshot());
+ + " should be empty after deleting s0", dir2Node.getDiffs().asList()
+ .isEmpty());
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 3e318b3..8bd7967 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 @@
assertEquals(snapshot1.getId(), diffList.getLast().getSnapshotId());
diffList = fsdir.getINode(metaChangeDir.toString()).asDirectory()
.getDiffs();
- assertEquals(null, diffList);
+ assertEquals(0, diffList.asList().size());
// check 2. noChangeDir and noChangeFile are still there
final INodeDirectory noChangeDirNode =
