From 49fea5e8875eb07914f79f51d70904d8c7e4ec6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Giedrius=20Statkevi=C4=8Dius?= Date: Tue, 1 Aug 2023 13:23:30 +0300 Subject: [PATCH] store/bucket: remove sort.Slice data race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The matchers slice is now sorted in each call but that introduces a data race because the slice is shared between all calls. Do the sorting once on the outermost function. Signed-off-by: Giedrius Statkevičius --- pkg/store/bucket.go | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/pkg/store/bucket.go b/pkg/store/bucket.go index c9d405e0056..86fc1563dca 100644 --- a/pkg/store/bucket.go +++ b/pkg/store/bucket.go @@ -1284,6 +1284,17 @@ func (s *BucketStore) Series(req *storepb.SeriesRequest, srv storepb.Store_Serie continue } + // Sort matchers to make sure we generate the same cache key. + sort.Slice(blockMatchers, func(i, j int) bool { + if blockMatchers[i].Type == blockMatchers[j].Type { + if blockMatchers[i].Name == blockMatchers[j].Name { + return blockMatchers[i].Value < blockMatchers[j].Value + } + return blockMatchers[i].Name < blockMatchers[j].Name + } + return blockMatchers[i].Type < blockMatchers[j].Type + }) + blocks := bs.getFor(req.MinTime, req.MaxTime, req.MaxResolutionWindow, reqBlockMatchers) if s.debugLogging { @@ -1527,6 +1538,17 @@ func (s *BucketStore) LabelNames(ctx context.Context, req *storepb.LabelNamesReq continue } + // Sort matchers to make sure we generate the same cache key. + sort.Slice(reqSeriesMatchersNoExtLabels, func(i, j int) bool { + if reqSeriesMatchersNoExtLabels[i].Type == reqSeriesMatchersNoExtLabels[j].Type { + if reqSeriesMatchersNoExtLabels[i].Name == reqSeriesMatchersNoExtLabels[j].Name { + return reqSeriesMatchersNoExtLabels[i].Value < reqSeriesMatchersNoExtLabels[j].Value + } + return reqSeriesMatchersNoExtLabels[i].Name < reqSeriesMatchersNoExtLabels[j].Name + } + return reqSeriesMatchersNoExtLabels[i].Type < reqSeriesMatchersNoExtLabels[j].Type + }) + resHints.AddQueriedBlock(b.meta.ULID) indexr := b.indexReader() @@ -1727,6 +1749,17 @@ func (s *BucketStore) LabelValues(ctx context.Context, req *storepb.LabelValuesR reqSeriesMatchersNoExtLabels = append(reqSeriesMatchersNoExtLabels, m) } + // Sort matchers to make sure we generate the same cache key. + sort.Slice(reqSeriesMatchersNoExtLabels, func(i, j int) bool { + if reqSeriesMatchersNoExtLabels[i].Type == reqSeriesMatchersNoExtLabels[j].Type { + if reqSeriesMatchersNoExtLabels[i].Name == reqSeriesMatchersNoExtLabels[j].Name { + return reqSeriesMatchersNoExtLabels[i].Value < reqSeriesMatchersNoExtLabels[j].Value + } + return reqSeriesMatchersNoExtLabels[i].Name < reqSeriesMatchersNoExtLabels[j].Name + } + return reqSeriesMatchersNoExtLabels[i].Type < reqSeriesMatchersNoExtLabels[j].Type + }) + resHints.AddQueriedBlock(b.meta.ULID) indexr := b.indexReader() @@ -2203,16 +2236,6 @@ func (r *bucketIndexReader) ExpandedPostings(ctx context.Context, ms []*labels.M return nil, nil } - // Sort matchers to make sure we generate the same cache key. - sort.Slice(ms, func(i, j int) bool { - if ms[i].Type == ms[j].Type { - if ms[i].Name == ms[j].Name { - return ms[i].Value < ms[j].Value - } - return ms[i].Name < ms[j].Name - } - return ms[i].Type < ms[j].Type - }) hit, postings, err := r.fetchExpandedPostingsFromCache(ctx, ms, bytesLimiter) if err != nil { return nil, err