diff --git a/CHANGELOG.md b/CHANGELOG.md index fa1367c41b..a1bbcc43c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re - [#7658](https://github.com/thanos-io/thanos/pull/7658) Store: Fix panic because too small buffer in pool. - [#7643](https://github.com/thanos-io/thanos/pull/7643) Receive: fix thanos_receive_write_{timeseries,samples} stats - [#7644](https://github.com/thanos-io/thanos/pull/7644) fix(ui): add null check to find overlapping blocks logic +- [#7814](https://github.com/thanos-io/thanos/pull/7814) Store: label_values: if matchers contain __name__=="something", do not add != "" to fetch less postings. - [#7679](https://github.com/thanos-io/thanos/pull/7679) Query: respect store.limit.* flags when evaluating queries ### Added diff --git a/pkg/store/acceptance_test.go b/pkg/store/acceptance_test.go index 793f16eb60..9a3cba1311 100644 --- a/pkg/store/acceptance_test.go +++ b/pkg/store/acceptance_test.go @@ -740,6 +740,46 @@ func testStoreAPIsAcceptance(t *testing.T, startStore startStoreFn) { }, }, }, + { + desc: "label_values(kube_pod_info{}, pod) don't fetch postings for pod!=''", + appendFn: func(app storage.Appender) { + _, err := app.Append(0, labels.FromStrings("__name__", "up", "pod", "pod-1"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + _, err = app.Append(0, labels.FromStrings("__name__", "up", "pod", "pod-2"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + _, err = app.Append(0, labels.FromStrings("__name__", "kube_pod_info", "pod", "pod-1"), timestamp.FromTime(now), 1) + testutil.Ok(t, err) + testutil.Ok(t, app.Commit()) + }, + labelNameCalls: []labelNameCallCase{ + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + expectedNames: []string{"__name__", "pod", "region"}, + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "__name__", Value: "kube_pod_info"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + expectedNames: []string{"__name__", "pod", "region"}, + }, + }, + labelValuesCalls: []labelValuesCallCase{ + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "pod", + expectedValues: []string{"pod-1"}, + matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_EQ, Name: "__name__", Value: "kube_pod_info"}}, + }, + { + start: timestamp.FromTime(minTime), + end: timestamp.FromTime(maxTime), + label: "pod", + expectedValues: []string{"pod-1", "pod-2"}, + }, + }, + }, } { t.Run(tc.desc, func(t *testing.T) { appendFn := tc.appendFn diff --git a/pkg/store/bucket.go b/pkg/store/bucket.go index 865ea3878d..ec9378cfa2 100644 --- a/pkg/store/bucket.go +++ b/pkg/store/bucket.go @@ -1992,6 +1992,14 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR resHints := &hintspb.LabelValuesResponseHints{} + var hasMetricNameEqMatcher = false + for _, m := range reqSeriesMatchers { + if m.Name == labels.MetricName && m.Type == labels.MatchEqual { + hasMetricNameEqMatcher = true + break + } + } + g, gctx := errgroup.WithContext(ctx) var reqBlockMatchers []*labels.Matcher @@ -2015,6 +2023,7 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR var seriesLimiter = s.seriesLimiterFactory(s.metrics.queriesDropped.WithLabelValues("series", tenant)) var bytesLimiter = s.bytesLimiterFactory(s.metrics.queriesDropped.WithLabelValues("bytes", tenant)) var logger = s.requestLoggerFunc(ctx, s.logger) + var stats = &queryStats{} for _, b := range s.blocks { b := b @@ -2033,7 +2042,8 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR // If we have series matchers and the Label is not an external one, add != "" matcher // to only select series that have given label name. - if len(reqSeriesMatchersNoExtLabels) > 0 && !b.extLset.Has(req.Label) { + // We don't need such matcher if matchers already contain __name__=="something" matcher. + if !hasMetricNameEqMatcher && len(reqSeriesMatchersNoExtLabels) > 0 && !b.extLset.Has(req.Label) { m, err := labels.NewMatcher(labels.MatchNotEqual, req.Label, "") if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) @@ -2101,7 +2111,12 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR s.metrics.lazyExpandedPostingSeriesOverfetchedSizeBytes, tenant, ) - defer blockClient.Close() + defer func() { + mtx.Lock() + stats = blockClient.MergeStats(stats) + mtx.Unlock() + blockClient.Close() + }() if err := blockClient.ExpandPostings( sortedReqSeriesMatchersNoExtLabels, @@ -2130,7 +2145,7 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR } val := labelpb.ZLabelsToPromLabels(ls.GetSeries().Labels).Get(req.Label) - if val != "" { // Should never be empty since we added labelName!="" matcher to the list of matchers. + if val != "" { values[val] = struct{}{} } } @@ -2154,6 +2169,15 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR s.mtx.RUnlock() + defer func() { + if s.debugLogging { + level.Debug(logger).Log("msg", "stats query processed", + "request", req, + "tenant", tenant, + "stats", fmt.Sprintf("%+v", stats), "err", err) + } + }() + if err := g.Wait(); err != nil { code := codes.Internal if s, ok := status.FromError(errors.Cause(err)); ok {