Repository navigation
Conversation
# Conflicts: # hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java
# Conflicts: # hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java # hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for working on this! @F64116045 Looks good overall. I left a few inline comments.
| + "client-side InfoBucket layout check for getFileChecksum"), | ||
| ZDU(100, "OzoneManager version that supports zero downtime upgrade"), | ||
|
|
||
| S3_MULTIPART_UPLOAD_ABORT_CONTEXT(101, |
There was a problem hiding this comment.
Thanks for the heads-up. I’ll bump the OM version in #11458 if this merges first.
|
|
||
| String keyName = "key"; | ||
| String uploadID = bucket.initiateMultipartUpload(keyName).getUploadID(); | ||
| store.getClientProxy().abortS3MultipartUpload(BUCKET_NAME, keyName, uploadID); |
There was a problem hiding this comment.
Could we also cover a link bucket in the tenant volume? The link may point to another volume with a different layout.
There was a problem hiding this comment.
Sure, added it!
Also covered different layouts between the link and target bucket.
|
|
||
| /** Abort a multipart upload, optionally resolving its S3 volume on OM. */ | ||
| default void abortMultipartUpload(OmKeyArgs omKeyArgs, boolean assumeS3Context) throws IOException { | ||
| throw new UnsupportedOperationException("Write requests are handled through OMClientRequest"); |
There was a problem hiding this comment.
nit: Should we reuse the message from the neighboring methods for consistency?
| throw new UnsupportedOperationException("Write requests are handled through OMClientRequest"); | |
| throw new UnsupportedOperationException("OzoneManager does not require " + | |
| "this to be implemented, as write requests use a new approach."); |
There was a problem hiding this comment.
yes, updated. thanks
|
thanks @chihsuan for the review, I've addressed the comment! |
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks @F64116045, I just noticed #11479 puts assumeS3Context on OMRequest, while this PR adds it to MultipartUploadAbortRequest. Could we align on one place?
I'd lean toward the OMRequest approach in #11479, so writes can clear it once before Ratis.
@chungen0126 would that work as the convention for the remaining subtasks?
| "this to be implemented, as write requests use a new approach."); | ||
| } | ||
|
|
||
| /** Abort a multipart upload, optionally resolving its S3 volume on OM. */ |
There was a problem hiding this comment.
nit: Could we mention that OM replaces the volume in omKeyArgs when assumeS3Context is true?
| /** Abort a multipart upload, optionally resolving its S3 volume on OM. */ | |
| /** | |
| * Abort a multipart upload. | |
| * @param assumeS3Context if true, OM resolves the S3 volume and ignores the volume in omKeyArgs. | |
| */ |
Thanks, agree. moved |
3 participants
What changes were proposed in this pull request?
S3G currently makes two OM RPCs to abort a multipart upload:
GetS3VolumeContextto find the volume, followed byAbortMultiPartUploadto cancel the upload.With
assumeS3Context=true, S3G can send the abort request directly. OM can resolve the volume while handling that request, reducing the RPCs from 2 to 1.What is the link to the Apache JIRA?
https://issues.apache.org/jira/browse/HDDS-16797
How was this patch tested?
CI: https://github.com/F64116045/ozone/actions/runs/38330665974
Generated-by: Codex (GPT-6)