[
https://issues.apache.org/jira/browse/HDFS-16000?focusedWorklogId=655046&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-655046
]
ASF GitHub Bot logged work on HDFS-16000:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 24/Sep/21 18:39
Start Date: 24/Sep/21 18:39
Worklog Time Spent: 10m
Work Description: jojochuang commented on a change in pull request #2964:
URL: https://github.com/apache/hadoop/pull/2964#discussion_r715810510
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
##########
@@ -1091,6 +1091,24 @@ static void unprotectedUpdateCount(INodesInPath
inodesInPath,
}
}
+ /**
+ * check that all parent directories have quotas set.
+ */
+ static boolean verifyIsQuota(INodesInPath iip, int pos) {
Review comment:
looks like pos is always assigned iip.length() -1. So why not simplify
the logic and remove the pos?
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
##########
@@ -417,12 +434,32 @@ static RenameResult unprotectedRenameTo(FSDirectory fsd,
// Ensure dst has quota to accommodate rename
verifyFsLimitsForRename(fsd, srcIIP, dstIIP);
- verifyQuotaForRename(fsd, srcIIP, dstIIP);
+ QuotaCounts srcPolicyCounts = new QuotaCounts.Builder(true).build();
+ QuotaCounts dstPolicyCounts = new QuotaCounts.Builder(true).build();
+ boolean srcIIPIsQuota = FSDirectory.verifyIsQuota(
+ srcIIP, srcIIP.length() - 1);
+ boolean dstIIPIsQuota = FSDirectory.verifyIsQuota(
+ dstIIP, dstIIP.length() - 1);
+ if (srcIIPIsQuota || dstIIPIsQuota) {
+ if (dstParent.getStoragePolicyID() ==
+ srcIIP.getLastINode().getStoragePolicyID()) {
+ srcPolicyCounts = srcIIP.getLastINode().computeQuotaUsage(bsps);
Review comment:
line 446 and 449 are the same. You may move them to line 444.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
##########
@@ -1091,6 +1091,24 @@ static void unprotectedUpdateCount(INodesInPath
inodesInPath,
}
}
+ /**
+ * check that all parent directories have quotas set.
+ */
+ static boolean verifyIsQuota(INodesInPath iip, int pos) {
+ for (int i = (Math.min(pos, iip.length()) - 1); i > 0; i--) {
+ INode currNode = iip.getINode(i);
+ if (currNode == null) {
+ continue;
+ }
+ if (currNode.isDirectory()) {
+ if (currNode.isQuotaSet()) {
Review comment:
if the bottom most directory has quota set, does it imply all the
ancestor directories have quota set?
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirRenameOp.java
##########
@@ -417,12 +434,32 @@ static RenameResult unprotectedRenameTo(FSDirectory fsd,
// Ensure dst has quota to accommodate rename
verifyFsLimitsForRename(fsd, srcIIP, dstIIP);
- verifyQuotaForRename(fsd, srcIIP, dstIIP);
Review comment:
this entire method unprotectedRenameTo() needs a refactor. It's way too
long too complex to understand.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
##########
@@ -1091,6 +1091,24 @@ static void unprotectedUpdateCount(INodesInPath
inodesInPath,
}
}
+ /**
+ * check that all parent directories have quotas set.
+ */
+ static boolean verifyIsQuota(INodesInPath iip, int pos) {
Review comment:
the method name verifyIsQuota() isn't intuitive.
--
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-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 655046)
Time Spent: 40m (was: 0.5h)
> HDFS : Rename performance optimization
> --------------------------------------
>
> Key: HDFS-16000
> URL: https://issues.apache.org/jira/browse/HDFS-16000
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: hdfs, namenode
> Affects Versions: 3.1.4, 3.3.1
> Reporter: Xiangyi Zhu
> Assignee: Xiangyi Zhu
> Priority: Major
> Labels: pull-request-available
> Attachments: 20210428-143238.svg, 20210428-171635-lambda.svg,
> HDFS-16000.patch
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> It takes a long time to move a large directory with rename. For example, it
> takes about 40 seconds to move a 1000W directory. When a large amount of data
> is deleted to the trash, the move large directory will occur when the recycle
> bin makes checkpoint. In addition, the user may also actively trigger the
> move large directory operation, which will cause the NameNode to lock too
> long and be killed by Zkfc. Through the flame graph, it is found that the
> main time consuming is to create the EnumCounters object.
> h3. I think the following two points can optimize the efficiency of rename
> execution
> h3. QuotaCount calculation time-consuming optimization:
> * Create a QuotaCounts object in the calculation directory quotaCount, and
> pass the quotaCount to the next calculation function through a parameter each
> time, so as to avoid creating an EnumCounters object for each calculation.
> * In addition, through the flame graph, it is found that using lambda to
> modify QuotaCounts takes longer than the ordinary method, so the ordinary
> method is used to modify the QuotaCounts count.
> h3. Rename logic optimization:
> * Regardless of whether the rename operation is the source directory and the
> target directory, the quota count must be calculated three times. The first
> time, check whether the moved directory exceeds the target directory quota,
> the second time, calculate the mobile directory quota to update the source
> directory quota, and the third time, calculate the mobile directory
> configuration update to the target directory.
> * I think some of the above three quota quota calculations are unnecessary.
> For example, if all parent directories of the source directory and target
> directory are not configured with quota, there is no need to calculate
> quotaCount. Even if both the source directory and the target directory use
> quota, there is no need to calculate the quota three times. The calculation
> logic for the first and third times is the same, and it only needs to be
> calculated once.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
