Retry certain errors more times #543
Conversation
|
Mind just version bumping in here? |
|
Bumped! |
|
bors merge |
|
🔒 Permission denied Existing reviewers: click here to make ericphanson a reviewer |
|
bors r+ |
543: Retry 500 errors more times r=mattBrzezinski a=ericphanson Closes #542 Co-authored-by: Eric Hanson <[email protected]>
|
Added you as a maintainer for the repo :) |
|
Build failed: |
|
Ahh, we are relying on exceeding the number of retries in some tests, I think. I'll have to update them. |
|
I think maybe that was it; let's see if it goes through: bors r+ |
|
🔒 Permission denied Existing reviewers: click here to make ericphanson a reviewer |
|
bors r+ |
543: Retry 500 errors more times r=mattBrzezinski a=ericphanson Closes #542 Co-authored-by: Eric Hanson <[email protected]>
|
Updated you to an admin, should fix your perms 🤷 |
|
Build failed: |
|
Hm, it says the build was cancelled; not really sure how that can happen |
|
Going to just try again bors r+ |
543: Retry 500 errors more times r=ericphanson a=ericphanson Closes #542 Co-authored-by: Eric Hanson <[email protected]>
|
Build failed: |
|
Cancelled again 🤔 |
ericphanson
changed the title
|
bors try |
tryBuild failed: |
|
Oh! I know what's happening. It is reaching the 30 minute timeout and being cancelled. So it is likely a problem in the code, not a bors problem. |
|
I wonder if Retry.jl's exponential delays just become too huge 😂 edit: julia> sleep(x::Float64) = println("Would sleep for $x")
sleep (generic function with 1 method)
julia> @repeat n try; error()
catch
@delay_retry if true end
end
Would sleep for 0.052740343728706396
Would sleep for 0.5430940435531718
Would sleep for 5.6191153198632655
Would sleep for 43.07752844528991
Would sleep for 556.7462558824232
Would sleep for 4004.774901154357
Would sleep for 54213.91981134377
Would sleep for 502438.01830805355
Would sleep for 5.625640217525721e6
|
|
Closing in favor of #550 |
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]>
Closes #542
Closes #541