Skip to content

Conversation

tianon
Copy link
Member

@tianon tianon commented May 22, 2024

This caps total attempts of any given request to at most 3x 503 responses (in other words, the third 503 will "bubble up" as-is). The intent there is that if there is a serious issue with Hub, we don't want to continue retrying aggressively as that would likely contribute to the outage persisting longer.

(inspired by the recurring random 503s we keep getting that resolve on a retry)

This caps total attempts of any given request to at most 3x 503 responses (in other words, the third 503 will "bubble up" as-is).  The intent there is that if there *is* a serious issue with Hub, we don't want to continue retrying aggressively as that would likely contribute to the outage persisting longer.
@tianon tianon requested a review from yosifkit as a code owner May 22, 2024 18:39
@tianon
Copy link
Member Author

tianon commented May 22, 2024

Hmmm, maybe we should also consider checking the Retry-After header (which is valid for both 503 and 429; https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After). 🤔

@tianon
Copy link
Member Author

tianon commented May 22, 2024

(I have no idea whether Hub is sending Retry-After with these though -- error reporting for these errors in almost all tools rarely includes the headers for a lot of good reasons 🙈)

Copy link

@whalelines whalelines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(inspired by the recurring random 503s we keep getting that resolve on a retry)

So we want to retry, but not too many times?

maybe we should also consider checking the Retry-After header

Probably worth checking, after this is merged.

@tianon
Copy link
Member Author

tianon commented May 22, 2024

Yeah, retrying one request is lighter weight than restarting the whole job, and should be enough for spurious 503s (and the explicit limit helps protect Hub if they aren't spurious 👍) 👀

@yosifkit yosifkit merged commit 744a76d into docker-library:main May 28, 2024
@yosifkit yosifkit deleted the retry503 branch May 28, 2024 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants