Add querier metric for block source and compaction level #7112

jhalterman · 2024-01-13T03:11:58Z

What this PR does

Adds source and compaction level to the cortex_bucket_store_series_blocks_queried metric, which indicates the number of compacted blocks that were queried from store gateways.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

Tests updated.
Documentation added.
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
about-versioning.md updated with experimental features.

dimitarvdimitrov

this is great. I never knew this is is "knowable". It could serve as a reliable indicator for the compactor falling behind.

pkg/querier/block_streaming.go

pkg/querier/blocks_store_queryable.go

pkg/storage/tsdb/block/meta.go

pkg/storegateway/bucket.go

pkg/storage/tsdb/block/meta.go

pkg/querier/blocks_store_queryable.go

jhalterman · 2024-01-17T01:11:52Z

Updated the PR to move this metric to the store-gateway itself, and use a summary to be consistent with cortex_bucket_store_series_blocks_queried. There don't seem to be any related tests to update.

I'm on the fence about whether this metric should be about compacted or non-compacted blocks - input appreciated. My immediate use case is mostly interested in non-compacted blocks, but @dimitarvdimitrov pointed out that understanding compacted blocks could be useful as well. Either one can be derived from the other, along with cortex_bucket_store_series_blocks_queried.

pracucci

Thanks Jonathan for re-iterating on it. I left a couple of final comments. About tests to improve, I think you can look at the test using assertQueryStatsMetricsRecorded().

pkg/storage/tsdb/block/meta.go

dimitarvdimitrov · 2024-01-17T19:08:54Z

I'm on the fence about whether this metric should be about compacted or non-compacted blocks - input appreciated. My immediate use case is mostly interested in non-compacted blocks, but @dimitarvdimitrov pointed out that understanding compacted blocks could be useful as well.

The potential value that I see is to be able to tell when store-gateways start querying non-compcated 2-hour blocks. If the compactor falls behind on compacting 12-hour blocks it's not that big of a deal, so I don't think observability there is as critical.

So if the new metric you are adding only counts non-compacted blocks, then I think it will still serve this purpose. If we cover all blocks via the (level, source) tuple Marco suggested might future-proof us, so if it's not too hard, maybe we should do that?

Adds a querier metric, cortex_querier_compacted_blocks_queried_total, which indicates the number of blocks fetched from store gateways that were compacted. This can be compared to cortex_querier_blocks_queried_total.

- move metric to storegateway - rename metric to cortex_bucket_store_series_non_compacted_blocks_queried

This reverts commit 50d04932f40ba470e2f6e72a3e5ed9a6b82f8acd.

jhalterman · 2024-01-18T04:31:11Z

Moving to draft since testing in dev shows that some block meta isn't populated before creating the metrics. Feel free to comment/review otherwise.

pkg/storegateway/stats.go

pkg/storegateway/bucket_store_metrics.go

pkg/storegateway/bucket.go

jhalterman · 2024-01-19T00:10:51Z

@dimitarvdimitrov I noticed that some block meta I was using wasn't being populated since it was being fetched via the BucketIndexMetadataFetcher rather than the MetadataFetcher, which I see was changed in #6808. Since compaction level and source aren't available in the bucket index, I reverted to using the MetadataFetcher (integration tests still use this). Let me know what you think.

dimitarvdimitrov · 2024-01-19T10:00:07Z

reverted to using the MetadataFetcher

I'm afraid this will make the store-gateway scan the bucket each every 15 minutes instead of relying on the bucket index. This makes scanning more brittle, slower, and adds costs for list operations (we have docs on the bucket index which contain some more details). Because of this I think it's best to keep using the bucket index scanner.

One option to resolve this is to update the bucket index to start including these two items.

pracucci · 2024-01-19T13:39:42Z

I reverted to using the MetadataFetcher (integration tests still use this)

I agree with what @dimitarvdimitrov said. It's not an option. Actually we want to get rid of MetadataFetcher.

This reverts commit fdd002a6f49f45f43a1919d77822aa17eccfb575.

jhalterman · 2024-01-20T01:26:14Z

@dimitarvdimitrov I took your suggestion, thanks. The bucket index JSON field name is compaction_level, but the label is just level. Let me know if something different is better.

dimitarvdimitrov

LGTM, thank you for your patience with this PR :)

dimitarvdimitrov · 2024-01-23T17:19:46Z

perhaps cortex_bucket_store_blocks_loaded_by_duration could be removed in a separate PR if it's not needed or redundant with this?

once this PR is merged, should I open a follow-up PR to do this or have you already started on it?

jhalterman · 2024-01-23T17:21:28Z

once this PR is merged, should I open a follow-up PR to do this or have you already started on it?

I have not started that yet. If you'd like to do a follow up that would be great.

pkg/storegateway/bucket.go

jhalterman force-pushed the compacted-blocks-metric branch 2 times, most recently from e7fe57a to 75042c9 Compare January 13, 2024 03:20

jhalterman marked this pull request as ready for review January 13, 2024 03:27

jhalterman requested a review from a team as a code owner January 13, 2024 03:27

jhalterman marked this pull request as draft January 13, 2024 18:29

dimitarvdimitrov reviewed Jan 15, 2024

View reviewed changes

pkg/querier/block_streaming.go Outdated Show resolved Hide resolved

pkg/querier/blocks_store_queryable.go Outdated Show resolved Hide resolved

pkg/storage/tsdb/block/meta.go Outdated Show resolved Hide resolved

pkg/storegateway/bucket.go Outdated Show resolved Hide resolved

pracucci reviewed Jan 15, 2024

View reviewed changes

pkg/storage/tsdb/block/meta.go Outdated Show resolved Hide resolved

