-
Notifications
You must be signed in to change notification settings - Fork 58
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
Rate Limiting Not Working #99
Comments
Hi, thanks for reporting this. There is definitely room for improvement with the rate limiting strategies, and we have plans. The "sleep after request" behavior is not ideal, but in practice it is still useful. Because of the way rate-limiting is implemented in the API, we really can't know for sure if we are going to be rate-limited until we've made the request, so the "strategies" are necessarily about "avoidance" and could certainly hit edge cases. And we can't know about all other activity that may be hitting the API and consuming requests. For terraform, the "concurrent" strategy is probably best, with a "parallelism" number roughly matching the parallelism used for terraform. The strategy does try to "saturate" the rate-limiter, it doesn't leave any room for anything else. To start, I'd like to ask what you are setting If we rule out a bit more of a "buffer" being all that's needed, we can discuss next steps. |
Hello @jamesgoodhouse, is this issue solved? |
Closing due to inactivity. @jamesgoodhouse if you are still experiencing problems not addressed by the doc linked in the above comment, please re-open or file a ticket with NS1 support. |
We are currently making use of the
terraform-provider-ns1
to manage our users. The provider is configured to either set the RateLimitFunc toRateLimitStrategyConcurrent
orRateLimitStrategySleep
(see https://github.com/terraform-providers/terraform-provider-ns1/blob/master/ns1/config.go#L63-L67) for thens1-go
lib. Unfortunately, during large refreshes of users we are still receiving429 Rate limit exceeded
errors. After closer inspection of the code, I may not understand how rate limiting is implemented, but I don't believe it is working as expected.When looking at the
Do
method, I see that the request is first executed, and then the RateLimitFunc is executed. This seems backwards. If the client is configured to utilize theRateLimitStrategySleep
, the request will only sleep after it has been executed, making it essentially useless. I was thinking that maybe it was done this way because a retry would occur once the sleep happened, but I'm not seeing anywhere in the code where a retry might occur.Apologies if I'm completely mis-reading the code, but ultimately, we're still having errors occur due to rate limits which I feel should be working correctly with this lib.
The text was updated successfully, but these errors were encountered: