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

Update seriesFlushReason when flushing, not enqueing. #2802

Merged
merged 3 commits into from
Jun 29, 2020
Merged

Update seriesFlushReason when flushing, not enqueing. #2802

merged 3 commits into from
Jun 29, 2020

Conversation

pstibrany
Copy link
Contributor

What this PR does: This PR changes cortex_ingester_flush_reasons. It is renamed to cortex_ingester_series_flush_reasons since it counts series and not chunks, and it is now updated when flushing, not enqueing. This fixes problem described in issue #2287, when idle series could have been enqueued, but then updated before flushing.

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

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Peter Štibraný <[email protected]>
@pstibrany pstibrany mentioned this pull request Jun 26, 2020
3 tasks
Copy link
Contributor

@sandeepsukhani sandeepsukhani left a comment

Choose a reason for hiding this comment

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

I also agree with Marco about the metric name otherwise it LGTM.

Signed-off-by: Peter Štibraný <[email protected]>
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM!

@pstibrany pstibrany merged commit d16b681 into cortexproject:master Jun 29, 2020
@bboreham
Copy link
Contributor

This change makes the metric less useful.
If you have an enormous queue you want to know why now, not in two hours when it is flushed.

bboreham added a commit that referenced this pull request Jun 29, 2020
This reverts commit d16b681.

Make the metric useful again.

Signed-off-by: Bryan Boreham <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a counter for chunks removed from the flush queue but not flushed
4 participants