[v22.2.x] metrics: ensure consistent histogram upper bounds #9219
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
In order to publish histogram metrics, we convert from our internal HDR histogram representation, to Seastar's histogram type. Seastar expects the histograms of a given metric to always have the same number of buckets, with idential upper bounds. If that's not the case, it will throw whilst handling the request to fetch metrics. The visible behaviour for this is different depending on the Redpanda version where the issue occurs. For v22.2.x, the connection for the metrics request will be killed, and, therefore the metrics are truncated. For later versions, Redpanda crashes as the exception happens on a noexcept code-path. This commit ensures that the upper bounds of Seastar histograms generated from the same HDR histogram are always the same. Previously, the we used `hdr_iter_log_init.iterated_to` as the upper bound for the bucket. However, (due to a bug?) the C HDR histogram implementation, does not update this field under certain condititions. To fix this, we keep track of the value we've iterated to outside of the library's iterator. I noticed a couple other issues with the conversion logic and they're also fixed by this commit: * We used the `hdr_iter_log_init.iterated_to` for the upper bound of the Seastar histogram bucket. Internally, HDR histogram uses a representation similar to floating point numbers, which means that it can't actually represent every value in the range, and it treats spans(buckets) of values as equivalent. Therefore, the upper bound we reported was not correct. This has been fixed by reporting the highest equivalent value instead. * The smallest discernible value is a configurable knob of HDR histogram. When that's set, it doesn't make sense to iterate over the histogram from values smaller than that. We did previously, but it's been fixed by taking the maximum between the requested start value and the configured minimum discernible value. (cherry picked from commit bb55761)
jcsp
approved these changes
Mar 7, 2023
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.
Backport of PR #9192
Mostly clean backport. There was a conflict in
CMakeLists.txt, but the logic was clean.