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

Fix issue with rate limit guard waiting sub ms #204

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Conversation

Jonnern
Copy link
Sponsor Contributor

@Jonnern Jonnern commented Jun 18, 2024

When the rate limit is hit and a request is delayed, it loops extremely fast without delay because of Task.Wait(TimeSpan) is not able to wait on a shorter timespan than 1 ms (ref Task.cs source)

Delaying call to 0/public/Ticker by 00:00:00.0002703 because of ratelimit guard RateLimitGuard; Limit of 1 per 00:00:01, Request weight: 1, Current: 1, Limit: 1, requests now being limited: 1
Delaying call to 0/public/Ticker by 00:00:00.0002376 because of ratelimit guard RateLimitGuard; Limit of 1 per 00:00:01, Request weight: 1, Current: 1, Limit: 1, requests now being limited: 1
Delaying call to 0/public/Ticker by 00:00:00.0002323 because of ratelimit guard RateLimitGuard; Limit of 1 per 00:00:01, Request weight: 1, Current: 1, Limit: 1, requests now being limited: 1
...

@JKorf
Copy link
Owner

JKorf commented Jun 18, 2024

Thanks for the catch. Just wondering, but wouldn't it be safer to instead wait for a minimum of 1 milliseconds?

@Jonnern
Copy link
Sponsor Contributor Author

Jonnern commented Jun 18, 2024

I agree. I updated the fix

@Jonnern
Copy link
Sponsor Contributor Author

Jonnern commented Jun 18, 2024

Maybe it's better to use Task.Delay(int32) and round up number of ms. Because using wait with TimeSpan will leave < 1ms more wait time which will trigger another loop of waiting

@JKorf
Copy link
Owner

JKorf commented Jun 18, 2024

Good point, I think that would make sense yes

@Jonnern
Copy link
Sponsor Contributor Author

Jonnern commented Jun 18, 2024

It should be good to go now. I force-pushed to squash my commits

@JKorf JKorf merged commit 1555f8d into JKorf:master Jun 21, 2024
1 check passed
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.

2 participants