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

Need a simple mechanism to totally abort a multipart upload #3274

Closed
2 tasks
alamaral opened this issue Jun 29, 2022 · 10 comments
Closed
2 tasks

Need a simple mechanism to totally abort a multipart upload #3274

alamaral opened this issue Jun 29, 2022 · 10 comments
Labels
closed-for-staleness feature-request A feature should be added or improved. p2 This is a standard priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. transfer-manager

Comments

@alamaral
Copy link

Describe the feature

The proposed feature is a simple mechanism to cancel a multipart upload, close socket, kill upload threads, etc.
Something simple, like transferManager.cancelTransfer(fileUpload). I assume that the transferManager, or some
class that it uses internally, is creating threads to perform the uploads of the individual file pieces. This feature
would need to signal those threads to stop, and each open socket should be closed.

Use Case

We have a java app that allows users to stream and record live video, and queue recorded video files for upload to S3. The upload queue is handled in our app, with only a single upload allowed at one time, and transfers are not allowed while doing a live stream, due to network bandwidth constraints. There are two cases where we want to cancel the current upload:

  1. The user decides he doesn't want to upload the file to S3 anymore and hits the "Cancel Upload" button.
  2. The user wants to stream video, and we need the network bandwidth, so we need to stop any upload that is in progress.

Proposed Solution

No response

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

AWS Java SDK version used

2

JDK version used

openjdk version "1.8.0_282" OpenJDK Runtime Environment (build 1.8.0_282-8u282-b08-0ubuntu1~18.04-b08) OpenJDK 64-Bit Server VM (build 25.282-b08, mixed mode)

Operating System and version

Ubuntu 18.04

@alamaral alamaral added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 29, 2022
@debora-ito
Copy link
Member

You can call cancel() on the CompletableFuture that is returned with every TransferManager operation. For example:

S3TransferManager tm = S3TransferManager.create();

FileUpload fileUpload =
                tm.uploadFile(u -> u.source(Paths.get("myFile.txt"))
                        .putObjectRequest(p -> p.bucket("bucket").key("key")));

fileUpload.completionFuture().cancel(true);

Let us know if this helps.

@debora-ito debora-ito added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. transfer-manager and removed needs-triage This issue or PR still needs to be triaged. labels Jul 6, 2022
@debora-ito debora-ito self-assigned this Jul 6, 2022
@TheKeveloper
Copy link

@debora-ito does canceling the completion future terminate the upstream execution in this case? I don't believe the standard implementation of CompletableFuture#cancel would terminate the actual upload and would just propagate a cancellation exception to any listeners. There might be additional logic in the implementation of completionFuture that addresses this issue by actually terminating the upload though?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Jul 10, 2022
@zoewangg
Copy link
Contributor

@TheKeveloper yes, canceling the completion future will terminate the requests. We have cancellation propagation logic in place :) Under the hood, S3 CRT client will abort the upload and clean up resources properly after the request is cancelled.

https://github.com/aws/aws-sdk-java-v2/blob/master/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java#L130-L133

@alamaral
Copy link
Author

From what I've seen cancelling the completion future does not abort the upload. I can start an upload, cancel the CF and I can see the connections to S3 and the traffic going through them continue on for as long as it would have taken to upload the file had it not been cancelled. If I then look at the incomplete MPU's on S3 it clearly shows it has increased by the size of the file being uploaded. Note that the file upload is never completed on S3 by combining the various MPU's into a single file, but the bytes are all there.

@zoewangg As far as the code that you sited above, could it be that what it's doing is sending a cancel request to the S3 server, but the client threads keep happily on sending data which the server just doesn't assemble into the final file? I.e. it tells S3 the upload has been cancelled, but the threads doing the upload aren't stopped?

We ended up working around the problem by forking off a separate java process to do the file upload, and if we want to cancel it we just kill the process. When we do that I can see the traffic on the network connections to S3 go to 0 within a couple of seconds, whereas with the CF.cancel alone the traffic would continue for an hour or more for a very large file on a slow network.

