Browse Source

fix(storage/mergeQuerier): copy the matcjers slice before passing it to queriers as

some of them may alter it.

Signed-off-by: machine424 <ayoubmrini424@gmail.com>
pull/14983/head
machine424 2 months ago
parent
commit
cebcdce78a
No known key found for this signature in database
GPG Key ID: A4B001A4FDEE017D
  1. 13
      storage/merge.go
  2. 8
      tsdb/querier_test.go

13
storage/merge.go

@ -155,19 +155,16 @@ func (q *mergeGenericQuerier) Select(ctx context.Context, sortSeries bool, hints
for _, querier := range q.queriers { for _, querier := range q.queriers {
// copy the matchers as some queriers may alter the slice. // copy the matchers as some queriers may alter the slice.
// See https://github.com/prometheus/prometheus/issues/14723 // See https://github.com/prometheus/prometheus/issues/14723
// matchersCopy := make([]*labels.Matcher, len(matchers)) matchersCopy := make([]*labels.Matcher, len(matchers))
// copy(matchersCopy, matchers) copy(matchersCopy, matchers)
wg.Add(1) wg.Add(1)
go func(qr genericQuerier) { go func(qr genericQuerier, m []*labels.Matcher) {
// go func(qr genericQuerier, m []*labels.Matcher) {
defer wg.Done() defer wg.Done()
// We need to sort for NewMergeSeriesSet to work. // We need to sort for NewMergeSeriesSet to work.
// seriesSetChan <- qr.Select(ctx, true, hints, m...) seriesSetChan <- qr.Select(ctx, true, hints, m...)
seriesSetChan <- qr.Select(ctx, true, hints, matchers...) }(querier, matchersCopy)
}(querier)
// }(querier, matchersCopy)
} }
go func() { go func() {
wg.Wait() wg.Wait()

8
tsdb/querier_test.go

@ -3794,6 +3794,9 @@ func (m mockReaderOfLabels) Symbols() index.StringIter {
func TestMergeQuerierConcurrentSelectMatchers(t *testing.T) { func TestMergeQuerierConcurrentSelectMatchers(t *testing.T) {
block, err := OpenBlock(nil, createBlock(t, t.TempDir(), genSeries(1, 1, 0, 1)), nil) block, err := OpenBlock(nil, createBlock(t, t.TempDir(), genSeries(1, 1, 0, 1)), nil)
require.NoError(t, err) require.NoError(t, err)
defer func() {
require.NoError(t, block.Close())
}()
p, err := NewBlockQuerier(block, 0, 1) p, err := NewBlockQuerier(block, 0, 1)
require.NoError(t, err) require.NoError(t, err)
@ -3808,7 +3811,10 @@ func TestMergeQuerierConcurrentSelectMatchers(t *testing.T) {
matchers := append([]*labels.Matcher{}, originalMatchers...) matchers := append([]*labels.Matcher{}, originalMatchers...)
mergedQuerier := storage.NewMergeQuerier([]storage.Querier{p}, []storage.Querier{s}, storage.ChainedSeriesMerge) mergedQuerier := storage.NewMergeQuerier([]storage.Querier{p}, []storage.Querier{s}, storage.ChainedSeriesMerge)
defer mergedQuerier.Close() defer func() {
require.NoError(t, mergedQuerier.Close())
}()
mergedQuerier.Select(context.Background(), false, nil, matchers...) mergedQuerier.Select(context.Background(), false, nil, matchers...)
require.Equal(t, originalMatchers, matchers) require.Equal(t, originalMatchers, matchers)

Loading…
Cancel
Save