Skip to content
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

Promtail: Add configuration to drop batches when rate limited by Loki #7973

Merged
merged 5 commits into from
Jan 11, 2023

Conversation

chodges15
Copy link
Contributor

@chodges15 chodges15 commented Dec 19, 2022

What this PR does / why we need it:
This change will allow for more deterministic behavior by multitenant Promtail instances where one or more tenants are being heavily throttled due to limits exceeded within Loki. Specifically, it will reduce the performance impact of having exponential backoff cause HOL blocking for rate limit response codes while other tenants are ready to send batches.

Which issue(s) this PR fixes:
Fixes #7972

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

@chodges15 chodges15 requested a review from a team as a code owner December 19, 2022 18:52
@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Dec 19, 2022
@chodges15 chodges15 force-pushed the promtailRateLimitDrop branch from ee9cd84 to 7be2f5a Compare December 19, 2022 18:54
@chodges15
Copy link
Contributor Author

I realized when looking at the metrics with this change that I was not able to distinguish between drops due to rate limiting versus other reasons like stream limiting or exhausting all retries. Use of a reason label for drops would allow alerts to differentiate.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Jan 4, 2023
@chodges15 chodges15 force-pushed the promtailRateLimitDrop branch from 2abbef9 to 8333b68 Compare January 4, 2023 22:16
@chodges15 chodges15 force-pushed the promtailRateLimitDrop branch from 8333b68 to f088e7d Compare January 9, 2023 22:41
@CLAassistant
Copy link

CLAassistant commented Jan 11, 2023

CLA assistant check
All committers have signed the CLA.

@chodges15 chodges15 force-pushed the promtailRateLimitDrop branch from 3812654 to 23bb42a Compare January 11, 2023 16:27
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-target-branch/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@chodges15
Copy link
Contributor Author

Enabling this option has eliminated drops we were seeing with Loki canary when using it with a shared Promtail alongside a heavily rate-limited tenant. cc @MichelHollands @chaudum @DylanGuedes for review/comment.

Copy link
Contributor

@MasslessParticle MasslessParticle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the contribution!

@MasslessParticle MasslessParticle merged commit eb6ba61 into grafana:main Jan 11, 2023
@chodges15 chodges15 deleted the promtailRateLimitDrop branch January 11, 2023 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Promtail option to drop rate limited batches
4 participants