[TM upload pause/resume Part 2] Implement pause and resume for uploadFile #3357

zoewangg · 2022-08-16T18:17:58Z

This is part 2 of pause/resume for uploadFile.

zoewangg · 2022-08-16T18:19:46Z

...ain/java/software/amazon/awssdk/transfer/s3/internal/serialization/CrtUploadResumeToken.java

+import software.amazon.awssdk.protocols.jsoncore.JsonWriter;
+
+@SdkInternalApi
+public class CrtUploadResumeToken {


This is the POJO class for the JSON returned from crt classS3MetaRequest#pause

zoewangg · 2022-08-16T18:21:25Z

.../main/java/software/amazon/awssdk/services/s3/internal/crt/S3MetaRequestPauseObservable.java

+import software.amazon.awssdk.crt.s3.S3MetaRequest;
+
+@SdkInternalApi
+public class S3MetaRequestPauseObservable {


The naming might be confusing. LMK if you can think of a better name, but this is the class created in Transfer Manager and passed to S3CrtAsyncHttpClient through SdkHttpExecutionAttribute.

I thought about using generic types instead of String, but it's a bit awkward in SdkHttpExecutionAttribute because we'd need to do casting for generic types

public static final S3InternalSdkHttpExecutionAttribute<S3MetaRequestPauseObservable> METAREQUEST_PAUSE_OBSERVABLE = new S3InternalSdkHttpExecutionAttribute<>(S3MetaRequestPauseObservable.class);

Could you please add a Java Doc for this class to know the intent/responsibility of this task , just for internal references.

...va/software/amazon/awssdk/transfer/s3/S3TransferManagerUploadPauseResumeIntegrationTest.java

...sfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/model/ResumableFileUpload.java

joviegas · 2022-08-22T01:33:11Z

...s/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java

@@ -87,15 +89,21 @@ public CompletableFuture<Void> execute(AsyncExecuteRequest asyncRequest) {
            new S3CrtResponseHandlerAdapter(executeFuture, asyncRequest.responseHandler());

        S3MetaRequestOptions.MetaRequestType requestType = requestType(asyncRequest);
+        String resumeToken = asyncRequest.httpExecutionAttributes().getAttribute(CRT_PAUSE_RESUME_TOKEN);


what happen if we try to do following

clientOne.uploadFile_One

clientOne.Pause_one

clientOne.uploadFile_Two

Wait for uploadFile_Two to complete

Then Resume file One that is clientOne.Pause_one

Do we need a test case for this ?

Do you mean uploading the file, pausing and then uploading the same file from start again? hmm, good question, let me try it. I think the parts uploaded in the first attempt will be overridden by the second attempt.

I ran tests with the following order:

uploading the file

pausing

uploading the same file from start

resuming from step2

Step 1, 2 and 3 succeeded. Step 4 threw NoSuchUploadException, which makes sense because the object was uploaded successfully in step 3

I'm not sure if we should add a test case for this because it is more of a service behavior

joviegas · 2022-08-22T01:37:31Z

.../main/java/software/amazon/awssdk/services/s3/internal/crt/S3MetaRequestPauseObservable.java

+import software.amazon.awssdk.crt.s3.S3MetaRequest;
+
+@SdkInternalApi
+public class S3MetaRequestPauseObservable {


Could you please add a Java Doc for this class to know the intent/responsibility of this task , just for internal references.

...ager/src/main/java/software/amazon/awssdk/transfer/s3/internal/DefaultS3TransferManager.java

...ain/java/software/amazon/awssdk/transfer/s3/internal/serialization/CrtUploadResumeToken.java

...3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClient.java

sonarcloud · 2022-08-31T23:26:04Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
66 Code Smells

75.7% Coverage
7.1% Duplication

joviegas · 2022-09-06T18:55:15Z

...ager/src/main/java/software/amazon/awssdk/transfer/s3/internal/DefaultS3TransferManager.java

+        UploadFileRequest uploadFileRequest = resumableFileUpload.uploadFileRequest();
+        PutObjectRequest putObjectRequest = uploadFileRequest.putObjectRequest();
+        if (fileModified) {
+            log.debug(() -> String.format("The file (%s) has been modified since "


This looks more of a warning or Info so IMO this should of log level higher than debug

I thought about this, and imo INFO may be a bit too verbose (we have customers complaining about verbosity of our logs before) and WARN may scare customers and this is probably not something we should alarm on.

* Replacing S3TransferManager interfaces that allowed builder methods of S3ClientConfiguration with builder methods of S3AsyncClient (#3247) * Added customization in codegen to generate additional builder methods (#3252) * S3Object based DownloadFilter and removing DownloadFileContext as destination based filter is removed (#3258) * Moved tm POJO classes to model pckage and tm config classes to config package. Added integration tests for s3 select using S3CrtAsyncClient (#3289) * Fix broken integ test (#3301) * S3 Transfer manager renamings based on feedback: (#3297) 1. Rename destinationDirectory to destination. 2. Move DownloadDirectoryRequest.prefix and delimiter to just rely on modifying the list requests. 3. Remove upload directory recursive option in favor of using maxDepth(1). 4. Rename UploadDirectoryRequest's prefix and delimiter to s3Prefix and s3Delimiter. 5. Rename ResumableFileDownload's to* and writeTo* methods to serializeTo*. Remove charsets from write/read methods, and just use UTF-8. 6. Do not base64 encode when writing ResumableFileDownload to disk. * Allow pausing a resumed download even when the download hasn't already started. (#3300) * Add POJO classes for upload pause/resume (#3337) * Refactoring of Transfer manager APIs (#3374) * Refactoring of Transfer manager APIs * Merging the integ test failure Pr 2119 from stagging branch * Add flexible checksum support and update perf tests (#3376) * Fix flexiblechecksum implementation (#3391) * [TM upload pause/resume Part 2] Implement pause and resume for uploadFile (#3357) * Implement pause and resume for uploadFile * Update Javadocs * address feedback * Implement automatic multipart copy functionality in S3 CRT async client (#3403) * Implement automatic multipart copy functionality in S3 CRT async client * Add more tests * fix cancellation logic * Refactor CopyRequestProvider, fix request conversion and add more tests * Fix checkstyle * Transfer Manager tests refactoring (#3420) * Remove use of Junit4, clean up and consolidate tests in tm module * Ignoring the test if unicode can't be used as directory name * Add serialization and deserialization support for ResumableFileUpload (#3432) * Support serialization and deserialization of ResumableFileUpload * Address feedback * Empty json should be unmarshalled to empty map * Errors should not be wrapped - S3 Transfer Manager (#3433) * Errors should not be wrapped * update handleException() * Changelog entry * Resolve comments Update changelog description, refactor handleException(), add test * Add failed message to SdkException * Refactor handleException() and format changelog (#3461) * Fixed an issue where SSEC params were not correctly passed in copy operation (#3464) * Replace inline snippets with external compilable snippets (#3465) * Replace inline snippets with external compilable snippets * Fix build and address feedback * Fix build * Only enable CRT checksum for getObject and putObject (#3477) * Only use CRT flexible checksum for getObject and putObject * Fix build * Fix integ tests set up and tear down steps (#3485) * Enable backpressure in TM (#3533) * integrate with crt s3 flow control * Update benchmark code * Add backpressure config * Change window size * Update initial window size * Change intial window size * Use heap max memory for initial window size * Give some buffer * change window size * Make read buffer size configurable * Log result to a file * Various updates * Various updates * Add CRT benchmark * Various updates * Fix checkstyle errors and tests * Fix flaky test * Fix checkstyle errors * Add validation * Add tests * For copy operation, always forward multipart copy exception from one … (#3549) * For copy operation, always forward multipart copy exception from one request to other multipart copy requests * Minor refactoring in CopyObjectHelper (#3552) * Add benchmarks for copy, uploadDirectory and downloadDirectory (#3551) * Add benchmarks for copy, uploadDirectory and downloadDirectory * Update sample code and fix snippet path (#3567) * Update sample code and fix snippet path * Fix link * Integrate with CRT checksum fix (#3566) * Integrate with CRT checksum fix * Rename sourceDirectory to source and add S3AsycncClient#crtCreate (#3572) * Rname sourceDirectory to source and add S3AsycncClient#crtCreate * Use ByteBufferStoringSubscriber (#3581) * Use ByteBufferStoringSubscriber * Add a comment * Create constant for bytes bufferred * Increase chunk size for file upload (#3583) * Rename S3TransferManager.build().maxDepth to uploadDirectoryMaxDepth, rename S3TransferManager.builder().s3AsyncClient to .s3Client (#3584) * Fixed an issue where sdkRepsonse is not present in the ProgressSnapshot for upload and copy (#3585) * Throw UnsupportedOperationException if a user tries to pause a upload… (#3586) * Throw UnsupportedOperationException if a user tries to pause a upload with non CRT-based S3 client * Use SimplePublisher (#3594) * Update documentation for Transfer Manager (#3592) * Update javadoc * Integrate with latest CRT pause/resume fix (#3588) * Integrate with latest CRT pause/resume fix * Bump CRT version * Fixed an issue that could result in uncompletable future when headObject request threw exception in copy (#3609) * Make crt dependency optional in transfer manager module (#3613) * Make aws-crt an optional dependency in s3-transfer-manager module. * Update README * Fix category for changelog entries Co-authored-by: John Viegas <70235430+joviegas@users.noreply.github.com> Co-authored-by: Matthew Miller <millem@amazon.com> Co-authored-by: David Ho <70000000+davidh44@users.noreply.github.com>

…9a39033b7 Pull request: release <- staging/289204b4-9332-46d7-8aee-1b69a39033b7

zoewangg commented Aug 16, 2022

View reviewed changes

joviegas reviewed Aug 22, 2022

View reviewed changes

zoewangg force-pushed the zoewang/uploadPause-parts2 branch 2 times, most recently from 0fbf57c to 1cc5c42 Compare August 30, 2022 21:50

zoewangg marked this pull request as ready for review August 31, 2022 18:33

zoewangg requested a review from a team as a code owner August 31, 2022 18:33

millems reviewed Aug 31, 2022

View reviewed changes

zoewangg added 6 commits August 31, 2022 13:45

Implement pause and resume for uploadFile

a41eb3a

Fix merge conflict

8798d6d

Adding more tests

25292d7

Add more tests

098c702

Update Javadocs

b6da563

address feedback

3be4cf1

zoewangg force-pushed the zoewang/uploadPause-parts2 branch from 68b2c47 to 3be4cf1 Compare August 31, 2022 22:19

joviegas approved these changes Sep 6, 2022

View reviewed changes

zoewangg merged commit 20bcefe into feature/master/transfermanager-GA Sep 7, 2022

zoewangg deleted the zoewang/uploadPause-parts2 branch September 7, 2022 00:21

zoewangg mentioned this pull request Sep 9, 2022

S3TransferManager V2 - Support for Upload Pause/Resume #3120

Closed

1 task

aws-sdk-java-automation added a commit that referenced this pull request Oct 17, 2024

Merge pull request #3357 from aws/staging/289204b4-9332-46d7-8aee-1b6…

a6d90aa

…9a39033b7 Pull request: release <- staging/289204b4-9332-46d7-8aee-1b69a39033b7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TM upload pause/resume Part 2] Implement pause and resume for uploadFile #3357

[TM upload pause/resume Part 2] Implement pause and resume for uploadFile #3357

zoewangg commented Aug 16, 2022 •

edited

Loading

zoewangg Aug 16, 2022

zoewangg Aug 16, 2022

joviegas Aug 22, 2022

joviegas Aug 22, 2022

zoewangg Aug 26, 2022

zoewangg Aug 30, 2022

joviegas Aug 22, 2022

sonarcloud bot commented Aug 31, 2022 •

edited

Loading

joviegas Sep 6, 2022

zoewangg Sep 6, 2022

[TM upload pause/resume Part 2] Implement pause and resume for uploadFile #3357

[TM upload pause/resume Part 2] Implement pause and resume for uploadFile #3357

Conversation

zoewangg commented Aug 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zoewangg commented Aug 16, 2022 •

edited

Loading

sonarcloud bot commented Aug 31, 2022 •

edited

Loading