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

fix cortex_compactor_remaining_planned_compactions not set after plan generation #4772

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

roystchiang
Copy link
Contributor

@roystchiang roystchiang commented Jul 5, 2022

What this PR does:
cortex_compactor_remaining_planned_compactions is not getting set as plans are generated. This PR fixes the issue

Which issue(s) this PR fixes:
Fixes #

Checklist

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

@roystchiang roystchiang force-pushed the fix-metrics branch 3 times, most recently from 51b7743 to 5c0f7e6 Compare July 6, 2022 00:14
@alvinlin123
Copy link
Contributor

@roystchiang can you please explain why

cortex_compactor_remaining_planned_compactions is not getting set as plans are generated.

is an issue?

@roystchiang
Copy link
Contributor Author

roystchiang commented Jul 6, 2022

cortex_compactor_remaining_planned_compactions gives us visibility into whether the compactor is lagging behind.

if users are setting HPA for compactor based on this metric, it will not scale out properly as needed.

@roystchiang roystchiang force-pushed the fix-metrics branch 2 times, most recently from a088771 to 8f8d2fe Compare July 6, 2022 16:36
@alanprot
Copy link
Member

alanprot commented Jul 6, 2022

Nice Catch!

@alvinlin123
Copy link
Contributor

@roystchiang can you merge this into release-1.13 branch instead? I will work on merging it back to main branch later - this is part of the release process

@roystchiang roystchiang changed the base branch from master to release-1.13 July 6, 2022 17:39
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 6, 2022
@roystchiang roystchiang changed the base branch from release-1.13 to master July 6, 2022 17:40
@pull-request-size pull-request-size bot added size/M and removed size/L labels Jul 6, 2022
Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

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

I like when we have more tests than code changes 🥳

@roystchiang roystchiang changed the base branch from master to release-1.3 July 6, 2022 18:30
@roystchiang roystchiang changed the base branch from release-1.3 to release-1.13 July 6, 2022 18:33
@pull-request-size pull-request-size bot added size/M and removed size/L labels Jul 6, 2022
CHANGELOG.md Outdated
@@ -47,6 +47,7 @@
* [BUGFIX] Memberlist: Avoid clock skew by limiting the timestamp accepted on gossip. #4750
* [BUGFIX] Compactor: skip compaction if there is only 1 block available for shuffle-sharding compactor. #4756
* [BUGFIX] Compactor: Fixes #4770 - an edge case in compactor with shulffle sharding where compaction stops when a tenant stops ingesting samples. #4771
* [BUGFIX] Compactor: fix cortex_compactor_remaining_planned_compactions not set after plan generation. #4772
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice if we make it clear it's only for shuffle sharding like the above bug fix :-)

@alvinlin123 alvinlin123 merged commit 3338c0c into cortexproject:release-1.13 Jul 6, 2022
alvinlin123 added a commit that referenced this pull request Jul 6, 2022
This commit essentially pulls in #4772 from 1.13 release branch.
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.

4 participants