-
-
Notifications
You must be signed in to change notification settings - Fork 365
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
Support retry delay with jitter? #509
Comments
Yes, I'm open to all of this. 👍 |
@rileytomasek Only number 2 of these requests have been done. |
@sindresorhus can’t number 1 be accomplished by adding jitter to the delay function? I’m not familiar with the beforeRetry hook to know if you could do it there, but is it required if you have 1 and 2? |
It can yes, but I think it could still be useful to have it built-in. |
Any thoughts on how to do We could allow |
It could potentially apply to the next retry instead. You don't really need a jitter for the first retry. Alternatively, skip this and only do a jitter function. Something like: retry: {
delay: 1000,
jitter: delay => delay * (0.8 + Math.random() * 0.4)
} |
Thanks for creating ky! I used it extensively in my libraries and projects.
I'm running into an issue where I'm making a lot of calls in parallel to an API with a rate limit and the 429 retries are all happening at the same time and triggering the rate limit again. I've tested with a forked version of Ky and adding a random jitter to the retry time makes things much better.
I'm happy to contribute a PR, but wanted to see which, if any, solutions you are open to:
BACKOFF_FACTOR * (2 ** (this._retryCount - 1)) * 1000
beforeRetry
hook?For more context, what I tested was calculating a jitter factor like
JITTER_FACTOR * randomBetween(1, -1)
and then multiplying the existing retry time by that. It could be configured so that the default value is 1, so there is no change to existing retry time calculation.The text was updated successfully, but these errors were encountered: