Re: [PR] HDFS-16000. Rename performance optimization [hadoop]

["lfxy (via GitHub)" <gi...@apache.org>]

lfxy commented on code in PR #2964:
URL: https://github.com/apache/hadoop/pull/2964#discussion_r1436951488


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java:
##########
@@ -470,17 +475,53 @@ static RenameResult unprotectedRenameTo(FSDirectory fsd,
       }
     } finally {
       if (undoRemoveSrc) {
-        tx.restoreSource();
+        tx.restoreSource(srcStoragePolicyCounts);
       }
       if (undoRemoveDst) { // Rename failed - restore dst
-        tx.restoreDst(bsps);
+        tx.restoreDst(bsps, dstStoragePolicyCounts);
       }
     }
     NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedRenameTo: " +
         "failed to rename " + src + " to " + dst);
     throw new IOException("rename from " + src + " to " + dst + " failed.");
   }
 
+  /*
+   * Calculate QuotaCounts based on parent directory and storage policy
+   * 1. If the storage policy of src and dst are different,
+   *  calculate the QuotaCounts of src and dst respectively.
+   * 2. If all parent nodes of src and dst are not set with Quota,
+   *  there is no need to calculate QuotaCount.
+   * 3. if parent nodes of src and dst have Quota configured,
+   *  the QuotaCount is calculated once using the storage policy of src.
+   * */
+  private static void computeQuotaCounts(
+          QuotaCounts srcStoragePolicyCounts,
+          QuotaCounts dstStoragePolicyCounts,
+          INodesInPath srcIIP,
+          INodesInPath dstIIP,
+          BlockStoragePolicySuite bsps,
+          RenameOperation tx) {
+    INode dstParent = dstIIP.getINode(-2);
+    INode srcParentNode = FSDirectory.
+            getFirstSetQuotaParentNode(srcIIP);
+    INode srcInode = srcIIP.getLastINode();
+    INode dstParentNode = FSDirectory.
+            getFirstSetQuotaParentNode(dstIIP);
+    byte srcStoragePolicyID = FSDirectory.getStoragePolicyId(srcInode);
+    byte dstStoragePolicyID = FSDirectory.getStoragePolicyId(dstParent);
+    if (srcStoragePolicyID != dstStoragePolicyID) {
+      srcStoragePolicyCounts.add(srcIIP.getLastINode().
+              computeQuotaUsage(bsps));
+      dstStoragePolicyCounts.add(srcIIP.getLastINode()
+              .computeQuotaUsage(bsps, dstParent.getStoragePolicyID(), false,
+                      Snapshot.CURRENT_STATE_ID));
+    } else if (srcParentNode != dstParentNode || tx.withCount != null) {
+      srcStoragePolicyCounts.add(srcIIP.getLastINode().computeQuotaUsage(bsps));
+      dstStoragePolicyCounts.add(srcStoragePolicyCounts);
+    }

Review Comment:
   @Hexiaoqiao @zhuxiangyi Our production cluster encountered performance issues related to rename and needed to be optimized.
   In (srcStoragePolicyID == dstStoragePolicyID) condition, do we need to compute only when both conditions srcParentNode != dstParentNode and tx.isSrcInSnapshot == false are met? If it is right, can we use the below logics:
   if (srcStoragePolicyID != dstStoragePolicyID) {
   compute;
   } else if (srcParentNode != dstParentNode && !tx.isSrcInSnapshot) {
   compute;
   }
   Looking forward to your reply, thanks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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