From 53a720eed5a13cf2f9b7f841a29975aa5737b734 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 28 Aug 2025 15:00:25 +0200 Subject: [PATCH] promql: Minor improvements for HistogramStatsIterator - Add a code comment about a counter reset edge case (which is hopefully not relevant in practice). - Rename the receiver from `f` to `hsi`. (`f` seemed like completely off as a name. `i` or `it` might have worked, too, but I ended up with `hsi` as the easiest for the reader.) Signed-off-by: beorn7 --- promql/histogram_stats_iterator.go | 110 ++++++++++++++++------------- 1 file changed, 59 insertions(+), 51 deletions(-) diff --git a/promql/histogram_stats_iterator.go b/promql/histogram_stats_iterator.go index cbc717cac0..f5224825d3 100644 --- a/promql/histogram_stats_iterator.go +++ b/promql/histogram_stats_iterator.go @@ -46,127 +46,135 @@ func NewHistogramStatsIterator(it chunkenc.Iterator) *HistogramStatsIterator { // Reset resets this iterator for use with a new underlying iterator, reusing // objects already allocated where possible. -func (f *HistogramStatsIterator) Reset(it chunkenc.Iterator) { - f.Iterator = it - f.currentSeriesRead = false +func (hsi *HistogramStatsIterator) Reset(it chunkenc.Iterator) { + hsi.Iterator = it + hsi.currentSeriesRead = false } // AtHistogram returns the next timestamp/histogram pair. The counter reset // detection is guaranteed to be correct only when the caller does not switch // between AtHistogram and AtFloatHistogram calls. -func (f *HistogramStatsIterator) AtHistogram(h *histogram.Histogram) (int64, *histogram.Histogram) { +func (hsi *HistogramStatsIterator) AtHistogram(h *histogram.Histogram) (int64, *histogram.Histogram) { var t int64 - t, f.currentH = f.Iterator.AtHistogram(f.currentH) - if value.IsStaleNaN(f.currentH.Sum) { - h = &histogram.Histogram{Sum: f.currentH.Sum} + t, hsi.currentH = hsi.Iterator.AtHistogram(hsi.currentH) + if value.IsStaleNaN(hsi.currentH.Sum) { + h = &histogram.Histogram{Sum: hsi.currentH.Sum} return t, h } if h == nil { h = &histogram.Histogram{ - CounterResetHint: f.getResetHint(f.currentH), - Count: f.currentH.Count, - Sum: f.currentH.Sum, + CounterResetHint: hsi.getResetHint(hsi.currentH), + Count: hsi.currentH.Count, + Sum: hsi.currentH.Sum, } - f.setLastH(f.currentH) + hsi.setLastH(hsi.currentH) return t, h } returnValue := histogram.Histogram{ - CounterResetHint: f.getResetHint(f.currentH), - Count: f.currentH.Count, - Sum: f.currentH.Sum, + CounterResetHint: hsi.getResetHint(hsi.currentH), + Count: hsi.currentH.Count, + Sum: hsi.currentH.Sum, } returnValue.CopyTo(h) - f.setLastH(f.currentH) + hsi.setLastH(hsi.currentH) return t, h } // AtFloatHistogram returns the next timestamp/float histogram pair. The counter // reset detection is guaranteed to be correct only when the caller does not // switch between AtHistogram and AtFloatHistogram calls. -func (f *HistogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int64, *histogram.FloatHistogram) { +func (hsi *HistogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int64, *histogram.FloatHistogram) { var t int64 - t, f.currentFH = f.Iterator.AtFloatHistogram(f.currentFH) - if value.IsStaleNaN(f.currentFH.Sum) { - return t, &histogram.FloatHistogram{Sum: f.currentFH.Sum} + t, hsi.currentFH = hsi.Iterator.AtFloatHistogram(hsi.currentFH) + if value.IsStaleNaN(hsi.currentFH.Sum) { + return t, &histogram.FloatHistogram{Sum: hsi.currentFH.Sum} } if fh == nil { fh = &histogram.FloatHistogram{ - CounterResetHint: f.getFloatResetHint(f.currentFH.CounterResetHint), - Count: f.currentFH.Count, - Sum: f.currentFH.Sum, + CounterResetHint: hsi.getFloatResetHint(hsi.currentFH.CounterResetHint), + Count: hsi.currentFH.Count, + Sum: hsi.currentFH.Sum, } - f.setLastFH(f.currentFH) + hsi.setLastFH(hsi.currentFH) return t, fh } returnValue := histogram.FloatHistogram{ - CounterResetHint: f.getFloatResetHint(f.currentFH.CounterResetHint), - Count: f.currentFH.Count, - Sum: f.currentFH.Sum, + CounterResetHint: hsi.getFloatResetHint(hsi.currentFH.CounterResetHint), + Count: hsi.currentFH.Count, + Sum: hsi.currentFH.Sum, } returnValue.CopyTo(fh) - f.setLastFH(f.currentFH) + hsi.setLastFH(hsi.currentFH) return t, fh } -func (f *HistogramStatsIterator) setLastH(h *histogram.Histogram) { - f.lastFH = nil - if f.lastH == nil { - f.lastH = h.Copy() +func (hsi *HistogramStatsIterator) setLastH(h *histogram.Histogram) { + hsi.lastFH = nil + if hsi.lastH == nil { + hsi.lastH = h.Copy() } else { - h.CopyTo(f.lastH) + h.CopyTo(hsi.lastH) } - f.currentSeriesRead = true + hsi.currentSeriesRead = true } -func (f *HistogramStatsIterator) setLastFH(fh *histogram.FloatHistogram) { - f.lastH = nil - if f.lastFH == nil { - f.lastFH = fh.Copy() +func (hsi *HistogramStatsIterator) setLastFH(fh *histogram.FloatHistogram) { + hsi.lastH = nil + if hsi.lastFH == nil { + hsi.lastFH = fh.Copy() } else { - fh.CopyTo(f.lastFH) + fh.CopyTo(hsi.lastFH) } - f.currentSeriesRead = true + hsi.currentSeriesRead = true } -func (f *HistogramStatsIterator) getFloatResetHint(hint histogram.CounterResetHint) histogram.CounterResetHint { +func (hsi *HistogramStatsIterator) getFloatResetHint(hint histogram.CounterResetHint) histogram.CounterResetHint { if hint != histogram.UnknownCounterReset { return hint } - prevFH := f.lastFH - if prevFH == nil || !f.currentSeriesRead { - if f.lastH == nil || !f.currentSeriesRead { + prevFH := hsi.lastFH + if prevFH == nil || !hsi.currentSeriesRead { + if hsi.lastH == nil || !hsi.currentSeriesRead { // We don't know if there's a counter reset. return histogram.UnknownCounterReset } - prevFH = f.lastH.ToFloat(nil) + prevFH = hsi.lastH.ToFloat(nil) } - if f.currentFH.DetectReset(prevFH) { + if hsi.currentFH.DetectReset(prevFH) { return histogram.CounterReset } return histogram.NotCounterReset } -func (f *HistogramStatsIterator) getResetHint(h *histogram.Histogram) histogram.CounterResetHint { +func (hsi *HistogramStatsIterator) getResetHint(h *histogram.Histogram) histogram.CounterResetHint { if h.CounterResetHint != histogram.UnknownCounterReset { return h.CounterResetHint } var prevFH *histogram.FloatHistogram - if f.lastH == nil || !f.currentSeriesRead { - if f.lastFH == nil || !f.currentSeriesRead { - // We don't know if there's a counter reset. + if hsi.lastH == nil || !hsi.currentSeriesRead { + if hsi.lastFH == nil || !hsi.currentSeriesRead { + // We don't know if there's a counter reset. Note that + // this generally will trigger an explicit counter reset + // detection by the PromQL engine, which in turn isn't + // as reliable in this case because the PromQL engine + // will not see the buckets. However, we can assume that + // in cases where the counter reset detection is + // relevant, an iteration through the series has + // happened, and therefore we do not end up here in the + // first place. return histogram.UnknownCounterReset } - prevFH = f.lastFH + prevFH = hsi.lastFH } else { - prevFH = f.lastH.ToFloat(nil) + prevFH = hsi.lastH.ToFloat(nil) } fh := h.ToFloat(nil) if fh.DetectReset(prevFH) {