-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: throttling #21
feat: throttling #21
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
==========================================
+ Coverage 76.31% 77.21% +0.89%
==========================================
Files 41 47 +6
Lines 2820 3155 +335
==========================================
+ Hits 2152 2436 +284
- Misses 563 594 +31
- Partials 105 125 +20
... and 3 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@@ -0,0 +1,75 @@ | |||
package resource |
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.
since we are moving this to commons, it would be wise to add support for overriding the docker image tag.
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.
Good idea, will add that 👍
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.
Done here.
|
||
const defaultMaxCASAttemptsLimit = 100 | ||
|
||
type gcra struct { |
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.
For this specific implementation we are effectively using 1+N stores:
- One
cachettl
which holds all different GCRARateLimiterCtxs. - Multiple (throttled) memstores, one for every instance of GCRARateLimiterCtx that we are creating.
It should be possible to use our own implementation of throttled.GCRAStoreCtx
e.g. by leveraging cachettl
and avoid using throttled's own memstore. Or since each GCRARateLimiterCtx
we are creating is handling a single key, we can make a store that always holds a singular item :)
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.
@atzoum what do you think of this approach?
The benchmark gives:
goos: linux
goarch: amd64
pkg: github.com/rudderlabs/rudder-go-kit/throttling
cpu: 12th Gen Intel(R) Core(TM) i9-12900K
BenchmarkInMemoryGCRA
BenchmarkInMemoryGCRA/one_unlimited_store_per_throttler
BenchmarkInMemoryGCRA/one_unlimited_store_per_throttler/single_key
BenchmarkInMemoryGCRA/one_unlimited_store_per_throttler/single_key-24 8378788 141.1 ns/op 0 B/op 0 allocs/op
BenchmarkInMemoryGCRA/one_unlimited_store_per_throttler/multiple_keys
BenchmarkInMemoryGCRA/one_unlimited_store_per_throttler/multiple_keys-24 6042226 197.6 ns/op 46 B/op 0 allocs/op
BenchmarkInMemoryGCRA/one_unlimited_store_for_all_throttlers
BenchmarkInMemoryGCRA/one_unlimited_store_for_all_throttlers/single_key
BenchmarkInMemoryGCRA/one_unlimited_store_for_all_throttlers/single_key-24 8469865 141.0 ns/op 0 B/op 0 allocs/op
BenchmarkInMemoryGCRA/one_unlimited_store_for_all_throttlers/multiple_keys
BenchmarkInMemoryGCRA/one_unlimited_store_for_all_throttlers/multiple_keys-24 5978118 223.6 ns/op 24 B/op 0 allocs/op
BenchmarkInMemoryGCRA/one_single_key_store_per_throttler
BenchmarkInMemoryGCRA/one_single_key_store_per_throttler/single_key
BenchmarkInMemoryGCRA/one_single_key_store_per_throttler/single_key-24 6954384 169.8 ns/op 16 B/op 1 allocs/op
BenchmarkInMemoryGCRA/one_single_key_store_per_throttler/multiple_keys
BenchmarkInMemoryGCRA/one_single_key_store_per_throttler/multiple_keys-24 4416854 285.8 ns/op 102 B/op 3 allocs/op
BenchmarkInMemoryGCRA/custom_store_per_throttler
BenchmarkInMemoryGCRA/custom_store_per_throttler/single_key
BenchmarkInMemoryGCRA/custom_store_per_throttler/single_key-24 9134334 130.0 ns/op 0 B/op 0 allocs/op
BenchmarkInMemoryGCRA/custom_store_per_throttler/multiple_keys
BenchmarkInMemoryGCRA/custom_store_per_throttler/multiple_keys-24 7789651 161.9 ns/op 16 B/op 0 allocs/op
PASS
ok github.com/rudderlabs/rudder-go-kit/throttling 21.661s
The last one (i.e. custom_store_per_throttler
) is the new implementation.
Description
I think this library is generic enough to live in the kit. Please provide feedback.
Security