-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-18107 Adding scale test for vectored reads for large file #4273
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-18107 Adding scale test for vectored reads for large file #4273
Conversation
part of HADOOP-18103.
steveloughran
left a comment
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.
made some minor suggestions
...mon-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Test | ||
| public void test_045_VectoredIOHugeFile() throws Throwable { | ||
| public void test_045_VectoredIoHugeFile() throws Throwable { |
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.
capital IO is OK here; your choice
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.
Yeah I thought that was the problem but don't know why checkstyle is crying
./hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractSTestS3AHugeFiles.java:459: public void test_045_vectoredIoHugeFile() throws Throwable {:15: Name 'test_045_vectoredIoHugeFile' must match pattern '^[a-z][a-zA-Z0-9]*$'. [MethodName]
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.
the filename with the _ breaks checkstyle rules. maybe we should add a new rule which permits
test_[0-9]+_[a-zA-Z0-9]*$
in test code only?
...mon-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
Outdated
Show resolved
Hide resolved
...mon-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
Outdated
Show resolved
Hide resolved
|
@steveloughran I think it is ready to be merged. Just the java docs. |
steveloughran
left a comment
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, made some minor comments
...tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractSTestS3AHugeFiles.java
Outdated
Show resolved
Hide resolved
...mon-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
Outdated
Show resolved
Hide resolved
...mon-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
Outdated
Show resolved
Hide resolved
...mon-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
Outdated
Show resolved
Hide resolved
|
+1 pending that move into a constant, just for clarity. thanks (btw, suggest another rebase to get rid of those javadoc issues) |
Will do the rebase of feature branch in one go after the smaller patches are merged. |
steveloughran
left a comment
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.
+1 pending the javadoc correction
| public static final int DEFAULT_IO_CHUNK_MODULUS_SIZE = 128; | ||
|
|
||
| /** | ||
| * Timeout in seconds for vectored read |
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.
nit: add a .
|
💔 -1 overall
This message was automatically generated. |
steveloughran
left a comment
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.
+1
* HADOOP-18107 Adding scale test for vectored reads for large file part of HADOOP-18103.
part of HADOOP-18103. Contributed By: Mukund Thakur
part of HADOOP-18103. Contributed By: Mukund Thakur
part of HADOOP-18103. Contributed By: Mukund Thakur
…che#4273) part of HADOOP-18103. Contributed By: Mukund Thakur
part of HADOOP-18103.
Description of PR
How was this patch tested?
Reran the changed tests.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?