Increase some retries from 4 to 10 #550
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 12, 2022
|
bors try |
tryBuild failed: |
|
Failed on MacOS only, with a no such bucket error. Strange! |
Might just be an error with timings. bors try |
|
Maybe one day I'll have time to think about these timing issues, or I just continue to kick the bucket and bors down the road.... works though! |
|
I think #553 is a better approach- if we have a bad downloader, it might not matter how many retries we do |
bors bot
added a commit
that referenced
this pull request
May 25, 2022
548: translate Retry to `Base.retry` r=ericphanson a=ericphanson Suggested in #542 (comment) I'm not sure we should merge this as-is, because before we could do immediate retries OR delay retries, and now we can only do delay retries. ~~I'm also not 100% sure the delay amounts are the same.~~ The functional approach is nicer than the macro approach though in my opinion, ~~since the retries don't need to be literals~~. So we could let folks request more retries on a per-request basis, or even put it in the Backend struct. edit 1: Retry.jl expands to a for-loop, so non-literals are OK. I thought it unrolled the loop. edit 2: the delay amounts are definitely not the same, but that's a good thing- now we copy the upstream recommendation from https://docs.aws.amazon.com/sdkref/latest/guide/feature-retry-behavior.html. In particular, putting a cap of 20s means we can increase the retries further without danger, xref #550 (we can't do that with Retry.jl since the retries are hard-coded with no cap and get too big- ref #543 (comment)). --- 12 May update: I'm now in favor of merging this, because I want to get in #550. The first few retries are pretty short, e.g. ```julia julia> collect(AWS.AWSExponentialBackoff(; max_attempts=10)) 9-element Vector{Any}: 0.3365311959885935 1.2370247946870188 2.925043257220695 12.165245886682294 20.0 20.0 5.5969335490336505 20.0 20.0 ``` So I think having a short delay instead of an immediate retry in a few cases is probably OK. Also, the AWS docs don't talk about non-delayed retries, they seem to only do delayed retries. Co-authored-by: Eric Hanson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Redo of #543 on top of #548 since then we have more control over the retry timers.
Closes #542
Closes #541