Skip to content

Commit

Permalink
Preserve reset hint during histogram deduplication
Browse files Browse the repository at this point in the history
Penalty based deduplication can over-skip samples because when switching
between replicas, because it uses a penalty of 2 times the estimated scrape interval.
This works okay for float counters because the reset is detected based on the value of the counter.
With native histograms, detecting a reset is much more expensive which is why there is a
hint stored in the first sample of a chunk, which indicates whether the chunk has been created
because of a reset.

It can easily happen that the dedup iterator switches replicas, and skips the first sample of a chunk
because of the added penalty. In this case we will not read the hint and will assume that no
histogram reset happened.

This commit fixes the issue by explicitly calling DetectReset if a replica stream has been switched.
The result is then stored and set in the histogram returned by AtHistogram and AtFloatHistogram.

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
  • Loading branch information
fpetkovski committed Jan 29, 2024
1 parent 6a0a491 commit 6cd1b77
Showing 1 changed file with 65 additions and 1 deletion.
66 changes: 65 additions & 1 deletion pkg/query/iter.go
Expand Up @@ -168,7 +168,7 @@ func getFirstIterator(cs ...*storepb.Chunk) chunkenc.Iterator {
if err != nil {
return errSeriesIterator{err}
}
return chk.Iterator(nil)
return newHistogramResetDetector(chk.Iterator(nil))
}
return errSeriesIterator{errors.New("no valid chunk found")}
}
Expand Down Expand Up @@ -304,3 +304,67 @@ func (c *lazySeriesSet) Warnings() annotations.Annotations {
}
return nil
}

// histogramResetDetector sets the CounterResetHint to UnknownCounterReset for the first histogram read from the Iterator.
//
// The reset hint is always present in the first sample of the chunk, and all
// consecutive samples will have a NotCounterReset hint. During deduplication the
// first sample could be skipped, which means the reset will not be properly
// detected. This iterator will make sure that in that case the hint for the
// first read sample is set to UnknownCounterReset so that the PromQL engine will
// do the reset detection manually.
type histogramResetDetector struct {
chunkenc.Iterator

lastT int64
lastValType chunkenc.ValueType

i int16
histogramRead bool
}

func newHistogramResetDetector(iterator chunkenc.Iterator) *histogramResetDetector {
return &histogramResetDetector{
Iterator: iterator,
i: -1,
}
}

func (it *histogramResetDetector) Seek(t int64) chunkenc.ValueType {
for {
if it.lastT >= t {
return it.lastValType
}
if it.lastValType = it.Next(); it.lastValType == chunkenc.ValNone {
return chunkenc.ValNone
}
}
}

func (it *histogramResetDetector) Next() chunkenc.ValueType {
it.i++
it.lastValType = it.Iterator.Next()
if it.lastValType == chunkenc.ValNone {
return chunkenc.ValNone
}
it.lastT = it.Iterator.AtT()
return it.lastValType
}

func (it *histogramResetDetector) AtHistogram() (int64, *histogram.Histogram) {
t, h := it.Iterator.AtHistogram()
if !it.histogramRead && it.i != 0 {
h.CounterResetHint = histogram.UnknownCounterReset
}
it.histogramRead = true
return t, h
}

func (it *histogramResetDetector) AtFloatHistogram() (int64, *histogram.FloatHistogram) {
t, fh := it.Iterator.AtFloatHistogram()
if !it.histogramRead && it.i != 0 {
fh.CounterResetHint = histogram.UnknownCounterReset
}
it.histogramRead = true
return t, fh
}

0 comments on commit 6cd1b77

Please sign in to comment.