From 7bbbd55aada925efb3d2f925e2ade3f9fc6ebc48 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Wed, 3 May 2017 22:45:28 +0530 Subject: [PATCH] Fix bug where having one chunk can cause panic When we have only one chunk that is out of range, then we are returning it unpopulated (w/o calling `Chunk(ref)`). This would cause a panic downstream. Fixes: prometheus/prometheus#2629 Signed-off-by: Goutham Veeramachaneni --- querier.go | 4 +++- querier_test.go | 28 ++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/querier.go b/querier.go index ca4ca5738..a0276b230 100644 --- a/querier.go +++ b/querier.go @@ -420,7 +420,7 @@ func (s *populatedChunkSeries) Next() bool { continue } if c.MinTime > s.maxt { - chks = chks[from+1 : i] + chks = chks[:i] break } c.Chunk, s.err = s.chunks.Chunk(c.Ref) @@ -428,6 +428,8 @@ func (s *populatedChunkSeries) Next() bool { return false } } + + chks = chks[from+1:] if len(chks) == 0 { continue } diff --git a/querier_test.go b/querier_test.go index 73359926c..864e76cbb 100644 --- a/querier_test.go +++ b/querier_test.go @@ -41,6 +41,12 @@ type mockSeries struct { iterator func() SeriesIterator } +func newSeries(l map[string]string, s []sample) Series { + return &mockSeries{ + labels: func() labels.Labels { return labels.FromMap(l) }, + iterator: func() SeriesIterator { return newListSeriesIterator(s) }, + } +} func (m *mockSeries) Labels() labels.Labels { return m.labels() } func (m *mockSeries) Iterator() SeriesIterator { return m.iterator() } @@ -81,12 +87,6 @@ func (it *listSeriesIterator) Err() error { } func TestMergedSeriesSet(t *testing.T) { - newSeries := func(l map[string]string, s []sample) Series { - return &mockSeries{ - labels: func() labels.Labels { return labels.FromMap(l) }, - iterator: func() SeriesIterator { return newListSeriesIterator(s) }, - } - } cases := []struct { // The input sets in order (samples in series in b are strictly @@ -885,6 +885,22 @@ func TestPopulatedCSReturnsValidChunkSlice(t *testing.T) { p.maxt = 9 require.False(t, p.Next()) + // Test the case where 1 chunk could cause an unpopulated chunk to be returned. + chunkMetas = [][]*ChunkMeta{ + { + {MinTime: 1, MaxTime: 2, Ref: 1}, + }, + } + + m = &mockChunkSeriesSet{l: lbls, cm: chunkMetas, i: -1} + p = &populatedChunkSeries{ + set: m, + chunks: cr, + + mint: 10, + maxt: 15, + } + require.False(t, p.Next()) return }