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 <beorn@grafana.com>

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) {