Skip to content
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

Pause/Resume Upload for Transfer Manager with Java S3Client #4886

Conversation

davidh44
Copy link
Contributor

@davidh44 davidh44 commented Feb 3, 2024

Motivation and Context

Currently pausing/resuming a file upload with TransferManager is only supported with CRT S3 Client.

Modifications

Adding support when using Java S3 Client

Testing

added integ tests
to add unit tests in separate PR

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@davidh44 davidh44 requested a review from a team as a code owner February 3, 2024 00:08
resumedUpload.completionFuture().join();
verifyMultipartUploadIdNotExist(resumableFileUpload);
assertThat(resumedUpload.progress().snapshot().totalBytes()).hasValue(bytes.length);

Files.write(largeFile.toPath(), originalBytes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unintentional change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to write back the original largeFile , since we overwrote it with "helloworld" to test file changed,
we are doing parameterized tests so this will run twice, one for each type of TM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do it in afterEach?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only overwrite it in this test, so dont need to write back for other tests

dagnir and others added 14 commits February 5, 2024 15:34
* including S3 Access Grants Plugin as part of Java SDK Bundle

---------

Co-authored-by: Shiva Kumar Mukkapati <mshvkmr@amazon.com>
- .json files files for each version are grouped under minor versions in
   .changes
 - Markdown files for each minor version are created in changelogs/ directory
 - Changelog scripts updated to add a link to older version in the generated
   changelog
…or use with your CAPTCHA JavaScript integration API.
…to the customers on the changes that they make on the domain.
…pArn, to the response of the logs:DescribeLogGroups action.
…ty to the customers on the changes that they make on the domain.
…c59dc86fd

Pull request: release <- staging/6f43c303-a28b-4966-b004-d38c59dc86fd
resumedUpload.completionFuture().join();
verifyMultipartUploadIdNotExist(resumableFileUpload);
assertThat(resumedUpload.progress().snapshot().totalBytes()).hasValue(bytes.length);

Files.write(largeFile.toPath(), originalBytes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do it in afterEach?

.uploadId(uploadId)
.bucket(putObjectRequest.bucket())
.key(putObjectRequest.key())
.partNumberMarker(0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any other parameter we should pass? Should we add a convert method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup we need SSE-C fields, added conversion method to setAllFields

.build();
ListPartsPublisher listPartsPublisher = s3AsyncClient.listPartsPaginator(request);
SdkPublisher<Part> partsPublisher = listPartsPublisher.parts();
partsPublisher.subscribe(part -> existingParts.put(part.partNumber(), SdkPojoConversionUtils.toCompletedPart(part)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we guarantee this gets finished before uploadParts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passing future and calling join() in Subscriber :: onNext()

@SdkInternalApi
public class PauseObservable {

private volatile UploadWithKnownContentLengthHelper.KnownContentLengthAsyncRequestBodySubscriber subscriber;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid exposing internal class and implementation detail, suggesting creating another layer of abstraction like this: hdavidh/pause-resume-upload-java...zoewang/PR4886-review

AWS and others added 10 commits February 7, 2024 19:06
…t replication feature as part of Lex Global Resiliency offering. This feature leverages a new set of APIs that allow customers to create bot replicas and replicate changes to bots across regions.
…2798c8cb1

Pull request: release <- staging/1c0c8ade-7bcb-4158-8160-4962798c8cb1
AWS and others added 13 commits February 8, 2024 19:04
… Chart Color Configuration; Documentation Update
…rallel & queued execution modes and add support for triggers with filtering on branches and file paths.
…ture. This feature allows Workspaces Core customers to provision workspaces without providing users. CreateWorkspaces and DescribeWorkspaces APIs will now take a new optional parameter "WorkspaceName".
…9f93cd5d9

Pull request: release <- staging/e358ef45-7969-4222-a98d-7fc9f93cd5d9
* Configure modeled signer properties for endpoint based auth scheme resolver

* Refactor the logic to use Knowledge Indexes
* AWS Glue Update: Introduce Catalog Encryption Role within Glue Data Catalog Settings. Introduce SASL/PLAIN as an authentication method for Glue Kafka connections

* Amazon WorkSpaces Update: Added definitions of various WorkSpace states

* Release 2.23.18. Updated CHANGELOG.md, README.md and all pom.xml.

* update aws-sdk-java pom to add imds and dyanmodb-enhanced (#4890)

* Performance improvement for sigv4 signing. (#4891)

This is a reintroduction of the reverted commit #4867. This includes a fix to the issue that caused the revert: improper handling of empty header values.

* Update to next snapshot version: 2.23.19-SNAPSHOT

* Delete CloudSearchv2IntegrationTest (#4888)

* Fix tag deletion command (#4892)

* including S3 Access Grants Plugin as part of Java SDK Bundle (#4881)

* including S3 Access Grants Plugin as part of Java SDK Bundle

---------



* Archive old changelog entries (< 2.23.0) (#4873)

- .json files files for each version are grouped under minor versions in
   .changes
 - Markdown files for each minor version are created in changelogs/ directory
 - Changelog scripts updated to add a link to older version in the generated
   changelog

* Changing indentation of config files to 4 spaces (#4889)

* Amazon EC2 Container Service Update: This release is a documentation only update to address customer issues.

* AWS WAFV2 Update: You can now delete an API key that you've created for use with your CAPTCHA JavaScript integration API.

* Amazon OpenSearch Service Update: This release adds clear visibility to the customers on the changes that they make on the domain.

* AWS AppSync Update: Support for environment variables in AppSync GraphQL APIs

* Amazon CloudWatch Logs Update: This release adds a new field, logGroupArn, to the response of the logs:DescribeLogGroups action.

* Amazon Elasticsearch Service Update: This release adds clear visibility to the customers on the changes that they make on the domain.

* Release 2.23.19. Updated CHANGELOG.md, README.md and all pom.xml.

* Update to next snapshot version: 2.23.20-SNAPSHOT

* Bump CRT version and expose setting memory limits for S3 calls (#4885)

* Bump aws-crt version to 0.29.9

* Exposes a setting to set the memory limit when making asynchronous calls with the CRT-based S3 client

* Activating SRA for this service (#4896)

* Fix request cancellation logic in the AWS CRT Sync HTTP client (#4887)

* Fix request cancellation logic in the AWS CRT Sync HTTP client

* Address feedback

* Fix bug to allow MpuS3Client to PUT COPY with SSE-C and Checksum

* Enable CRC32 for Multipart PUT COPY

* Address comments

* add changelog and update checksum check

* Address comments

* update javadocs

* Add unit tests

---------

Co-authored-by: AWS <>

Co-authored-by: aws-sdk-java-automation <43143862+aws-sdk-java-automation@users.noreply.github.com>
Co-authored-by: Zoe Wang <33073555+zoewangg@users.noreply.github.com>
Co-authored-by: Matthew Miller <millem@amazon.com>
Co-authored-by: John Viegas <70235430+joviegas@users.noreply.github.com>
Co-authored-by: Dongie Agnir <261310+dagnir@users.noreply.github.com>
Co-authored-by: shiva kumar <62441208+shiva958@users.noreply.github.com>
Co-authored-by: Shiva Kumar Mukkapati <mshvkmr@amazon.com>
Co-authored-by: Anna-Karin Salander <salande@amazon.com>
* Fix bug to allow MpuS3Client to PUT COPY with SSE-C and Checksum

* Enable CRC32 for Multipart PUT COPY

* Address comments

* add changelog and update checksum check

* Address comments

* update javadocs

* Add unit tests
Copy link

sonarcloud bot commented Feb 9, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
57.6% Coverage on New Code (required ≥ 80%)
3.2% Duplication on New Code (required ≤ 3%)
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

@davidh44
Copy link
Contributor Author

davidh44 commented Feb 9, 2024

messy merges from other PR, closing in favor of new PR
#4908

@davidh44 davidh44 closed this Feb 9, 2024
@davidh44 davidh44 deleted the hdavidh/pause-resume-upload-java branch February 9, 2024 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants