[
https://issues.apache.org/jira/browse/HADOOP-19256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17876979#comment-17876979
]
ASF GitHub Bot commented on HADOOP-19256:
-----------------------------------------
ahmarsuhail commented on code in PR #7011:
URL: https://github.com/apache/hadoop/pull/7011#discussion_r1732579400
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java:
##########
@@ -608,6 +611,16 @@ private long putObject() throws IOException {
builder.putOptions,
false);
+ PutObjectRequest.Builder maybeModifiedPutIfAbsentRequest =
putObjectRequest.toBuilder();
+ Map<String, String> optionHeaders = builder.putOptions.getHeaders();
+
+ if (optionHeaders != null &&
optionHeaders.containsKey(IF_NONE_MATCH_HEADER)) {
+ maybeModifiedPutIfAbsentRequest.overrideConfiguration(
Review Comment:
as mentioned below as well, think we should upgrade SDK and then use the new
.ifNoneMatch().
Also I would recommend you move all of this logic into a new private method
in this class.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java:
##########
@@ -608,6 +611,16 @@ private long putObject() throws IOException {
builder.putOptions,
false);
+ PutObjectRequest.Builder maybeModifiedPutIfAbsentRequest =
putObjectRequest.toBuilder();
+ Map<String, String> optionHeaders = builder.putOptions.getHeaders();
+
+ if (optionHeaders != null &&
optionHeaders.containsKey(IF_NONE_MATCH_HEADER)) {
+ maybeModifiedPutIfAbsentRequest.overrideConfiguration(
Review Comment:
actually, I would move this logic to RequestFactoryImpl. But you will need
to add in a flag to the method signature, "isConditonalPutEnabled", and only
add the if-none-match header in when sConditonalPutEnabled is true.
Basically, you only want to add ` if-none-match` when the PUT is coming
from S3ABlockOutputStream.close() and not from anywhere else.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/RequestFactoryImpl.java:
##########
@@ -517,12 +518,22 @@ public CreateMultipartUploadRequest.Builder
newMultipartUploadRequestBuilder(
public CompleteMultipartUploadRequest.Builder
newCompleteMultipartUploadRequestBuilder(
String destKey,
String uploadId,
- List<CompletedPart> partETags) {
+ List<CompletedPart> partETags,
+ PutObjectOptions putOptions) {
+
// a copy of the list is required, so that the AWS SDK doesn't
// attempt to sort an unmodifiable list.
- CompleteMultipartUploadRequest.Builder requestBuilder =
-
CompleteMultipartUploadRequest.builder().bucket(bucket).key(destKey).uploadId(uploadId)
+ CompleteMultipartUploadRequest.Builder requestBuilder;
+ Map<String, String> optionHeaders = putOptions.getHeaders();
+
+ if (optionHeaders != null && optionHeaders.containsKey("If-None-Match")) {
+ requestBuilder =
CompleteMultipartUploadRequest.builder().bucket(bucket).key(destKey).uploadId(uploadId)
+ .overrideConfiguration(override
->override.putHeader("If-None-Match", optionHeaders.get("If-None-Match")))
.multipartUpload(CompletedMultipartUpload.builder().parts(partETags).build());
+ } else {
+ requestBuilder =
CompleteMultipartUploadRequest.builder().bucket(bucket).key(destKey).uploadId(uploadId)
Review Comment:
you don't need to duplicate all of this. just do:
```
requestBuilder =
CompleteMultipartUploadRequest.builder().bucket(bucket).key(destKey).uploadId(uploadId)
.multipartUpload(CompletedMultipartUpload.builder().parts(partETags).build());
if (optionHeaders != null && optionHeaders.containsKey("If-None-Match"))
{
requestBuilder =
CompleteMultipartUploadRequest.builder().overrideConfiguration(override
->override.putHeader("If-None-Match", optionHeaders.get("If-None-Match"));
}
```
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1390,6 +1390,13 @@ private Constants() {
*/
public static final String FS_S3A_CREATE_PERFORMANCE =
"fs.s3a.create.performance";
+ /**
+ * Flag for commit if none match.
+ * This can be set in the {code createFile()} builder.
+ * Value {@value}.
+ */
+ public static final String FS_S3A_CREATE_IF_NONE_MATCH =
"fs.s3a.create.header.If-None-Match";
Review Comment:
+1, should change to `fs.s3a.create.header.if.none.match`
> Support S3 Conditional Writes
> -----------------------------
>
> Key: HADOOP-19256
> URL: https://issues.apache.org/jira/browse/HADOOP-19256
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Reporter: Ahmar Suhail
> Priority: Major
> Labels: pull-request-available
>
> S3 Conditional Write (Put-if-absent) capability is now generally available -
> [https://aws.amazon.com/about-aws/whats-new/2024/08/amazon-s3-conditional-writes/]
>
> S3A should allow passing in this put-if-absent header to prevent over writing
> of files.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
