Revert "metric: don't omit empty histogram buckets" #89532
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
This reverts commit 6f28649. Release note (<category, see below>): <what> <show> <why>
dhartunian
force-pushed
the
revert-histogram-changes
branch
from
262734a to
783b9ef
Compare
|
Yikes! Really sorry about that. This bug just keeps on giving! |
tbg
approved these changes
Oct 7, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 6f28649.
This change led to a ~4x increase in the size of the
_status/varsexport which caused memory issues in many deployments with Prometheus and OpenTelemetry collector instances. The benefits of full histogram outputs are not strong enough to pay the cost of significantly higher memory usage on these services that run alongside a cluster. On a minor version bump, they could cause instability in customer infra.Especially since we've overhauled the histograms on
masterand22.2, we're reverting this in the22.1branch. This "bug" was in place for years so the revert should not have unexpected negative effects.Release note (ops change): previously a change was made to how histogram buckets were output in the
/_status/varsendpoint that is scraped by Prometheus. This caused a large increase in the number of buckets that causes significantly higher memory consumption in scrapers such as Prometheus and the OpenTelemetry collector. This change is being reverted to reduce the memory pressure on those tools. The next major version of CRDB contains a full overhaul of our histogram outputs that removes this problem entirely.Reverts: #88331