HADOOP-19256 Integrate PutIfNotExist functionality into S3A createFile #7329
+848
−67
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
steveloughran
requested changes
Feb 5, 2025
There was a problem hiding this comment.
catching up with this.
I can see it is still part way through doing those first changes I'd suggested. I don't think we need that map of options, I'd rather they get extracted.
we have two create options then
- reject overwrite
- later: require match + etag
the code is very, very similar, all that is different is that the second one needs an etag
we need to think of the best way to get both of these down.
For testing large files, I also want to add an option for "always do a multipart write" -but that can go in as a header with a new option defined.
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/ITestS3APutIfMatch.java
Outdated
Show resolved
Hide resolved
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/ITestS3APutIfMatch.java
Outdated
Show resolved
Hide resolved
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/ITestS3APutIfMatch.java
Outdated
Show resolved
Hide resolved
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/ITestS3APutIfMatch.java
Outdated
Show resolved
Hide resolved
...ols/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/magic/S3MagicCommitTracker.java
Outdated
Show resolved
Hide resolved
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/PutObjectOptions.java
Outdated
Show resolved
Hide resolved
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/RequestFactoryImpl.java
Outdated
Show resolved
Hide resolved
| .multipartUpload(CompletedMultipartUpload.builder().parts(partETags).build()); | ||
| if (putOptions.isconditionalPutEnabled()) { | ||
| requestBuilder.overrideConfiguration( |
There was a problem hiding this comment.
this is from the first patch, isn't it? I'll have to look at my comments there
|
|
||
| package org.apache.hadoop.fs.s3a.impl; | ||
|
|
||
| import org.apache.hadoop.conf.Configuration; |
There was a problem hiding this comment.
import ordering is wrong...i'll give you the IDE settings for this
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/impl/ITestS3APutIfMatch.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
+ hadoop common and s3a create file options wired up to s3a create file builder, and nowhere else. Change-Id: Ieaafa9ecdfd68306deb41ecf40c416374a310859
|
💔 -1 overall
This message was automatically generated. |
…art files createFile() options are passed down all the way to object creation * fs.option.create.conditional.overwrite: sets the header. MUST be in close(). After all, create(overwrite=false) is eager. * fs.option.create.conditional.overwrite.etag requests etag writes. MAY be in create(); may be in close(). * fs.option.create.content.type for mime type. * fs.option.create.in.close to ask for create in close, as fs capability. (maybe make this an fs path capability only?) * fs.s3a.create.multipart : allows tests to create small multipart files Javadocs of the fs.option.create try to define semantics; will need strict fs specification soon. Bool params come down in an enumset of flags; I'm going to do that in more code as it is more flexible over time than many booleans. - etag values are passed down but not wired up/tested - content type flag exists but is ignored New WriteObjectFlags enum is in new package o.a.h.fs.s3a.write; to match the streams package in another ongoing PR. Goal, as we maintain things, all code related to writing can go in here. also: options set by WriteObjectFlags return true in hasCapability() of the stream Change-Id: I301abd7397accbd278d05f42f858223ba1349fc8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
Cherry-pick of #7011, and additional changes on top of it
How was this patch tested?
integration tests ran on ap-south-1
Failing tests:
ITestAwsSdkWorkarounds.testNoisyLogging
ITestConnectionTimeouts.testObjectUploadTimeouts
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?