Retry in all error cases but a few #1580
Merged
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
copumpkin
force-pushed
the
retry-after-timeout
branch
from
63d5f50
to
6c2b669
Compare
|
Maybe it would be better to set the default value for |
|
@edolstra I considered that, but I don't think the retry count is fixed or known ahead of time, right? Different download invocations can pass in different values for it I think. |
It was getting too much like whac-a-mole listing all the retriable error conditions, so we now retry by default and list the cases where retrying is almost certainly hopeless.
copumpkin
force-pushed
the
retry-after-timeout
branch
from
6c2b669
to
476493d
Compare
copumpkin
changed the title
|
@edolstra I updated it as discussed on IRC. Let me know what you think about the timeout concern. |
|
@edolstra any thoughts on this? I'm happy to make further changes if you see fit, but ultimately just want to stop trying to catch every little error that flaky servers throw my way. |
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.
I could make this optional but it doesn't seem like too weird of a thing to enable in general.As we discussed on IRC
This is what I'm currently running into in #1573, even though it no longer causes deadlocks. I'm honestly not sure if it was what was causing deadlocks before, but since updating Nix I'm now getting timeout errors with the same frequency as I was getting deadlocks before.
I'm unsure what to do about timeouts. The behavior in this PR will retry on curl connect timeouts (which happens to be the behavior I want, since I'm getting occasional timeouts from dud S3 backend servers), but that doesn't seem like something everyone would want.
cc @edolstra