-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HADOOP-19141. Vector IO: Update default values consistently #6702
Conversation
Could you review this, @steveloughran ? I filed this as a subtask of HADOOP-18855. If it's inappropriate, I'll convert this to an independent JIRA. Please let me know. |
looks good to me -suspect it was a latent typo or something. Note that org.apache.hadoop.fs.azure.integration.Sizes defines some of the sizes; there's something else in bits of hadoop-aws tests. really we should have something in hadoop-common. side issue: have you tested this yet? as right now I know the delegation token mr job fails until a pending pr is merged |
Thank you for review. Yes, I've been testing |
💔 -1 overall
This message was automatically generated. |
CI seems to be happy except |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
+1
merging to trunk and branch 3.4/3.3 |
Contributed by Dongjoon Hyun
Thank you, @steveloughran ! |
Not sure how this happened but thanks for fixing this. |
Description of PR
This PR updates two Vector IO default values.
If a user set a configuration like the documented
4K
or1M
, Apache Hadoop will use 4096 and 1048576 which are different from the AS-IS defined value ofConstants.java
. Thus, this PR fixesConstants.java
with the values, 4096 and 1048576.hadoop/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
Lines 779 to 782 in 87fb977
How was this patch tested?
Apache Hadoop uses 1M as
1048576 = 1024 x 1024
historically like the following.hadoop/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
Line 521 in 87fb977
So, the default value of Vector IO is inconsistent from not only the other values but also from its comment,
//1M
, and its documentation,1M
.hadoop/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
Line 1431 in 87fb977
hadoop/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/performance.md
Line 79 in 87fb977
The AS-IS default values of Vector IO are also inconsistent from the values of Vector IO test cases.
hadoop/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractVectoredRead.java
Lines 144 to 147 in 87fb977
The inconsistency happens for
4K
in the same way.hadoop/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
Line 1426 in 87fb977
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?