improve

Author: huangzhaobo99

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java

@@ -85,9 +85,8 @@ private static Triple<Boolean, Optional<QuotaCounts>, Optional<QuotaCounts>> ver
     // Verify srcPath and dstPath without valid 'DirectoryWithQuotaFeature'
     // Note: In overwrite scenarios, quota calculation is still required because:
     // Overwrite operations delete existing content (affecting rootDir's quota)
-    if (FSDirectory.verifyPathWithoutValidQuotaFeature(src, src.length() - 1)
-        && FSDirectory.verifyPathWithoutValidQuotaFeature(dst, dst.length() - 1) 
-        && !overwrite) {
+    if (!overwrite && FSDirectory.verifyPathWithoutValidQuotaFeature(src)
+        && FSDirectory.verifyPathWithoutValidQuotaFeature(dst)) {
       return Triple.of(false, srcDelta, dstDelta);
     }
 
@@ -768,9 +767,9 @@ long removeSrc() throws IOException {
         throw new IOException(error);
       } else {
         // update the quota count if necessary
-        Optional<QuotaCounts> countOp = sameStoragePolicy ?
-            srcSubTreeCount : Optional.empty();
         if (updateQuota) {
+          Optional<QuotaCounts> countOp = sameStoragePolicy ?
+              srcSubTreeCount : Optional.empty();
           fsd.updateCountForDelete(srcChild, srcIIP, countOp);
         }
         srcIIP = INodesInPath.replace(srcIIP, srcIIP.length() - 1, null);
@@ -787,9 +786,9 @@ boolean removeSrc4OldRename() {
         return false;
       } else {
         // update the quota count if necessary
-        Optional<QuotaCounts> countOp = sameStoragePolicy ?
-            srcSubTreeCount : Optional.empty();
         if (updateQuota) {
+          Optional<QuotaCounts> countOp = sameStoragePolicy ?
+              srcSubTreeCount : Optional.empty();
           fsd.updateCountForDelete(srcChild, srcIIP, countOp);
         }
         srcIIP = INodesInPath.replace(srcIIP, srcIIP.length() - 1, null);

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java

@@ -1241,19 +1241,18 @@ static void verifyQuota(INodesInPath iip, int pos, QuotaCounts deltas,
       }
     }
   }
-  
+
   /**
    * Verifies that the path from the specified position to the root
    * (excluding the root itself) does not contain any valid quota features.
    *
    * @param iip the INodesInPath containing all the ancestral INodes.
-   * @param pos Starting position in the path (0-based index).
    * @return true if no valid quota features are found along the path (root excluded),
    * false if any directory in the path has an active quota feature.
    */
-  static boolean verifyPathWithoutValidQuotaFeature(INodesInPath iip, int pos) {
-    // Does not include the root path
-    for (int i = Math.min(pos, iip.length()) - 1; i >= 1; i--) {
+  static boolean verifyPathWithoutValidQuotaFeature(INodesInPath iip) {
+    // Exclude the root inode
+    for (int i = iip.length() - 2; i >= 1; i--) {
       final DirectoryWithQuotaFeature q =
           iip.getINode(i).asDirectory().getDirectoryWithQuotaFeature();
       if (q != null) {
@@ -1367,8 +1366,8 @@ private void copyINodeDefaultAcl(INode child, FsPermission modes) {
    * @return an INodesInPath instance containing the new INode
    */
   @VisibleForTesting
-  public INodesInPath addLastINode(INodesInPath existing, INode inode,
-      FsPermission modes, boolean checkQuota, Optional<QuotaCounts> quotaCount, boolean updateQuota)
+  public INodesInPath addLastINode(INodesInPath existing, INode inode, FsPermission modes,
+      boolean checkQuota, Optional<QuotaCounts> quotaCount, boolean updateQuota)
       throws QuotaExceededException {
     assert existing.getLastINode() != null &&
         existing.getLastINode().isDirectory();
@@ -1400,9 +1399,8 @@ public INodesInPath addLastINode(INodesInPath existing, INode inode,
     // always verify inode name
     verifyINodeName(inode.getLocalNameBytes());
 
-    QuotaCounts counts;
     if (updateQuota) {
-      counts = quotaCount.orElseGet(() -> inode.
+      QuotaCounts counts = quotaCount.orElseGet(() -> inode.
           computeQuotaUsage(getBlockStoragePolicySuite(),
           parent.getStoragePolicyID(), false,
           Snapshot.CURRENT_STATE_ID));
@@ -1413,7 +1411,7 @@ public INodesInPath addLastINode(INodesInPath existing, INode inode,
     final boolean added = parent.addChild(inode, true,
         existing.getLatestSnapshotId());
     if (!added && updateQuota) {
-      counts = quotaCount.orElseGet(() -> inode.
+      QuotaCounts counts = quotaCount.orElseGet(() -> inode.
           computeQuotaUsage(getBlockStoragePolicySuite(),
           parent.getStoragePolicyID(), false,
           Snapshot.CURRENT_STATE_ID));

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCorrectnessOfQuotaAfterRenameOp.java

@@ -172,7 +172,7 @@ public void testQuotaUsageWhenRenameWithDifferStoragePolicy() throws Exception {
   public void testRenameWithoutValidQuotaFeature() throws Exception {
     final int fileLen = 1024;
     final short replication = 3;
-    final Path root = new Path("/testRoot");
+    final Path root = new Path("/testRename");
     assertTrue(dfs.mkdirs(root));
 
     Path testParentDir1 = new Path(root, "testDir1");
@@ -189,36 +189,38 @@ public void testRenameWithoutValidQuotaFeature() throws Exception {
     }
 
     // 1. Test rename1
-    QuotaUsage quotaUsage1 = dfs.getQuotaUsage(new Path("/"));
-    final Path dstDir1 = new Path(testParentDir2, "dst-dir");
-    ContentSummary cs1 = dfs.getContentSummary(testParentDir1);
-
-    // srcDir=/testRoot/testDir1/src-dir
-    // dstDir=/testRoot/testDir2/dst-dir dstDir1 not exist
-    boolean rename = dfs.rename(srcDir, dstDir1);
-    assertTrue(rename);
-    ContentSummary cs2 = dfs.getContentSummary(testParentDir2);
-    assertEquals(cs1, cs2);
-
-    QuotaUsage quotaUsage2 = dfs.getQuotaUsage(new Path("/"));
-    assertEquals(quotaUsage1.getFileAndDirectoryCount(), quotaUsage2.getFileAndDirectoryCount());
+    ContentSummary rootContentSummary1 = dfs.getContentSummary(new Path("/"));
+    QuotaUsage rootQuotaUsage1 = dfs.getQuotaUsage(new Path("/"));
+    ContentSummary contentSummary1 = dfs.getContentSummary(testParentDir1);
+    // srcDir=/testRename/testDir1/src-dir
+    // dstDir=/testRename/testDir2/dst-dir dstDir1 not exist
+    final Path dstDir2 = new Path(testParentDir2, "dst-dir");
+    assertTrue(dfs.rename(srcDir, dstDir2));
+    ContentSummary contentSummary2 = dfs.getContentSummary(testParentDir2);
+    assertEquals(contentSummary1, contentSummary2);
+    QuotaUsage rootQuotaUsage2 = dfs.getQuotaUsage(new Path("/"));
+    assertEquals(rootQuotaUsage1.getFileAndDirectoryCount(),
+        rootQuotaUsage2.getFileAndDirectoryCount());
+    // The return values of the getContentSummary() and getQuotaUsage() should be consistent
+    assertEquals(rootContentSummary1.getFileAndDirectoryCount(),
+        rootQuotaUsage2.getFileAndDirectoryCount());
 
     // 2. Test rename2
-    final Path dstDir2 = new Path(testParentDir3, "dst-dir");
-    assertTrue(dfs.mkdirs(dstDir2));
-    long originDstDir2Usage = dfs.getQuotaUsage(dstDir2).getFileAndDirectoryCount();
-    // The usage for covering the root dir should not include dstDir2 usage
+    final Path dstDir3 = new Path(testParentDir3, "dst-dir");
+    assertTrue(dfs.mkdirs(dstDir3));
+    long originDstDir2Usage = dfs.getQuotaUsage(dstDir3).getFileAndDirectoryCount();
+    // Exclude dstDir2 usage
     long rootINodeCount1 =
         dfs.getQuotaUsage(new Path("/")).getFileAndDirectoryCount() - originDstDir2Usage;
-    ContentSummary cs3 = dfs.getContentSummary(testParentDir2);
+    ContentSummary contentSummary3 = dfs.getContentSummary(testParentDir2);
 
     // Src and dst must be same (all file or all dir)
-    // dstDir1=/testRoot/testDir3/dst-dir
-    // dstDir2=/testRoot/testDir3/dst-dir
-    dfs.rename(dstDir1, dstDir2, Options.Rename.OVERWRITE);
+    // dstDir2=/testRename/testDir3/dst-dir
+    // dstDir3=/testRename/testDir3/dst-dir
+    dfs.rename(dstDir2, dstDir3, Options.Rename.OVERWRITE);
     long rootINodeCount2 = dfs.getQuotaUsage(new Path("/")).getFileAndDirectoryCount();
     assertEquals(rootINodeCount1, rootINodeCount2);
-    ContentSummary cs4 = dfs.getContentSummary(testParentDir3);
-    assertEquals(cs3, cs4);
+    ContentSummary contentSummary4 = dfs.getContentSummary(testParentDir3);
+    assertEquals(contentSummary3, contentSummary4);
   }
 }