-
Notifications
You must be signed in to change notification settings - Fork 594
HDDS-11699. Remove unnecessary information about parts when downloading multipart files. #7558
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
Conversation
|
Thanks @juncevich for the patch. Please take a look at failed tests: https://github.com/juncevich/ozone/actions/runs/12272551726/job/34242975199#step:7:13 https://github.com/juncevich/ozone/actions/runs/12272551726/job/34242757028#step:7:18 |
@adoroszlai Thank you. Will fix. |
ivandika3
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.
@juncevich Thanks for the patch. Left some comments.
High level comments
- Please add some tests to test the S3 get with part number
- Unit tests
TestKeyManagerImplto test the changes inKeyManagerImpl
- Integration tests
TestOzoneRpcClientAbstractto test thegetS3KeyDetailsandgetS3PartKeyInfoAbstractS3SDKV1Teststo test get object, specifying the part number.GetObjectRequestcan be specify the part number
- Acceptance test
objectputget.robot: Add theget-objectfor with--part-numberspecified (see https://docs.aws.amazon.com/cli/latest/reference/s3api/get-object.html)
- Unit tests
- Might need to add a new
OzoneManagerVersionfor compatibility.- See how
RpcClient#listStatusLightimplements it
- See how
- Please simplify the logic, the change should simply move the filtering logic from client (S3G) to server (OM) side
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyArgs.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
Outdated
Show resolved
Hide resolved
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
Outdated
Show resolved
Hide resolved
41abc0e to
3abbf6d
Compare
ff132b3 to
d1172c5
Compare
a2451e2 to
b7cb7ea
Compare
|
@ivandika3 would you re-review this pr |
|
@juncevich Thanks for the update. Just looking briefly, seems #7558 (comment) has not been addressed? I'll review the rest ASAP. Thanks again for iterating on this. |
Hmm, yep, you're right. I definitely fixed it. I will find a place where I lost it. |
b7cb7ea to
487a563
Compare
Fixed. Maybe I have to recheck tests. |
ivandika3
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.
Thanks for the update. Overall LGTM. I left some minor comments.
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
Outdated
Show resolved
Hide resolved
.../src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java
Show resolved
Hide resolved
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
Outdated
Show resolved
Hide resolved
ivandika3
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.
Thanks for the update. LGTM +1.
Let's wait for a clean CI.
Please check failures: https://github.com/apache/ozone/actions/runs/12687113055/job/35361805439?pr=7558#step:6:4472 |
@adoroszlai, thank you for your message. Build again green. |
|
Thanks @juncevich for the patch, @ivandika3 for the review. |
What changes were proposed in this pull request?
HDDS-11699. Remove unnecessary information about parts when downloading multipart files
Whenever information was requested about a particular part of a multipart file, it was obtained about all parts and filtered on the Ozone Manager side. To reduce work with unnecessary information, the ability to ask for information about the current part was added.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-11699
How was this patch tested?
It was tested manually and by perf tests. Perf tests shown performance improvements:
Reduce max CPU consumption approximately 3 times.
Reduce HEAP size approximately 3 times.
Average GC pauses approximately 2 times.