-
Notifications
You must be signed in to change notification settings - Fork 26
feat: Individual Table and Client rate limit #1411
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
Conversation
scheduler/scheduler.go
Outdated
| logger zerolog.Logger | ||
| concurrency int | ||
| singleTableConcurrency sync.Map | ||
| singleTableMaxConcurrency int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just use int64 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, maybe use negative to disable the code?
erezrokah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few comments/questions mostly. The blocking one is on making this a non breaking change
🤖 I have created a release *beep* *boop* --- ## [4.21.0](v4.20.0...v4.21.0) (2023-12-13) ### Features * Individual Table and Client rate limit ([#1411](#1411)) ([4d13b18](4d13b18)) ### Bug Fixes * **deps:** Update module github.com/cloudquery/cloudquery-api-go to v1.6.2 ([#1413](#1413)) ([f5a0d47](f5a0d47)) * **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.14.4 ([#1408](#1408)) ([7544967](7544967)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
This pr introduces a rate semaphore that limits the number of simultaneous go routines that can be resolving the same table + client to just 10. Prior to this a single table could be using all of the available concurrency to resolve a single table. This alleviates the problem where nested tables get throttled because the load is not spread across all client/table pairs but is concentrated on a single client and table.
I will update this PR with a benchmark case once #1410 gets merged