Skip to content

Commit

Permalink
Fixed incorrect query results caused by buffer reuse in merge adapter (
Browse files Browse the repository at this point in the history
…prometheus#7361)

Signed-off-by: Marco Pracucci <[email protected]>
  • Loading branch information
pracucci authored and bwplotka committed Jun 9, 2020
1 parent ecee9c8 commit bc1d7d7
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
15 changes: 11 additions & 4 deletions storage/fanout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,18 @@ func TestMergeQuerierWithChainMerger(t *testing.T) {
}
qs = append(qs, tc.extraQueriers...)

merged, _, _ := NewMergeQuerier(qs[0], qs, ChainedSeriesMerge).Select(false, nil)
for merged.Next() {
mergedQuerier, _, _ := NewMergeQuerier(qs[0], qs, ChainedSeriesMerge).Select(false, nil)

// Get all merged series upfront to make sure there are no incorrectly retained shared
// buffers causing bugs.
var mergedSeries []Series
for mergedQuerier.Next() {
mergedSeries = append(mergedSeries, mergedQuerier.At())
}
testutil.Ok(t, mergedQuerier.Err())

for _, actualSeries := range mergedSeries {
testutil.Assert(t, tc.expected.Next(), "Expected Next() to be true")
actualSeries := merged.At()
expectedSeries := tc.expected.At()
testutil.Equals(t, expectedSeries.Labels(), actualSeries.Labels())

Expand All @@ -207,7 +215,6 @@ func TestMergeQuerierWithChainMerger(t *testing.T) {
testutil.Equals(t, expErr, actErr)
testutil.Equals(t, expSmpl, actSmpl)
}
testutil.Ok(t, merged.Err())
testutil.Assert(t, !tc.expected.Next(), "Expected Next() to be false")
})
}
Expand Down
14 changes: 6 additions & 8 deletions storage/generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,26 +108,24 @@ func (q *chunkQuerierAdapter) Select(sortSeries bool, hints *SelectHints, matche

type seriesMergerAdapter struct {
VerticalSeriesMergeFunc
buf []Series
}

func (a *seriesMergerAdapter) Merge(s ...Labels) Labels {
a.buf = a.buf[:0]
buf := make([]Series, 0, len(s))
for _, ser := range s {
a.buf = append(a.buf, ser.(Series))
buf = append(buf, ser.(Series))
}
return a.VerticalSeriesMergeFunc(a.buf...)
return a.VerticalSeriesMergeFunc(buf...)
}

type chunkSeriesMergerAdapter struct {
VerticalChunkSeriesMergerFunc
buf []ChunkSeries
}

func (a *chunkSeriesMergerAdapter) Merge(s ...Labels) Labels {
a.buf = a.buf[:0]
buf := make([]ChunkSeries, 0, len(s))
for _, ser := range s {
a.buf = append(a.buf, ser.(ChunkSeries))
buf = append(buf, ser.(ChunkSeries))
}
return a.VerticalChunkSeriesMergerFunc(a.buf...)
return a.VerticalChunkSeriesMergerFunc(buf...)
}

0 comments on commit bc1d7d7

Please sign in to comment.