Hexiaoqiao commented on code in PR #2964:
URL: https://github.com/apache/hadoop/pull/2964#discussion_r1328663679
##########
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:
Sorry I didn't get it. I means that we do nothing when `srcStoragePolicyID
== dstStoragePolicyID && srcParentNode == dstParentNode && tx.withCount ==
null` because there is no `else` logic, So my question is what happen here, do
we need to do something?
IMO, We don't need to compute quota usage once when `srcParentNode ==
dstParentNode`, right? However L524 seems to still compute it.
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirMkdirOp.java:
##########
@@ -222,7 +222,8 @@ private static INodesInPath unprotectedMkdir(FSDirectory
fsd, long inodeId,
timestamp);
INodesInPath iip =
- fsd.addLastINode(parent, dir, permission.getPermission(), true);
+ fsd.addLastINode(parent, dir,
+ permission.getPermission(), true, null, true);
Review Comment:
Keep the code style as usual, such as here, we should leave 4 space blanks
compare to L225. Some other changes have the same format issues.
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java:
##########
@@ -1468,6 +1475,30 @@ static Collection<String>
normalizePaths(Collection<String> paths,
return normalized;
}
+ /**
+ * Get the first Node that sets Quota.
+ */
+ static INode getFirstSetQuotaParentNode(INodesInPath iip) {
+ for (int i = iip.length() - 1; i > 0; i--) {
+ INode currNode = iip.getINode(i);
+ if (currNode == null) {
Review Comment:
If it is not expected, we need to throw exception, or I think no need to
check it.
--
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]
