From fbdfb36fdfc9dd52d16ec517069a877a76a02095 Mon Sep 17 00:00:00 2001 From: Xiaoyu Yao Date: Thu, 5 Sep 2019 15:47:16 -0700 Subject: [PATCH] Revert "HDFS-14633. The StorageType quota and consume in QuotaFeature is not handled for rename. Contributed by Jinglun." This reverts commit 29e6a97faa81f6b143851b0a038706bccc223130. --- .../hdfs/server/namenode/FSDirRenameOp.java | 7 +-- .../hdfs/server/namenode/FSDirectory.java | 5 +- .../org/apache/hadoop/hdfs/TestQuota.java | 62 ------------------- 3 files changed, 2 insertions(+), 72 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java index 6ca6568ea2c..68cf3e7698a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java @@ -76,12 +76,7 @@ class FSDirRenameOp { while(src.getINode(i) == dst.getINode(i)) { i++; } // src[i - 1] is the last common ancestor. BlockStoragePolicySuite bsps = fsd.getBlockStoragePolicySuite(); - // Assume dstParent existence check done by callers. - INode dstParent = dst.getINode(-2); - // Use the destination parent's storage policy for quota delta verify. - final QuotaCounts delta = src.getLastINode() - .computeQuotaUsage(bsps, dstParent.getStoragePolicyID(), false, - Snapshot.CURRENT_STATE_ID); + final QuotaCounts delta = src.getLastINode().computeQuotaUsage(bsps); // Reduce the required quota by dst that is being removed final INode dstINode = dst.getLastINode(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java index 9ea1b50775c..65fdff59f5d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java @@ -17,7 +17,6 @@ */ package org.apache.hadoop.hdfs.server.namenode; -import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; import org.apache.hadoop.util.StringUtils; import com.google.common.annotations.VisibleForTesting; @@ -1292,9 +1291,7 @@ public class FSDirectory implements Closeable { // always verify inode name verifyINodeName(inode.getLocalNameBytes()); - final QuotaCounts counts = inode - .computeQuotaUsage(getBlockStoragePolicySuite(), - parent.getStoragePolicyID(), false, Snapshot.CURRENT_STATE_ID); + final QuotaCounts counts = inode.computeQuotaUsage(getBlockStoragePolicySuite()); updateCount(existing, pos, counts, checkQuota); boolean isRename = (inode.getParent() != null); 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 b63813b6ffe..1c4855fc701 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 @@ -1565,57 +1565,6 @@ public class TestQuota { assertEquals(0, cluster.getNamesystem().getNumFilesUnderConstruction()); } - @Test - public void testClrQuotaOnRoot() throws Exception { - long orignalQuota = dfs.getQuotaUsage(new Path("/")).getQuota(); - DFSAdmin admin = new DFSAdmin(conf); - String[] args; - args = new String[] {"-setQuota", "3K", "/"}; - runCommand(admin, args, false); - assertEquals(3 * 1024, dfs.getQuotaUsage(new Path("/")).getQuota()); - args = new String[] {"-clrQuota", "/"}; - runCommand(admin, args, false); - assertEquals(orignalQuota, dfs.getQuotaUsage(new Path("/")).getQuota()); - } - - @Test - public void testRename() throws Exception { - int fileLen = 1024; - short replication = 3; - - final Path parent = new Path(PathUtils.getTestDir(getClass()).getPath(), - GenericTestUtils.getMethodName()); - assertTrue(dfs.mkdirs(parent)); - - final Path srcDir = new Path(parent, "src-dir"); - Path file = new Path(srcDir, "file1"); - DFSTestUtil.createFile(dfs, file, fileLen, replication, 0); - dfs.setStoragePolicy(srcDir, HdfsConstants.HOT_STORAGE_POLICY_NAME); - - final Path dstDir = new Path(parent, "dst-dir"); - assertTrue(dfs.mkdirs(dstDir)); - dfs.setStoragePolicy(dstDir, HdfsConstants.ALLSSD_STORAGE_POLICY_NAME); - - dfs.setQuota(srcDir, 100000, 100000); - dfs.setQuota(dstDir, 100000, 100000); - - Path dstFile = new Path(dstDir, "file1"); - // Test quota check of rename. Expect a QuotaExceedException. - dfs.setQuotaByStorageType(dstDir, StorageType.SSD, 10); - try { - dfs.rename(file, dstFile); - fail("Expect QuotaExceedException."); - } catch (QuotaExceededException qe) { - } - - // Set enough quota, expect a successful rename. - dfs.setQuotaByStorageType(dstDir, StorageType.SSD, fileLen * replication); - dfs.rename(file, dstFile); - // Verify the storage type usage is properly updated on source and dst. - checkQuotaAndCount(dfs, srcDir); - checkQuotaAndCount(dfs, dstDir); - } - @Test public void testSpaceQuotaExceptionOnAppend() throws Exception { GenericTestUtils.setLogLevel(DFSOutputStream.LOG, Level.TRACE); @@ -1686,15 +1635,4 @@ public class TestQuota { } scanner.close(); } - - // quota and count should match. - private void checkQuotaAndCount(DistributedFileSystem fs, Path path) - throws IOException { - QuotaUsage qu = fs.getQuotaUsage(path); - ContentSummary cs = fs.getContentSummary(path); - for (StorageType st : StorageType.values()) { - // it will fail here, because the quota and consume is not handled right. - assertEquals(qu.getTypeConsumed(st), cs.getTypeConsumed(st)); - } - } }