This is an automated email from the ASF dual-hosted git repository.
weichiu pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/hadoop.git
commit a3493fe3288a3961cb85b8d50973d1563ffd3e35
Author: Wei-Chiu Chuang <weic...@apache.org>
AuthorDate: Tue Oct 1 08:46:46 2019 -0700
HDFS-14492. Snapshot memory leak. Contributed by Wei-Chiu Chuang. (#1370)
* HDFS-14492. Snapshot memory leak. Contributed by Wei-Chiu Chuang.
Change-Id: I9e5e450c07ad70aa1905973896c4f627042dbd37
* Fix checkstyle
Change-Id: I16d4bd4f03a971e1ed36cf57d89dc42357ef8fbf
(cherry picked from commit 6ef6594c7ee09b561e42c16ce4e91c0479908ad8)
(cherry picked from commit 570ffa1cd67990ef0e49c149abc04a92ed3670ac)
---
.../hadoop/hdfs/server/namenode/INodeDirectory.java | 6 ++++++
.../org/apache/hadoop/hdfs/server/namenode/INodeFile.java | 3 +++
.../server/namenode/snapshot/AbstractINodeDiffList.java | 4 ++++
.../server/namenode/snapshot/TestRenameWithSnapshots.java | 15 +--------------
.../server/namenode/snapshot/TestSnapshotDeletion.java | 2 +-
5 files changed, 15 insertions(+), 15 deletions(-)
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 e71cb0a..b592c3f 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 class INodeDirectory extends
INodeWithAdditionalFields
// 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 7b6f1e3..ce654b7 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 class INodeFile extends INodeWithAdditionalFields
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 4a00d20..25be143 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 @@ abstract class AbstractINodeDiffList<N extends INode,
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 d36e3b6..1f624d0 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
@@ -2141,24 +2141,11 @@ public class TestRenameWithSnapshots {
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 8bd7967..3e318b3 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 class TestSnapshotDeletion {
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 =
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org
