-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Enabling Support for Conditional Multi-Part Upload #18093
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
base: main
Are you sure you want to change the base?
Enabling Support for Conditional Multi-Part Upload #18093
Conversation
❌ Gradle check result for 2c31c83: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Tanishq Ranjan <[email protected]>
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.
We also support async multipart upload(AsyncTransferManager
has the logic). Do you want to extend that capability as well?
cc: @ashking94
@@ -509,6 +514,169 @@ private String buildKey(String blobName) { | |||
return keyPath + blobName; | |||
} | |||
|
|||
public void executeMultipartUploadIfEtagMatches( |
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.
Where is this code getting consumed? I don't see a corresponding change in the generic blob store interface
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.
Sorry missed this, The executeMultipartUploadIfEtagMatches
method (now renamed to executeMultipartUploadConditionally
) is meant to be consumed via the writeBlobWithMetadataConditionally
and writeBlobConditionally
methods in the generic blob store interface. These interfaces are introduced in this PR currently under review.
Check #18064 (review). I had the same question. Will let this be addressed at both places. |
Introduces ConditionalWriteOptions and ConditionalWriteResponse classes to support conditional write operations in BlobContainer implementations.
…tainer & AsyncMultiStreamEncryptedBlobContainer
❌ Gradle check result for 9fdefaa: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for d8d8f9a: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Flaky Test :: #17551 classMethod – org.opensearch.repositories.s3.S3BlobContainerRetriesTests |
Signed-off-by: Tanishq Ranjan <[email protected]>
❌ Gradle check result for 7a7e240: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 5cfeb6e: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for cbc0188: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 0eedd2c: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Hi @Bukhtawar (and thanks again @ashking94 for the nudge)—I’ve now implemented the full async conditional-upload support alongside the sync flow:
Comprehensive tests have been added to validate every path: Test suite
|
Enabling Support for Conditional Multi-Part Upload (#18093)
At a Glance:
Implement both synchronous and asynchronous conditional-upload methods in
S3BlobContainer
, backed by logical functions inAsyncTransferManager
Summary of Changes
Add
executeMultipartUploadConditionally(...)
toS3BlobContainer
Add
asyncBlobUploadConditionally(...)
toS3BlobContainer
Implement
uploadObjectConditionally(...)
inAsyncTransferManager
Introduce helper routines:
uploadInOneChunkConditionally
,uploadInPartsConditionally
,doUploadInPartsConditionally
, andhandleConditionalException
Define
ConditionalWriteOptions
to express “If-Match” and “If-None-Match” conditionsDefine
ConditionalWriteResponse
to wrap returned ETag or propagate failuresExtend tests to cover conditional success, precondition failures, data integrity, and size boundaries
Core Workflow
Validate part size (≥ 5 MB) and total upload size (≤ 5 TB).
Initialize upload by creating a multipart request with bucket, key, ACL, storage class, metadata, and SSE (AES256 or KMS).
Transfer Data
Sync: iterate parts sequentially using AWS SDK, buffering if retries are enabled.
Async: delegate to
AsyncTransferManager.uploadObjectConditionally
, which chooses single-chunk or multipart flow via its helper methods.Complete Conditionally by setting either
If-Match
orIf-None-Match: *
on the complete-upload request based onConditionalWriteOptions
.Respond with
ConditionalWriteResponse.success(etag)
on success or translate failures:HTTP 412 →
OpenSearchException("stale_primary_shard")
+ abort multipart.Other errors →
IOException
(sync) or failedCompletableFuture
/listener (async) + abort if uploadId exists.Abort failures are logged but do not mask the original exception.
Test Coverage
executeMultipartUploadConditionally Test Suite
testExecuteMultipartUploadConditionallyWithEtagMatchSuccess
testExecuteMultipartUploadConditionallyWithMetadataAndSSE
testExecuteMultipartUploadConditionallyContentIntegrity
testExecuteMultipartUploadConditionallySizeValidation
testExecuteMultipartUploadConditionallyPreconditionFailed
AsyncTransferManager Conditional Upload Tests
testConditionalOneChunkUpload
testConditionalOneChunkUploadPreconditionFailed
testConditionalOneChunkUploadCorruption
testConditionalMultipartUploadPreconditionFailed
testConditionalMultipartUploadCorruption
Container-Level and Retry Tests
testFailureWhenLargeFileRedirectedConditional
testLargeFileRedirectedConditional
testLargeFileRedirectedConditionalPreconditionFailed
testLargeFileRedirectedConditionalConflict
testWriteBlobByStreamsConditionalFailure
Related Issue
Concerned RFC : RFC #17763
Parent Meta Issue : Meta #17859
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Flow Diagram for executeMultipartUploadConditionally: