Skip to content

fix: skip streams over limits in dry-run mode - #17114

Merged
grobinson-grafana merged 3 commits into
mainfrom
grobinson/skip-streams-over-limits-in-dry-run-mode
Apr 15, 2025
Merged

fix: skip streams over limits in dry-run mode#17114
grobinson-grafana merged 3 commits into
mainfrom
grobinson/skip-streams-over-limits-in-dry-run-mode

Conversation

@grobinson-grafana

@grobinson-grafana grobinson-grafana commented Apr 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

This pull request fixes a bug when distributors were running with IngestLimitsDryRunEnabled where streams that were over the max stream limits would still be written to the metadata topic, meaning those streams would be included in the existing streams rather than outside the max stream limit.

This is stacked on top of #17124.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR
@grobinson-grafana
grobinson-grafana requested a review from a team as a code owner April 10, 2025 20:23
grobinson-grafana
grobinson-grafana commented Apr 10, 2025
View reviewed changes
Comment thread pkg/distributor/distributor.go Outdated

@grobinson-grafana grobinson-grafana Apr 10, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This tells us if it is an httpgrpc error or not.
@grobinson-grafana
grobinson-grafana force-pushed the grobinson/skip-streams-over-limits-in-dry-run-mode branch from b5073b2 to f4ddd20 Compare April 10, 2025 20:48
@grobinson-grafana
grobinson-grafana marked this pull request as draft April 10, 2025 21:18
@grobinson-grafana
grobinson-grafana force-pushed the grobinson/skip-streams-over-limits-in-dry-run-mode branch 3 times, most recently from 3e92b42 to 1a62b64 Compare April 11, 2025 13:34
@grobinson-grafana
grobinson-grafana marked this pull request as ready for review April 11, 2025 13:58
@grobinson-grafana
grobinson-grafana marked this pull request as draft April 11, 2025 14:41
@grobinson-grafana
grobinson-grafana force-pushed the grobinson/skip-streams-over-limits-in-dry-run-mode branch 2 times, most recently from 047978b to cba8c04 Compare April 14, 2025 14:22
@pull-request-size pull-request-size Bot added size/M and removed size/L labels Apr 14, 2025
@grobinson-grafana
grobinson-grafana force-pushed the grobinson/skip-streams-over-limits-in-dry-run-mode branch from cba8c04 to a897f0d Compare April 14, 2025 14:22
@grobinson-grafana
grobinson-grafana marked this pull request as ready for review April 14, 2025 14:23
@grobinson-grafana
grobinson-grafana force-pushed the grobinson/skip-streams-over-limits-in-dry-run-mode branch from f8b91ed to df10a75 Compare April 15, 2025 11:38
@grobinson-grafana
grobinson-grafana force-pushed the grobinson/skip-streams-over-limits-in-dry-run-mode branch from df10a75 to 3ebdc2f Compare April 15, 2025 11:39
@pull-request-size pull-request-size Bot added size/L and removed size/M labels Apr 15, 2025
periklis
periklis reviewed Apr 15, 2025
View reviewed changes
Comment thread pkg/distributor/distributor_test.go Outdated
Comment on lines 2831 to 2832

@periklis periklis Apr 15, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit. Do we need to save the mock records in the mock producer for other tests? I guess a counter would be sufficient right?
@grobinson-grafana
grobinson-grafana force-pushed the grobinson/skip-streams-over-limits-in-dry-run-mode branch from f0d5942 to 894d03e Compare April 15, 2025 13:48
@grobinson-grafana
grobinson-grafana merged commit 2cde9b1 into main Apr 15, 2025
@grobinson-grafana
grobinson-grafana deleted the grobinson/skip-streams-over-limits-in-dry-run-mode branch April 15, 2025 13:55
grobinson-grafana added a commit that referenced this pull request Apr 15, 2025
@grobinson-grafana
fix: skip streams over limits in dry-run mode (#17114)
5c9a6db
chaudum pushed a commit that referenced this pull request Apr 15, 2025
@grobinson-grafana
chore: backport ingest-limits fixes to k250 (#17237)
05edd28 
Contains backports of these commits:

```
2cde9b1 fix: skip streams over limits in dry-run mode (#17114)
805125c fix: fix a bug where limits were incorrect after #16937 (#17224)
d106042 fix: fix bug where expectedStreamUsageRequest was not used (#17223)
69aeda1 feat: add tests for enforcing limits in distributors (#17124)
```
This was referenced Jun 24, 2025
Closed
Closed
This was referenced Jul 7, 2025
Closed
Closed
This was referenced Jul 21, 2025
Closed
Closed
felix0102 pushed a commit to felix0102/loki that referenced this pull request Aug 2, 2026
@grobinson-grafana
fix: skip streams over limits in dry-run mode (grafana#17114)
4c9f416
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L

2 participants

@grobinson-grafana @periklis