diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 0493984a1..a3bfc6bb7 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -584,8 +584,6 @@ type reverseFloatBucketIterator struct { currCount float64 // Count in the current bucket. currIdx int32 // The actual bucket index. currLower, currUpper float64 // Limits of the current bucket. - - initiated bool } func newReverseFloatBucketIterator(h *FloatHistogram, positive bool) *reverseFloatBucketIterator { @@ -597,24 +595,21 @@ func newReverseFloatBucketIterator(h *FloatHistogram, positive bool) *reverseFlo r.spans = h.NegativeSpans r.buckets = h.NegativeBuckets } + + r.spansIdx = len(r.spans) - 1 + r.bucketsIdx = len(r.buckets) - 1 + if r.spansIdx >= 0 { + r.idxInSpan = int32(r.spans[r.spansIdx].Length) - 1 + } + r.currIdx = 0 + for _, s := range r.spans { + r.currIdx += s.Offset + int32(s.Length) + } + return r } func (r *reverseFloatBucketIterator) Next() bool { - if !r.initiated { - r.initiated = true - r.spansIdx = len(r.spans) - 1 - r.bucketsIdx = len(r.buckets) - 1 - if r.spansIdx >= 0 { - r.idxInSpan = int32(r.spans[r.spansIdx].Length) - 1 - } - - r.currIdx = 0 - for _, s := range r.spans { - r.currIdx += s.Offset + int32(s.Length) - } - } - r.currIdx-- if r.bucketsIdx < 0 { return false diff --git a/promql/quantile.go b/promql/quantile.go index cb9783ced..aa19bde73 100644 --- a/promql/quantile.go +++ b/promql/quantile.go @@ -151,32 +151,35 @@ func histogramQuantile(q float64, h *histogram.FloatHistogram) float64 { } var ( - bucket *histogram.FloatBucket + bucket histogram.FloatBucket count float64 it = h.AllBucketIterator() rank = q * h.Count - idx = -1 ) - // TODO(codesome): Do we need any special handling for negative buckets? for it.Next() { - idx++ - b := it.At() - count += b.Count + bucket = it.At() + count += bucket.Count if count >= rank { - bucket = &b break } } - if bucket == nil { - panic("histogramQuantile: not possible") - } - - if idx == 0 && bucket.Lower < 0 && bucket.Upper > 0 { - // Zero bucket has the result and it happens to be the first - // bucket of this histogram. So we consider 0 to be the lower - // bound. + if bucket.Lower < 0 && bucket.Upper > 0 && len(h.NegativeBuckets) == 0 { + // The result is in the zero bucket and the histogram has no + // negative buckets. So we consider 0 to be the lower bound. bucket.Lower = 0 } + // Due to numerical inaccuracies, we could end up with a higher count + // than h.Count. Thus, make sure count is never higher than h.Count. + if count > h.Count { + count = h.Count + } + // We could have hit the highest bucket without even reaching the rank + // (observations not counted in any bucket are considered "overflow" + // observations above the highest bucket), in which case we simple + // return the upper limit of the highest explicit bucket. + if count < rank { + return bucket.Upper + } rank -= count - bucket.Count // TODO(codesome): Use a better estimation than linear.