-
Notifications
You must be signed in to change notification settings - Fork 565
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
lib-storage Upload does not retry if one part fails. #2311
Comments
@alvaromat you bring up a really good point. I agree that this is a much better experience. However - this isn't how upload is designed today so I'll change this to a feature request. As you point out, this isn't a difficult change, and it could also be a good first PR for new contributors! In the mean time I'd love to open it up for public discussion. I'm looking to get a sense of what the retry would look like. No right or wrong answers, just looking to get a sense of where people are at in their thinking.
|
Assigning @ajredniwja to support the discussion |
The simplest version would just retry multiple times with a backoff. I'd set a max number of configurable retries per part, as multiple parallel parts can fail at the same time. It's also possible to get advantage of the offline/online events. This could be useful for long offline events that could happen in low connection environments, like getting into the subway. I'd apply this retry logic to only parts uploads, not in the initial request where configuration issues are more likely to happen. |
Retry uploads of failed multiparts apparently was a feature of AWS SDK v2. Now with lib-storage it doesn't have it, so it's a regression. Also, here's what AWS recommends:
There are 500 errors from time to time when uploading multi-part files and currently, none of JS AWS maintained libraries handle them properly. Quick search shows how common and frustrating those are with no solution offered: https://www.google.com/search?q=s3+multipart+upload+error+500 @alvaromat I would argue not to use navigator.onLine, as then it would leave ability to use this code in different environments, not just browser (next.js, react-native(web), node, etc.) All that's needed to retry is just retry failed parts for maxRetryAttempts with a delay of a second or so.. Since it's retrying individual parts to recover from an occasional error 500, or to survive a short drop of connectivity, the exponential backoff is really not required. |
If I understand it correctly, the Upload module is designed so that (with the default behavior Update: If I use |
@mifi This has also been my experience of using the
I agree with @mifi that the default should be If this is going to be the behaviour going forward a rename of the |
Has there been any update on this - is there a way to actually manually handle the dropped parts? |
hello, Is there any progress on this feature? |
created a PR #4414 |
@mifi hey man,can u show me the code? I don't know how to write, thank you very much! |
not sure which code? just set |
ok, thank you @mifi |
@RanVaknin Is there any way to get this assigned a higher priority? While there is a workaround available, it's not obvious that it needs to be used until something fails in a potentially catastrophic way, and teams spend potentially hours investigating. The documentation pointed to above suggests to a user reading it that the default behaviour is the safe one with retries. |
after set "leavePartsOnError: true" in @aws-sdk/lib-storage. how to set retry ? I want a part retry upload 3 times when upload fail. hope you anwser @mifi |
I think it will auto retry, see #2311 (comment) |
when i upload video using @aws-sdk/lib-storage and set "leavePartsOnError: true". During the upload process, i set network unconnect. as follow image you see, only 1 error " partNumber=7 upload fail " showing on network. if client-s3 will retry, may be i can see more than 1 error from network。hope you anwser@mifi |
@0zcl hey man, have you solved the problem? |
no, I can not set retry times. Although does not affect the use |
I also set With @RanVaknin Is it possible to either retry uploading the failed part or the whole upload? |
The Because You do not need to modify the retry configuration because the default will retry, but here is some information:
Once #6112 is merged, the only reason to set |
Since this is addressed in the PR I'm going to close this. Since closed issues are not as closely monitored, if you have any additional concerns about this, please create a new issue. Thanks, |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
Describe the bug
I'm using Upload from
lib-storage
to get advantage of multipart uploads regarding large files. I would expect it to retry in case any of failure uploading any part. Imagine we have a 5TB file that is splitted in 5MB parts. That would result in many, many parts, so the chance of individual failure is high.There is the
leavePartsOnError
option. Setting it to true interrupts the whole upload if any part fails.Your environment
SDK version number
@aws-sdk/lib-storage@3.13.1
Is the issue in the browser/Node.js/ReactNative?
Browser
Details of the browser/Node.js/ReactNative version
Steps to reproduce
Upload a file using Upload, exactly like in the example:
Observed behavior
If there are no errors with any part, the upload goes right.
If there is an error in one or more parts, that part is skipped and the final file is built without those parts. This results in a corrupt file. This can be reproduced stopping the connection (turning WiFi off) or using browser DevTools to stop the connection.
Expected behavior
If a single part fails to upload, what is likely to happen with big files in mobile environments, just retry to send that part.
This could be done for example here:
aws-sdk-js-v3/lib/lib-storage/src/Upload.ts
Lines 124 to 130 in 28008c2
Do not send the final build command if any part is missing:
aws-sdk-js-v3/lib/lib-storage/src/Upload.ts
Lines 152 to 162 in 28008c2
The text was updated successfully, but these errors were encountered: