-
Notifications
You must be signed in to change notification settings - Fork 3.7k
feat: enforce limits in distributors #16705
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: enforce limits in distributors #16705
Conversation
428f318
to
187a246
Compare
💻 Deploy preview deleted. |
187a246
to
78ae59e
Compare
tenantConfigs *runtime.TenantConfigs | ||
tenantsRetention *retention.TenantsRetention | ||
ingestersRing ring.ReadRing | ||
validator *Validator | ||
pool *ring_client.Pool | ||
ingesterClients *ring_client.Pool |
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 renamed this field as now we have a number of *ring_client.Pool
.
if factory == nil { | ||
factory = ring_client.PoolAddrFunc(func(addr string) (ring_client.PoolClient, error) { | ||
return client.New(clientCfg, addr) | ||
ingesterClientFactory := cfg.factory |
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.
Same here.
d.kafkaAppends.WithLabelValues(fmt.Sprintf("partition_%d", partitionID), "fail").Inc() | ||
d.kafkaAppends.WithLabelValues(fmt.Sprintf("partition_%d", streamPartitionID), "fail").Inc() | ||
finalErr = result.Err | ||
} else { | ||
d.kafkaAppends.WithLabelValues(fmt.Sprintf("partition_%d", partitionID), "success").Inc() | ||
d.kafkaAppends.WithLabelValues(fmt.Sprintf("partition_%d", streamPartitionID), "success").Inc() |
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.
nit. Count the metadata appends in this or a separate metric, wdyt?
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'm not sure if we know which record failed here, it's something I need to look into.
9f34f19
to
4a7d273
Compare
What this PR does / why we need it:
This pull request enforces stream limits in distributors.
Which issue(s) this PR fixes:
Fixes #
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