Fix max upper bound for last bucket of timer stats #3543
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
|
@Ghatage Thanks for your contribution, would you please help resolve the conflicts? Thanks a lot. |
Make sure the 99th percentile is reported as the actually recorded max (not as upper bucket bound Long.MAX_VALUE)
Ghatage
force-pushed
the
fix99percTimerStats
branch
from
760191c
to
bb6f12a
Compare
|
Hi @hangc0276 somehow I missed the comment, sorry for that. Fixing the conflicts and getting this PR ready to go. |
|
@hangc0276 can you PTAL? |
shoothzj
approved these changes
May 5, 2024
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.
Descriptions of the changes in this PR:
Motivation
Codahale metrics are used for dashboarding with Grafana and we must rely on these values to be accurate in order to measure the stability and performance of the service.
For Codahale's fast timer, when a value falls in the last bucket, the max is reported as the upper bound of the last bucket (Long.MAX_VALUE) which can lead to erroneous 99 percentile values.
Changes
We change the way we get the max value recorded if the bucket we're dealing with is the last bucket (for 99th percentile).
If we are in the last bucket, we return the actual max, otherwise we can return
getBucketBound()which should return the accurate calculated value.Also added a test.