pkg/querier/blocks_store_queryable.go Outdated Show resolved Hide resolved

jhalterman force-pushed the compacted-blocks-metric branch 2 times, most recently from 87de38c to 08630e1 Compare January 17, 2024 01:08

jhalterman changed the title ~~Add querier metric for compacted store gateway blocks~~ Add querier metric for non-compacted store gateway blocks Jan 17, 2024

jhalterman marked this pull request as ready for review January 17, 2024 01:44

pracucci reviewed Jan 17, 2024

View reviewed changes

pkg/storage/tsdb/block/meta.go Outdated Show resolved Hide resolved

pkg/storage/tsdb/block/meta.go Outdated Show resolved Hide resolved

pkg/storage/tsdb/block/meta.go Outdated Show resolved Hide resolved

jhalterman added 3 commits January 17, 2024 15:32

Add querier metric for compacted store gateway blocks

1a2b365

Adds a querier metric, cortex_querier_compacted_blocks_queried_total, which indicates the number of blocks fetched from store gateways that were compacted. This can be compared to cortex_querier_blocks_queried_total.

Address PR feedback, rename metric

937cc80

- move metric to storegateway - rename metric to cortex_bucket_store_series_non_compacted_blocks_queried

Revert "Auxiliary commit to revert individual files from 75042c9"

3dbf0b0

This reverts commit 50d04932f40ba470e2f6e72a3e5ed9a6b82f8acd.

jhalterman force-pushed the compacted-blocks-metric branch from 08630e1 to 1b5a1fc Compare January 17, 2024 23:35

jhalterman requested a review from pracucci January 18, 2024 00:11

Address PR comments - observe with block and compaction level

5da75da

jhalterman force-pushed the compacted-blocks-metric branch from 1b5a1fc to 5da75da Compare January 18, 2024 02:59

jhalterman removed the request for review from pracucci January 18, 2024 02:59

jhalterman marked this pull request as draft January 18, 2024 04:31

dimitarvdimitrov reviewed Jan 18, 2024

View reviewed changes

pkg/storegateway/stats.go Outdated Show resolved Hide resolved

pkg/storegateway/stats.go Show resolved Hide resolved

pkg/storegateway/bucket_store_metrics.go Outdated Show resolved Hide resolved

pkg/storegateway/bucket.go Outdated Show resolved Hide resolved

jhalterman marked this pull request as ready for review January 19, 2024 00:11

Address PR comments

e0c5cd2

jhalterman force-pushed the compacted-blocks-metric branch from 56026bf to e0c5cd2 Compare January 19, 2024 00:13

Revert "Auxiliary commit to revert individual files from e0c5cd2"

03e281b

This reverts commit fdd002a6f49f45f43a1919d77822aa17eccfb575.

jhalterman changed the title ~~Add querier metric for non-compacted store gateway blocks~~ Add querier metric for block source and compaction level Jan 19, 2024

Add source and level to bucket index

6b52c5b

jhalterman force-pushed the compacted-blocks-metric branch from 755c4c9 to 6b52c5b Compare January 20, 2024 00:52

jhalterman requested a review from dimitarvdimitrov January 20, 2024 01:25

dimitarvdimitrov approved these changes Jan 23, 2024

View reviewed changes

56quarters reviewed Jan 23, 2024

View reviewed changes

pkg/storegateway/bucket.go Show resolved Hide resolved

PR comment - do not aggregate stats

dd8cd53

jhalterman merged commit dcc2d9b into grafana:main Jan 25, 2024
28 checks passed

jhalterman deleted the compacted-blocks-metric branch January 25, 2024 18:19

dimitarvdimitrov mentioned this pull request Feb 6, 2024

store-gateway: remove cortex_bucket_store_blocks_loaded_by_duration #7309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add querier metric for block source and compaction level #7112

Add querier metric for block source and compaction level #7112

jhalterman commented Jan 13, 2024 •

edited

Loading

dimitarvdimitrov left a comment

jhalterman commented Jan 17, 2024 •

edited

Loading

pracucci left a comment

dimitarvdimitrov commented Jan 17, 2024

jhalterman commented Jan 18, 2024

jhalterman commented Jan 19, 2024 •

edited

Loading

dimitarvdimitrov commented Jan 19, 2024

pracucci commented Jan 19, 2024

jhalterman commented Jan 20, 2024 •

edited

Loading

dimitarvdimitrov left a comment

dimitarvdimitrov commented Jan 23, 2024

jhalterman commented Jan 23, 2024

Add querier metric for block source and compaction level #7112

Add querier metric for block source and compaction level #7112

Conversation

jhalterman commented Jan 13, 2024 • edited Loading

What this PR does

Which issue(s) this PR fixes or relates to

Checklist

dimitarvdimitrov left a comment

Choose a reason for hiding this comment

jhalterman commented Jan 17, 2024 • edited Loading

pracucci left a comment

Choose a reason for hiding this comment

dimitarvdimitrov commented Jan 17, 2024

jhalterman commented Jan 18, 2024

jhalterman commented Jan 19, 2024 • edited Loading

dimitarvdimitrov commented Jan 19, 2024

pracucci commented Jan 19, 2024

jhalterman commented Jan 20, 2024 • edited Loading

dimitarvdimitrov left a comment

Choose a reason for hiding this comment

dimitarvdimitrov commented Jan 23, 2024

jhalterman commented Jan 23, 2024

jhalterman commented Jan 13, 2024 •

edited

Loading

jhalterman commented Jan 17, 2024 •

edited

Loading

jhalterman commented Jan 19, 2024 •

edited

Loading

jhalterman commented Jan 20, 2024 •

edited

Loading