From bc1d7d7f270bf0c4553d3a972bcba72a3acf712a Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Mon, 8 Jun 2020 20:55:53 +0200 Subject: [PATCH 1/2] Fixed incorrect query results caused by buffer reuse in merge adapter (#7361) Signed-off-by: Marco Pracucci --- storage/fanout_test.go | 15 +++++++++++---- storage/generic.go | 14 ++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/storage/fanout_test.go b/storage/fanout_test.go index cc839b6ed..a5d04ea8f 100644 --- a/storage/fanout_test.go +++ b/storage/fanout_test.go @@ -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()) @@ -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") }) } diff --git a/storage/generic.go b/storage/generic.go index a5a694cb2..d23e51a6c 100644 --- a/storage/generic.go +++ b/storage/generic.go @@ -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...) } From 0f34535009dfae68ed68d5ee8a27e0f9b666c9ac Mon Sep 17 00:00:00 2001 From: Bartlomiej Plotka Date: Tue, 9 Jun 2020 05:47:53 +0100 Subject: [PATCH 2/2] Cut 2.18.2 + cherry pick of query bugfix. Signed-off-by: Bartlomiej Plotka --- CHANGELOG.md | 4 ++++ VERSION | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aec464e7f..623fa8f73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.18.2 / 2020-06-09 + +* [BUGFIX] TSDB: Fix incorrect query results when using Prometheus with remote reads configured #7361 + ## 2.18.1 / 2020-05-07 * [BUGFIX] TSDB: Fixed snapshot API. #7217 diff --git a/VERSION b/VERSION index a36e9b090..61dc3342f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.18.1 +2.18.2