hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From shashik...@apache.org
Subject [hadoop] 01/01: Revert "HDFS-14492. Snapshot memory leak. Contributed by Wei-Chiu Chuang. (#1370)"
Date Tue, 01 Oct 2019 15:48:10 GMT
This is an automated email from the ASF dual-hosted git repository.

shashikant pushed a commit to branch revert-1370-HDFS-14492
in repository https://gitbox.apache.org/repos/asf/hadoop.git

commit f0eafaac6bf50321d04e4437c25d2a601d3b36a5
Author: bshashikant <sbanerjee@hortonworks.com>
AuthorDate: Tue Oct 1 21:17:59 2019 +0530

    Revert "HDFS-14492. Snapshot memory leak. Contributed by Wei-Chiu Chuang. (#1370)"
    
    This reverts commit 6ef6594c7ee09b561e42c16ce4e91c0479908ad8.
---
 .../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 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 @@ 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 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 @@ 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 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 @@ 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 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 @@ public class TestRenameWithSnapshots {
     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 @@ public class TestSnapshotDeletion {
     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 = 


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org


Mime
View raw message