Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter external labels from matchers on LabelValues/LabelNames #5596

Merged
merged 4 commits into from
Aug 17, 2022

Conversation

oronsh
Copy link
Contributor

@oronsh oronsh commented Aug 15, 2022

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Filter external labels from matchers in LabelValues and LabelNames rpcs.
We noticed super high memory spikes in storegateway for the label apis and noticed that if there're matchers store will try to fetch series for postings.
When using prom-label-proxy to enforce tenant_id label for example, it's propagating to matchers and so going to slow path, since tenant_id or any other external label is only presented on meta.json, all series would have to be fetched resulting in huge memory spikes (~50Gb and more).

By filtering external labels from matchers we ensure this won't happen and also filtering the blocks that have those external labels but with different values.
Screen Shot 2022-08-15 at 18 52 44

Tested locally and it solved the issue.

Verification

@oronsh oronsh force-pushed the filter-ext-labels branch from dea5e79 to 02dfb61 Compare August 15, 2022 12:00
pkg/store/bucket_test.go Outdated Show resolved Hide resolved
pkg/store/bucket_test.go Outdated Show resolved Hide resolved
pkg/store/bucket.go Outdated Show resolved Hide resolved
pkg/store/bucket_test.go Outdated Show resolved Hide resolved
Signed-off-by: Oron Sharabi <[email protected]>
@oronsh
Copy link
Contributor Author

oronsh commented Aug 15, 2022

Fixes issue #5577

Signed-off-by: Oron Sharabi <[email protected]>
bwplotka
bwplotka previously approved these changes Aug 17, 2022
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job and idea, thanks! 💪🏽

Do you mind re-shaping comments to be compatible with our Go guide?

@@ -1254,6 +1254,11 @@ func (s *BucketStore) LabelNames(ctx context.Context, req *storepb.LabelNamesReq
if len(reqBlockMatchers) > 0 && !b.matchRelabelLabels(reqBlockMatchers) {
continue
}
// filter ext labels
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have comments as full sentences?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -1341,6 +1346,25 @@ func (s *BucketStore) LabelNames(ctx context.Context, req *storepb.LabelNamesReq
}, nil
}

func (b *bucketBlock) FilterExtLabelsMatchers(matchers []*labels.Matcher) ([]*labels.Matcher, bool) {
// we filter external labels from matchers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditoo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@oronsh
Copy link
Contributor Author

oronsh commented Aug 17, 2022

Thanks for the review @bwplotka ! fixed comments :)

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪🏽

@bwplotka
Copy link
Member

Thanks!

@bwplotka bwplotka merged commit 9ec3763 into thanos-io:main Aug 17, 2022
GiedriusS pushed a commit to vinted/thanos that referenced this pull request Aug 25, 2022
…s-io#5596)

* Filter external labels from matchers on LabelValues/LabelNames

Signed-off-by: Oron Sharabi <[email protected]>

* Fix linting

Signed-off-by: Oron Sharabi <[email protected]>

* Fix linter

Signed-off-by: Oron Sharabi <[email protected]>

* Fix comments to be full Sentences

Signed-off-by: Oron Sharabi <[email protected]>

Signed-off-by: Oron Sharabi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants