Hexiaoqiao commented on code in PR #2964:
URL: https://github.com/apache/hadoop/pull/2964#discussion_r1328244599
##########
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:
Will it meet null here? if it is not expected, we should throw exception IMO.
##########
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()
Review Comment:
IIUC, this result will be used for the next verify and storage used
addition/subtraction for src and dst inode, right? But I am confused if it will
meet some issues here, given directory /a/b (whose storage policy is HDD), /c/d
(whose storage policy is SSD), when rename from /a/b/r1 (let's assume 1GB used)
to /c/d/r2, then total HDD storage used will decrease 1GB, and SSD storage used
increase 1GB, this will be different with fact?
##########
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:
what happen if `srcParentNode == dstParentNode && tx.withCount == null`?
##########
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) {
Review Comment:
what's `tx.withCount` here?
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirMkdirOp.java:
##########
@@ -221,8 +221,8 @@ private static INodesInPath unprotectedMkdir(FSDirectory
fsd, long inodeId,
final INodeDirectory dir = new INodeDirectory(inodeId, name, permission,
timestamp);
- INodesInPath iip =
- fsd.addLastINode(parent, dir, permission.getPermission(), true);
+ INodesInPath iip = fsd.addLastINode(parent, dir,
+ permission.getPermission(), true, null, true);
Review Comment:
Suggest to keep code format as usual, please also check other changed line.
Not blocker issue. 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]
