Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

query: fixing dedup iterator when working on mixed sample types #7271

Merged
merged 10 commits into from
Apr 11, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
- [#7224](https://github.com/thanos-io/thanos/pull/7224) Query-frontend: Add Redis username to the client configuration.
- [#7220](https://github.com/thanos-io/thanos/pull/7220) Store Gateway: Fix lazy expanded postings caching partial expanded postings and bug of estimating remove postings with non existent value. Added `PromQLSmith` based fuzz test to improve correctness.
- [#7244](https://github.com/thanos-io/thanos/pull/7244) Query: Fix Internal Server Error unknown targetHealth: "unknown" when trying to open the targets page.
- [#7271](https://github.com/thanos-io/thanos/pull/7271) Query: fixing dedup iterator when working on mixed sample types.

### Added

Expand Down
69 changes: 12 additions & 57 deletions pkg/compact/downsample/downsample_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

"github.com/thanos-io/thanos/pkg/block"
"github.com/thanos-io/thanos/pkg/block/metadata"
"github.com/thanos-io/thanos/pkg/testutil/testiters"
)

func TestMain(m *testing.M) {
Expand Down Expand Up @@ -1511,26 +1512,26 @@ func TestApplyCounterResetsIteratorHistograms(t *testing.T) {

histograms := tsdbutil.GenerateTestHistograms(lenChunks * lenChunk)

var chunks [][]*histogramPair
var chunks [][]*testiters.HistogramPair
for i := 0; i < lenChunks; i++ {
var chunk []*histogramPair
var chunk []*testiters.HistogramPair
for j := 0; j < lenChunk; j++ {
chunk = append(chunk, &histogramPair{t: int64(i*lenChunk+j) * 100, h: histograms[i*lenChunk+j]})
chunk = append(chunk, &testiters.HistogramPair{T: int64(i*lenChunk+j) * 100, H: histograms[i*lenChunk+j]})
}
chunks = append(chunks, chunk)
}

var expected []*histogramPair
var expected []*testiters.HistogramPair
for i, h := range histograms {
expected = append(expected, &histogramPair{t: int64(i * 100), h: h})
expected = append(expected, &testiters.HistogramPair{T: int64(i * 100), H: h})
}

for _, tcase := range []struct {
name string

chunks [][]*histogramPair
chunks [][]*testiters.HistogramPair

expected []*histogramPair
expected []*testiters.HistogramPair
}{
{
name: "histogram series",
Expand All @@ -1541,21 +1542,21 @@ func TestApplyCounterResetsIteratorHistograms(t *testing.T) {
t.Run(tcase.name, func(t *testing.T) {
var its []chunkenc.Iterator
for _, c := range tcase.chunks {
its = append(its, newHistogramIterator(c))
its = append(its, testiters.NewHistogramIterator(c))
}

x := NewApplyCounterResetsIterator(its...)

var res []*histogramPair
var res []*testiters.HistogramPair
for x.Next() != chunkenc.ValNone {
t, h := x.AtHistogram(nil)
res = append(res, &histogramPair{t, h})
res = append(res, &testiters.HistogramPair{T: t, H: h})
}
testutil.Ok(t, x.Err())
testutil.Equals(t, tcase.expected, res)

for i := range res[1:] {
testutil.Assert(t, res[i+1].t >= res[i].t, "sample time %v is not monotonically increasing. previous sample %v is older", res[i+1], res[i])
testutil.Assert(t, res[i+1].T >= res[i].T, "sample time %v is not monotonically increasing. previous sample %v is older", res[i+1], res[i])
}
})
}
Expand Down Expand Up @@ -1739,52 +1740,6 @@ func (it *sampleIterator) AtT() int64 {
return it.l[it.i].t
}

type histogramPair struct {
t int64
h *histogram.Histogram
}

type histogramIterator struct {
l []*histogramPair
i int
}

func newHistogramIterator(l []*histogramPair) *histogramIterator {
return &histogramIterator{l: l, i: -1}
}

func (it *histogramIterator) Err() error {
return nil
}

func (it *histogramIterator) Next() chunkenc.ValueType {
if it.i >= len(it.l)-1 {
return chunkenc.ValNone
}
it.i++
return chunkenc.ValHistogram
}

func (it *histogramIterator) Seek(int64) chunkenc.ValueType {
panic("unexpected")
}

func (it *histogramIterator) At() (t int64, v float64) {
panic("not implemented")
}

func (it *histogramIterator) AtHistogram(*histogram.Histogram) (int64, *histogram.Histogram) {
return it.l[it.i].t, it.l[it.i].h
}

func (it *histogramIterator) AtFloatHistogram(*histogram.FloatHistogram) (int64, *histogram.FloatHistogram) {
panic("not implemented")
}

func (it *histogramIterator) AtT() int64 {
return it.l[it.i].t
}

// memBlock is an in-memory block that implements a subset of the tsdb.BlockReader interface
// to allow tsdb.StreamedBlockWriter to persist the data as a block.
type memBlock struct {
Expand Down
1 change: 1 addition & 0 deletions pkg/dedup/iter.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ func newDedupSeriesIterator(a, b adjustableSeriesIterator) *dedupSeriesIterator
b: b,
lastT: math.MinInt64,
lastIter: a,
useA: true,
aval: a.Next(),
bval: b.Next(),
}
Expand Down
Loading
Loading