I did read somewhere, forgive me I didn't note where, that the completion future doesn't have any information about the threads or other resources like sockets that may have been created when the upload was started, so it has no control over them, and can't stop them. This information may not have been specific to the S3 SDK, which may be maintaining that information itself, but in my experience cancelling the CF doesn't do what is expected.

@zoewangg
Copy link
Contributor

@alamaral thanks for the detailed analysis. The cancellation logic is on client side and it should have information of the request and ongoing transfers. I took another look, and it seems the current cancellation implementation is to stop scheduling new multipart uploads, wait for all existing uploads to finish and then invoke abortMultipart API. So in your case, if the network is slow, it could take longer for all existing upload tasks to finish. It seems cancelling existing upload tasks immediately is preferred, and I think it's fair. We will evaluate the design and discuss it within the team. I will update when we have more information.

@debora-ito debora-ito removed their assignment Aug 12, 2022
@alibenmessaoud
Copy link

alibenmessaoud commented Sep 13, 2022

Based on the added logs in my code, I had these traces:

2022-09-13 11:54:28.362+0200 WARN app [http-nio-8086-exec-5] [] [s.a.a.t.s.p.LoggingTransferListener.warn] - Transfer failed. java.util.concurrent.CancellationException: null at java.base/java.util.concurrent.CompletableFuture.cancel(CompletableFuture.java:2478) ... s3Tasks=[java.util.concurrent.CompletableFuture@5a4d138a[Completed exceptionally: java.util.concurrent.CancellationException]] 2022-09-13 11:54:30.049+0200 INFO app [Thread-23] [] [s.a.a.t.s.p.LoggingTransferListener.info] - |===== | 25.0% 2022-09-13 11:54:32.424+0200 INFO app [Thread-23] [] [s.a.a.t.s.p.LoggingTransferListener.info] - |====== | 30.0% ... 2022-09-13 11:54:59.980+0200 INFO app [Thread-27] [] [s.a.a.t.s.p.LoggingTransferListener.info] - |====================| 100.0% ...

Even with the cancellation being fired, the file was uploaded in the bucket at the same time as the logs:

image

LoggingTransferListener did not show logs before 25.0%.

For our case, we really need this feature to cancel large file upload operations.

@yasminetalby yasminetalby added the p2 This is a standard priority issue label Nov 14, 2022
@robertbio
Copy link

@zoewangg is there any workaround for this besides forking the Java process and killing it like @alamaral mentions?
In my case, it uploads the full 2.5 GB file correctly even if cancel is called right after it starts uploading.

@robertbio
Copy link

Calling FileUpload#pause() before canceling the future thread seems to make it stop uploading the current file too. The documentation mentions that the pause() only works with CRT-Based S3 clients.

S3TransferManager tm = S3TransferManager.create();

FileUpload fileUpload =
                tm.uploadFile(u -> u.source(Paths.get("myFile.txt"))
                        .putObjectRequest(p -> p.bucket("bucket").key("key")));
fileUpload.pause();
fileUpload.completionFuture().cancel(true);

@zoewangg
Copy link
Contributor

zoewangg commented Feb 24, 2024

Hey all, could you try with the latest Java SDK and aws-crt-java version? This is supported now via awslabs/aws-c-s3#395

If you cancel the return future, the underlying s3 requests should be aborted.

@zoewangg zoewangg added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Feb 24, 2024
Copy link

github-actions bot commented Mar 5, 2024

It looks like this issue has not been active for more than five days. In the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please add a comment to prevent automatic closure, or if the issue is already closed please feel free to reopen it.

@github-actions github-actions bot added closing-soon This issue will close in 4 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will close in 4 days unless further comments are made. labels Mar 5, 2024
@github-actions github-actions bot closed this as completed Mar 9, 2024
aws-sdk-java-automation pushed a commit that referenced this issue Sep 23, 2024
temporarily exclude rds InvalidMaxAcuException from japicmp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-for-staleness feature-request A feature should be added or improved. p2 This is a standard priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. transfer-manager
Projects
None yet
Development

No branches or pull requests

7 participants