Skip to content
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

Consider replacing maxRetries flag with something else #241

Open
Martoon-00 opened this issue Dec 15, 2022 · 0 comments
Open

Consider replacing maxRetries flag with something else #241

Martoon-00 opened this issue Dec 15, 2022 · 0 comments

Comments

@Martoon-00
Copy link
Member

Martoon-00 commented Dec 15, 2022

Clarification and motivation

To me it looks like maxRetries is an inconvenient flag in practice and we better provide an alternative to it.

What's wrong with it?

  1. It does not scale well. As soon as the repository grows, given that sites usually allow X requests per timeframe, the user has to adjust the value of this flag manually to keep verification of all links possible.

    Worse than that, how soon the links verification starts getting 429 is not a constant value for a given single job, rather depends on various factors and effectively looks random. So once getting too many links, the user will start getting job failures only periodically, which is annoying.

  2. Say, we have to check a site that, being poked too severely, suggests retrying after 1 hour. What should I do if I don't want to wait for 1 hour? Set maxRetries = 0? That's impractical.

Given the second example, I think what we actually want to limit is not the number of retries, but rather the overall time we wait for retries. E.g. if I passed "1 minute" value, then first 429 with Retry-After: 30 is ok, a second 429 with Retry-After: 20 is still ok, but another Retry-After: 20 (or Retry-After: 300) is not ok and we should not retry here rather tell that we gave up on a given link.

Given the first example, we may want to provide not the overall number of attempts / overall time duration, rather a coefficient - number of attempts or time duration granted for every, say, 10 links that fall into retry. But I'm not sure here, this thing seems questionable. Further in this message I assume that we don't go with this option, but it's up to the discussion.

maxRetries flag probably is still worth preserving, for backward compatibility and because some users may find it more straightforward, but I think we should be ready to accept some maxRetriesWaitingDuration flag too (but not both simultaneously).

On maxTimeoutRetries flag added in #234, it seems like we want to leave only it for timeouts, no need to add an alternative for it.

Acceptance criteria

  • I can limit the overall retries duration.
  • The default config suggests this new option, mentioning in its description that there is a simpler maxRetries alternative.
  • We discussed and settled on which exactly alternative options are worth providing.
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

No branches or pull requests

1 participant