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

[2.0] aws-s3-multipart does not retry prepareUploadParts requests #3190

Closed
anark opened this issue Sep 10, 2021 · 8 comments · Fixed by #3224
Closed

[2.0] aws-s3-multipart does not retry prepareUploadParts requests #3190

anark opened this issue Sep 10, 2021 · 8 comments · Fixed by #3224
Labels
AWS S3 Plugin that handles uploads to Amazon AWS S3 Bug

Comments

@anark
Copy link
Contributor

anark commented Sep 10, 2021

When using aws-s3-multipart with uppy 2.0 if a prepareUploadParts request fails due to a network hiccup or any other reason(Failed to fetch)
The upload stalls and never tries to continue.

We should be able to retry these requests as well as the actual uploads. Is there anyway to allow these requests to be retried?

@anark anark changed the title aws-s3-multipart does not retry request to prepareUploadParts [2.0] aws-s3-multipart does not retry request to prepareUploadParts Sep 11, 2021
@anark anark changed the title [2.0] aws-s3-multipart does not retry request to prepareUploadParts [2.0] aws-s3-multipart does not retry prepareUploadParts requests Sep 11, 2021
@Murderlon
Copy link
Member

@mifi @martin-brennan do you feel retrying prepareUploadParts makes sense for aws-s3-multipart?

@Murderlon Murderlon added AWS S3 Plugin that handles uploads to Amazon AWS S3 and removed Triage labels Sep 13, 2021
@martin-brennan
Copy link
Contributor

@Murderlon this is quite a timely issue. I am about to experiment more with the multipart upload functionality to see how it handles large (multi-GB) files, and last week at Discourse we discussed internally the need for retrying prepareUploadParts and each individual part upload to retry on failure.

Since each part is already retryable, I think it makes sense to make prepareUploadParts retryable as well. It would be quite frustrating to upload a multi-GB file which then fails on the final prepareUploadParts request to get the last 5 URLs needed to complete the upload.

I can take on this work if you'd like, since we need it as well?

@mifi
Copy link
Contributor

mifi commented Sep 14, 2021

As far as I can tell prepareUploadParts is just a GET that calls a pure function. It doesn't change any state, it just creates cryptographically signed URLs. So I don't see any reason why it cannot be retryable 👍

@Murderlon
Copy link
Member

Alright! @martin-brennan if you would like to pick this up that would be great as you're most familiar :)

@Murderlon
Copy link
Member

This issue may or may not be related to the code you are touching: #3191

@martin-brennan
Copy link
Contributor

Great thanks @Murderlon, I will start work on this soonish.

@martin-brennan
Copy link
Contributor

@Murderlon I am actually going on holiday for 3 weeks the week after next and I am not sure I will get to this before I go. Is it okay if I pick this up on the week of the 18th of October or is this more urgent?

@Murderlon
Copy link
Member

@Murderlon I am actually going on holiday for 3 weeks the week after next and I am not sure I will get to this before I go. Is it okay if I pick this up on the week of the 18th of October or is this more urgent?

Thanks again for showing interest in contributing to this. I'll discuss this with the team on Monday and let you know if we rather pick this up ourselves sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AWS S3 Plugin that handles uploads to Amazon AWS S3 Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants