From 6169c33fb8da100fed7bae7a3f07ee459101d750 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Mon, 1 May 2017 14:33:56 +0530 Subject: [PATCH 1/2] Fix #59 Mutating the chunks can change their length. Hence referencing using previous indices can cause panics. Signed-off-by: Goutham Veeramachaneni --- querier.go | 5 +++-- querier_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/querier.go b/querier.go index 2a58179a0..a13ae1188 100644 --- a/querier.go +++ b/querier.go @@ -400,13 +400,14 @@ func (s *populatedChunkSeries) Next() bool { for s.set.Next() { lset, chks := s.set.At() + from := 0 for i, c := range chks { if c.MaxTime < s.mint { - chks = chks[1:] + from = i continue } if c.MinTime > s.maxt { - chks = chks[:i] + chks = chks[from+1 : i] break } c.Chunk, s.err = s.chunks.Chunk(c.Ref) diff --git a/querier_test.go b/querier_test.go index d0b61c336..a1af21be6 100644 --- a/querier_test.go +++ b/querier_test.go @@ -838,3 +838,57 @@ func TestSeriesIterator(t *testing.T) { return } + +func TestPopulatedCSReturnsValidChunkSlice(t *testing.T) { + lbls := []labels.Labels{labels.New(labels.Label{"a", "b"})} + chunkMetas := [][]*ChunkMeta{ + { + {MinTime: 1, MaxTime: 2, Ref: 1}, + {MinTime: 3, MaxTime: 4, Ref: 2}, + {MinTime: 10, MaxTime: 12, Ref: 3}, + }, + } + + cr := mockChunkReader( + map[uint64]chunks.Chunk{ + 1: chunks.NewXORChunk(), + 2: chunks.NewXORChunk(), + 3: chunks.NewXORChunk(), + }, + ) + + m := &mockChunkSeriesSet{l: lbls, cm: chunkMetas, i: -1} + p := &populatedChunkSeries{ + set: m, + chunks: cr, + + mint: 6, + maxt: 9, + } + + require.False(t, p.Next()) + return +} + +type mockChunkSeriesSet struct { + l []labels.Labels + cm [][]*ChunkMeta + + i int +} + +func (m *mockChunkSeriesSet) Next() bool { + if len(m.l) != len(m.cm) { + return false + } + m.i++ + return m.i < len(m.l) +} + +func (m *mockChunkSeriesSet) At() (labels.Labels, []*ChunkMeta) { + return m.l[m.i], m.cm[m.i] +} + +func (m *mockChunkSeriesSet) Err() error { + return nil +} From 8b43b0d2c1efe850cb6ff70915604bea05700cf6 Mon Sep 17 00:00:00 2001 From: Goutham Veeramachaneni Date: Mon, 1 May 2017 15:01:17 +0530 Subject: [PATCH 2/2] Fix broken tests Signed-off-by: Goutham Veeramachaneni --- querier.go | 2 +- querier_test.go | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/querier.go b/querier.go index a13ae1188..82c1ff930 100644 --- a/querier.go +++ b/querier.go @@ -400,7 +400,7 @@ func (s *populatedChunkSeries) Next() bool { for s.set.Next() { lset, chks := s.set.At() - from := 0 + from := -1 for i, c := range chks { if c.MaxTime < s.mint { from = i diff --git a/querier_test.go b/querier_test.go index a1af21be6..64388423f 100644 --- a/querier_test.go +++ b/querier_test.go @@ -862,11 +862,16 @@ func TestPopulatedCSReturnsValidChunkSlice(t *testing.T) { set: m, chunks: cr, - mint: 6, - maxt: 9, + mint: 0, + maxt: 0, } require.False(t, p.Next()) + + p.mint = 6 + p.maxt = 9 + require.False(t, p.Next()) + return }