From 8274e248addacbca809f78012d333adba8c1a8dd Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Thu, 2 Nov 2023 15:29:09 +1100 Subject: [PATCH] Fix issue where `concatenatingChunkIterator` can obscure errors. Signed-off-by: Charles Korn --- storage/merge.go | 3 +++ storage/merge_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/storage/merge.go b/storage/merge.go index 50ae88ce0..501e8db09 100644 --- a/storage/merge.go +++ b/storage/merge.go @@ -860,6 +860,9 @@ func (c *concatenatingChunkIterator) Next() bool { c.curr = c.iterators[c.idx].At() return true } + if c.iterators[c.idx].Err() != nil { + return false + } c.idx++ return c.Next() } diff --git a/storage/merge_test.go b/storage/merge_test.go index f68261d27..25c8fa4a8 100644 --- a/storage/merge_test.go +++ b/storage/merge_test.go @@ -853,6 +853,65 @@ func TestConcatenatingChunkSeriesMerger(t *testing.T) { } } +func TestConcatenatingChunkIterator(t *testing.T) { + chunk1, err := chunks.ChunkFromSamples([]chunks.Sample{fSample{t: 1, f: 10}}) + require.NoError(t, err) + chunk2, err := chunks.ChunkFromSamples([]chunks.Sample{fSample{t: 2, f: 20}}) + require.NoError(t, err) + chunk3, err := chunks.ChunkFromSamples([]chunks.Sample{fSample{t: 3, f: 30}}) + require.NoError(t, err) + + testError := errors.New("something went wrong") + + testCases := map[string]struct { + iterators []chunks.Iterator + expectedChunks []chunks.Meta + expectedError error + }{ + "many successful iterators": { + iterators: []chunks.Iterator{ + NewListChunkSeriesIterator(chunk1, chunk2), + NewListChunkSeriesIterator(chunk3), + }, + expectedChunks: []chunks.Meta{chunk1, chunk2, chunk3}, + }, + "single failing iterator": { + iterators: []chunks.Iterator{ + errChunksIterator{err: testError}, + }, + expectedError: testError, + }, + "some failing and some successful iterators": { + iterators: []chunks.Iterator{ + NewListChunkSeriesIterator(chunk1, chunk2), + errChunksIterator{err: testError}, + NewListChunkSeriesIterator(chunk3), + }, + expectedChunks: []chunks.Meta{chunk1, chunk2}, // Should stop before advancing to last iterator. + expectedError: testError, + }, + } + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + it := concatenatingChunkIterator{iterators: testCase.iterators} + var chks []chunks.Meta + + for it.Next() { + chks = append(chks, it.At()) + } + + require.Equal(t, testCase.expectedChunks, chks) + + if testCase.expectedError == nil { + require.NoError(t, it.Err()) + } else { + require.EqualError(t, it.Err(), testCase.expectedError.Error()) + } + }) + } +} + type mockQuerier struct { LabelQuerier