partitioner: report more accurate metrics #3769
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 commit fixes a partitioner metric and adds a new one The partitioner currently exposes 4 counters * `cortex_bucket_store_partitioner_requested_bytes_total` * `cortex_bucket_store_partitioner_requested_ranges_total` * `cortex_bucket_store_partitioner_expanded_bytes_total` * `cortex_bucket_store_partitioner_expanded_ranges_total` Currently, `cortex_bucket_store_partitioner_requested_bytes_total` exposes the total sizes of all ranges independant of each other. This represents their total size without merging any overlapping sizes. This is not very useful information. This PR changes that metric to report the number of bytes that would be fetched if the gap that the partitioner fills is 0. This way you can use `cortex_bucket_store_partitioner_requested_bytes_total` in combination with `cortex_bucket_store_partitioner_expanded_bytes_total` to figure out how much more data the partitioner is fetching that wasn't in the original ranges. This PR also adds `cortex_bucket_store_partitioner_extended_ranges_total` This metric tracks the number of ranges that were merged together due to the gap that the partitioner fills. This metric will help determine whether the partitioner is mergning too many ranges and overfetching small gaps very often. Or whether it is overfetching a single very big range. This will help in tuning the default gap for the partitioner (currently at 512KB) Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
pracucci
reviewed
Dec 19, 2022
pracucci
reviewed
Dec 19, 2022
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
…es_total Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
pracucci
reviewed
Dec 19, 2022
pkg/storegateway/partitioner.go
Outdated
| @@ -42,38 +43,41 @@ func newGapBasedPartitioner(maxGapBytes uint64, reg prometheus.Registerer) *gapB | |||
| Name: "cortex_bucket_store_partitioner_expanded_ranges_total", | |||
| Help: "Total number of byte ranges returned by the partitioner after they've been combined together to reduce the number of bucket API calls.", | |||
| }), | |||
| extendedRanges: promauto.With(reg).NewCounter(prometheus.CounterOpts{ | |||
| Name: "cortex_bucket_store_partitioner_extended_ranges_total", | |||
| Help: "Total number of byte ranges that were not overlapping but were joined because they were closer than the configured maximum gap.", | |||
There was a problem hiding this comment.
[nit] Is my edit correct?
Suggested change
| Help: "Total number of byte ranges that were not overlapping but were joined because they were closer than the configured maximum gap.", | |
| Help: "Total number of byte ranges that were not adjacent or overlapping but were joined because they were closer than the configured maximum gap.", |
There was a problem hiding this comment.
you are correct 👍
There was a problem hiding this comment.
given [0, 10) and [10, 20) and gap size 0 the partitioner will merge these two
given [0, 10) and [11, 20) and gap size 1 the partitioner will merge these two
given [0, 10) and [12, 20) and gap size 1 the partitioner will not merge these two
i added a test case for the last two cases
pkg/storegateway/partitioner.go
Outdated
| if p.End >= s { | ||
| // The start of the next range overlaps with the current range's end, so we can merge them. | ||
| // We count the extra bytes between the current range's end and the next one's end - that's what's been requested. | ||
| stats.requestedRangesTotal += e - p.End |
There was a problem hiding this comment.
Isn't there a bug in the case of the ranges [10, 20], [15, 18]? I haven't tried to execute the code with this case, but I think we subtract for the 2 range instead of "adding zero".
There was a problem hiding this comment.
nice catch. I will add a test case for this
There was a problem hiding this comment.
added one in f646b87
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
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 commit fixes a partitioner metric and adds a new one
The partitioner currently exposes 4 counters
cortex_bucket_store_partitioner_requested_bytes_totalcortex_bucket_store_partitioner_requested_ranges_totalcortex_bucket_store_partitioner_expanded_bytes_totalcortex_bucket_store_partitioner_expanded_ranges_totalCurrently,
cortex_bucket_store_partitioner_requested_bytes_totalexposes the total sizes of all ranges independant of each other. This
represents their total size without merging any overlapping sizes.
This is not very useful information. This PR changes that metric to
report the number of bytes that would be fetched if the gap that the
partitioner fills is 0. This way you can use
cortex_bucket_store_partitioner_requested_bytes_totalin combinationwith
cortex_bucket_store_partitioner_expanded_bytes_totalto figureout how much more data the partitioner is fetching that wasn't in the
original ranges.
This PR also adds
cortex_bucket_store_partitioner_extended_ranges_totalThis metric tracks the number of ranges that were merged together due to
the gap that the partitioner fills. This metric will help determine
whether the partitioner is mergning too many ranges and overfetching
small gaps very often. Or whether it is overfetching a single very big
range. This will help in tuning the default gap for the partitioner
(currently at 512KB)
Signed-off-by: Dimitar Dimitrov dimitar.dimitrov@grafana.com