-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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(kafka): Remove rate limits for kafka ingestion #14460
Conversation
pkg/ingester/ingester.go
Outdated
@@ -404,18 +404,21 @@ func New(cfg Config, clientConfig client.Config, store Store, limits Limits, con | |||
i.SetExtractorWrapper(i.cfg.SampleExtractorWrapper) | |||
} | |||
|
|||
var limiterStrategy limiterRingStrategy | |||
var streamCountLimiterStrategy limiterRingStrategy |
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.
This is a massive nit, but just two suggestions to make the names a little shorter:
var streamCountLimiterStrategy limiterRingStrategy | |
var streamCountLimiter limiterRingStrategy |
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.
I like this, thanks!
pkg/ingester/ingester.go
Outdated
var ownedStreamsStrategy ownershipStrategy | ||
var streamRateLimitStrategy RateLimiterStrategy |
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.
var streamRateLimitStrategy RateLimiterStrategy | |
var streamRateLimiter RateLimiterStrategy |
What this PR does / why we need it:
When ingesting from Kafka, especially replaying during startup, we often hit stream rate limits. This PR enables two implementations for the stream rate limiter:
Which issue(s) this PR fixes:
Fixes https://github.com/grafana/loki-private/issues/1145
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
deprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR