Skip to content
This repository was archived by the owner on Aug 13, 2019. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ type Series interface {

// Iterator returns a new iterator of the data of the series.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth to mention that we iterate here over raw samples? SeriesIterator does not tell that.

Iterator() SeriesIterator

// Chunks returns a copy of the compressed chunks that make up this series.
Chunks() []chunks.Meta
Copy link
Contributor

@bwplotka bwplotka Jul 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Chunks is breaking the consistency here a bit, becaue we have Iterator above so maybe ChunksIterator would be more appropriate? and have ChunkIterator iterator that iterates over chunks.

I know iterator interface is sometimes bit controversial (e.g very hard to read and debug), but especially for remote read use case it might be actually more valuable to have it in iterator. This is to otentially avoid allocating the whole slice of chunks within Series interface implementation once we compose the frame. For Cortex use case, I am not sure as you just mentioned: This is to enable Cortex to use TSDB What do you think?

of the compressed chunks

Does it mean we guarantee ALL implementations to return chunks with chunks.Meta.Bytes and not only Ref? I don't think we can guarantee that so we might want to add or ref to chunk in chunk file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed iterators here: #665 (: Let me know if this makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the iterator approach is better.

}

// querier aggregates querying results from time blocks within
Expand Down Expand Up @@ -876,6 +879,10 @@ func (s *chunkSeries) Iterator() SeriesIterator {
return newChunkSeriesIterator(s.chunks, s.intervals, s.mint, s.maxt)
}

func (s *chunkSeries) Chunks() []chunks.Meta {
return s.chunks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a copy of chunks as comment suggests? I think it is exactly the same underlying array passed around.

}

// SeriesIterator iterates over the data of a time series.
type SeriesIterator interface {
// Seek advances the iterator forward to the given timestamp.
Expand Down Expand Up @@ -904,6 +911,14 @@ func (s *chainedSeries) Iterator() SeriesIterator {
return newChainedSeriesIterator(s.series...)
}

func (s *chainedSeries) Chunks() []chunks.Meta {
var chunks []chunks.Meta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Size of the slice can be preallocated to len(s.series)

for _, s := range s.series {
chunks = append(s.Chunks())
}
return chunks
}

// chainedSeriesIterator implements a series iterater over a list
// of time-sorted, non-overlapping iterators.
type chainedSeriesIterator struct {
Expand Down Expand Up @@ -961,6 +976,14 @@ func (it *chainedSeriesIterator) Err() error {
return it.cur.Err()
}

func (s *verticalChainedSeries) Chunks() []chunks.Meta {
var chunks []chunks.Meta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Size of the slice can be preallocated to len(s.series)

for _, s := range s.series {
chunks = append(s.Chunks())
}
return chunks
}

// verticalChainedSeries implements a series for a list of time-sorted, time-overlapping series.
// They all must have the same labels.
type verticalChainedSeries struct {
Expand Down
2 changes: 2 additions & 0 deletions querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@ type itSeries struct {

func (s itSeries) Iterator() SeriesIterator { return s.si }
func (s itSeries) Labels() labels.Labels { return labels.Labels{} }
func (s itSeries) Chunks() []chunks.Meta { return nil }

func TestSeriesIterator(t *testing.T) {
itcases := []struct {
Expand Down Expand Up @@ -1441,6 +1442,7 @@ func newSeries(l map[string]string, s []tsdbutil.Sample) Series {
}
func (m *mockSeries) Labels() labels.Labels { return m.labels() }
func (m *mockSeries) Iterator() SeriesIterator { return m.iterator() }
func (m *mockSeries) Chunks() []chunks.Meta { return nil }

type listSeriesIterator struct {
list []tsdbutil.Sample
Expand Down