Skip to content

Commit

Permalink
store/bucket: remove sort.Slice data race
Browse files Browse the repository at this point in the history
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 <giedrius.statkevicius@vinted.com>
  • Loading branch information
GiedriusS committed Aug 1, 2023
1 parent a35a5b2 commit 49fea5e
Showing 1 changed file with 33 additions and 10 deletions.
43 changes: 33 additions & 10 deletions pkg/store/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 49fea5e

Please sign in to comment.