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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
