From ea5bb483269b51a349c358b71f84904c76693a66 Mon Sep 17 00:00:00 2001
From: Xiaoyu Yao <xyao@apache.org>
Date: Thu, 5 Nov 2015 01:30:13 -0800
Subject: [PATCH] HDFS-9360. Storage type usage isn't updated properly after
 file deletion. Contributed by Ming Ma.

---
 hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt   |  3 +
 .../hdfs/server/namenode/INodeFile.java       |  4 +-
 .../org/apache/hadoop/hdfs/TestQuota.java     | 55 ++++++++++++++++++-
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
index ef1152e864..26079cb789 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -2236,6 +2236,9 @@ Release 2.8.0 - UNRELEASED
     HDFS-9357. NN UI renders icons of decommissioned DN incorrectly.
     (Surendra Singh Lilhore via wheat9)
 
+    HDFS-9360. Storage type usage isn't updated properly after file deletion.
+    (Ming Ma via xyao)
+
 Release 2.7.3 - UNRELEASED
 
   INCOMPATIBLE CHANGES
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 242c456bdb..962a2825f4 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
@@ -625,7 +625,9 @@ public final QuotaCounts computeQuotaUsage(BlockStoragePolicySuite bsps,
       byte blockStoragePolicyId, boolean useCache, int lastSnapshotId) {
     final QuotaCounts counts = new QuotaCounts.Builder().nameSpace(1).build();
 
-    final BlockStoragePolicy bsp = bsps.getPolicy(blockStoragePolicyId);
+    final BlockStoragePolicy bsp = (blockStoragePolicyId ==
+        BLOCK_STORAGE_POLICY_ID_UNSPECIFIED) ? null :
+        bsps.getPolicy(blockStoragePolicyId);
     FileWithSnapshotFeature sf = getFileWithSnapshotFeature();
     if (sf == null) {
       counts.add(storagespaceConsumed(bsp));
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java
index 00ff07f15d..16ab77d35a 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java
@@ -38,6 +38,7 @@
 import org.apache.hadoop.hdfs.protocol.DSQuotaExceededException;
 import org.apache.hadoop.hdfs.protocol.HdfsConstants;
 import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException;
+import org.apache.hadoop.hdfs.protocol.QuotaByStorageTypeExceededException;
 import org.apache.hadoop.hdfs.protocol.QuotaExceededException;
 import org.apache.hadoop.hdfs.server.namenode.FSImageTestUtil;
 import org.apache.hadoop.hdfs.tools.DFSAdmin;
@@ -574,7 +575,6 @@ public void testSpaceCommands() throws Exception {
       c = dfs.getContentSummary(quotaDir20);
       assertEquals(c.getSpaceQuota(), 6 * fileSpace);
 
-
       // Create /nqdir0/qdir1/qdir21 and set its space quota to 2 * fileSpace
       final Path quotaDir21 = new Path("/nqdir0/qdir1/qdir21");
       assertTrue(dfs.mkdirs(quotaDir21));
@@ -786,6 +786,59 @@ public void testSpaceCommands() throws Exception {
     }
   }
 
+  /**
+   * Test quota by storage type.
+   */
+  @Test
+  public void testQuotaByStorageType() throws Exception {
+    final Configuration conf = new HdfsConfiguration();
+    // set a smaller block size so that we can test with smaller
+    // diskspace quotas
+    conf.set(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, "512");
+    // Make it relinquish locks. When run serially, the result should
+    // be identical.
+    conf.setInt(DFSConfigKeys.DFS_CONTENT_SUMMARY_LIMIT_KEY, 2);
+    final MiniDFSCluster cluster =
+        new MiniDFSCluster.Builder(conf).numDataNodes(2).build();
+    final FileSystem fs = cluster.getFileSystem();
+    assertTrue("Not a HDFS: " + fs.getUri(),
+        fs instanceof DistributedFileSystem);
+    final DistributedFileSystem dfs = (DistributedFileSystem) fs;
+
+    try {
+      int fileLen = 1024;
+      short replication = 3;
+      int fileSpace = fileLen * replication;
+
+      final Path quotaDir20 = new Path("/nqdir0/qdir1/qdir20");
+      assertTrue(dfs.mkdirs(quotaDir20));
+      dfs.setQuota(quotaDir20, HdfsConstants.QUOTA_DONT_SET, 6 * fileSpace);
+
+      // Verify DirectoryWithQuotaFeature's storage type usage
+      // is updated properly after deletion.
+      // File creation followed by deletion shouldn't change storage type
+      // usage regardless whether storage policy is set.
+      Path file = new Path(quotaDir20, "fileDir/file1");
+      DFSTestUtil.createFile(dfs, file, fileLen * 3, replication, 0);
+      dfs.delete(file, false);
+      dfs.setStoragePolicy(quotaDir20, HdfsConstants.HOT_STORAGE_POLICY_NAME);
+      dfs.setQuotaByStorageType(quotaDir20, StorageType.DEFAULT,
+          2 * fileSpace);
+      boolean hasException = false;
+      try {
+        DFSTestUtil.createFile(dfs, file, fileLen * 3, replication, 0);
+      } catch (QuotaByStorageTypeExceededException e) {
+        hasException = true;
+      }
+      assertTrue(hasException);
+      dfs.delete(file, false);
+      dfs.setQuotaByStorageType(quotaDir20, StorageType.DEFAULT,
+          6 * fileSpace);
+    } finally {
+      cluster.shutdown();
+    }
+  }
+
   private static void checkContentSummary(final ContentSummary expected,
       final ContentSummary computed) {
     assertEquals(expected.toString(), computed.toString());