review comments

lib/streamaggr/dedup.go

@@ -72,7 +72,7 @@ func (da *dedupAggr) itemsCount() uint64 {
 	return n
 }
 
-func (da *dedupAggr) pushSamples(samples []pushSample, dedupIdx int) {
+func (da *dedupAggr) pushSamples(samples []pushSample, _ int64, dedupIdx int) {
 	pss := getPerShardSamples()
 	shards := pss.shards
 	for _, sample := range samples {

lib/streamaggr/dedup_test.go

@@ -21,7 +21,7 @@ func TestDedupAggrSerial(t *testing.T) {
 			sample.value = float64(i + j)
 			expectedSamplesMap[sample.key] = *sample
 		}
-		da.pushSamples(samples, 0)
+		da.pushSamples(samples, 0, 0)
 	}
 
 	if n := da.sizeBytes(); n > 5_000_000 {
@@ -73,7 +73,7 @@ func TestDedupAggrConcurrent(_ *testing.T) {
 					sample.key = fmt.Sprintf("key_%d", j)
 					sample.value = float64(i + j)
 				}
-				da.pushSamples(samples, 0)
+				da.pushSamples(samples, 0, 0)
 			}
 		}()
 	}

lib/streamaggr/dedup_timing_test.go

@@ -27,7 +27,7 @@ func benchmarkDedupAggr(b *testing.B, samplesPerPush int) {
 	b.RunParallel(func(pb *testing.PB) {
 		for pb.Next() {
 			for i := 0; i < loops; i++ {
-				da.pushSamples(benchSamples, 0)
+				da.pushSamples(benchSamples, 0, 0)
 			}
 		}
 	})

lib/streamaggr/deduplicator.go

@@ -118,7 +118,7 @@ func (d *Deduplicator) Push(tss []prompbmarshal.TimeSeries) {
 	}
 
 	for idx, ps := range pss {
-		d.da.pushSamples(ps, idx)
+		d.da.pushSamples(ps, 0, idx)
 	}
 
 	ctx.pss = pss

lib/streamaggr/rate.go

@@ -136,8 +136,8 @@ func (as *rateAggrState) flushState(ctx *flushCtx) {
 		}
 
 		// Delete outdated entries in state
-		var rate float64
-		var totalItems int
+		rate := 0.0
+		countSeries := 0
 		for k1, state := range sv.state {
 			if ctx.flushTimestamp > state.deleteDeadline {
 				delete(sv.state, k1)
@@ -156,7 +156,7 @@ func (as *rateAggrState) flushState(ctx *flushCtx) {
 				rate += (v1.total) * 1000 / float64(rateInterval)
 				state.prevTimestamp = v1.timestamp
 				state.prevValue = v1.value
-				totalItems++
+				countSeries++
 			}
 			state.lastValues[ctx.idx] = rateLastValueState{}
 			sv.state[k1] = state
@@ -164,9 +164,9 @@ func (as *rateAggrState) flushState(ctx *flushCtx) {
 
 		sv.mu.Unlock()
 
-		if totalItems > 0 {
+		if countSeries > 0 {
 			if as.isAvg {
-				rate /= float64(totalItems)
+				rate /= float64(countSeries)
 			}
 			key := k.(string)
 			ctx.appendSeries(key, suffix, rate)

lib/streamaggr/streamaggr.go

@@ -801,7 +801,7 @@ func (a *aggregator) runFlusher(pushFunc PushFunc, alignFlushToInterval, skipInc
 
 		dedupTime := ct.Truncate(tickInterval)
 		if a.ignoreOldSamples {
-			dedupIdx, flushIdx = a.getFlushIndices(dedupTime, flushDeadline)
+			dedupIdx, flushIdx = a.getAggrIdxs(dedupTime, flushDeadline)
 		}
 		pf := pushFunc
 
@@ -848,28 +848,24 @@ func (a *aggregator) runFlusher(pushFunc PushFunc, alignFlushToInterval, skipInc
 	if !skipIncompleteFlush && ignoreFirstIntervals <= 0 {
 		dedupTime := time.Now().Truncate(tickInterval).Add(tickInterval)
 		if a.ignoreOldSamples {
-			dedupIdx, flushIdx = a.getFlushIndices(dedupTime, flushDeadline)
+			dedupIdx, flushIdx = a.getAggrIdxs(dedupTime, flushDeadline)
 		}
 		a.dedupFlush(flushDeadline.UnixMilli(), dedupIdx, flushIdx)
 		a.flush(pushFunc, flushDeadline.UnixMilli(), flushIdx)
 	}
 }
 
-func (a *aggregator) getFlushIndices(dedupTime, flushTime time.Time) (int, int) {
-	flushTimestamp := flushTime.UnixMilli()
-	flushIntervals := int(flushTimestamp / int64(a.interval/time.Millisecond))
-	var dedupIndex, flushIndex int
+func (a *aggregator) getAggrIdxs(dedupTime, flushTime time.Time) (int, int) {
+	flushIdx := getStateIdx(a.interval.Milliseconds(), flushTime.Add(-a.interval).UnixMilli())
+	dedupIdx := flushIdx
 	if a.dedupInterval > 0 {
-		dedupTimestamp := dedupTime.UnixMilli()
-		dedupIntervals := int(dedupTimestamp / int64(a.dedupInterval/time.Millisecond))
-		intervalsRatio := int(a.interval / a.dedupInterval)
-		dedupIndex = dedupIntervals % aggrStateSize
-		flushIndex = flushIntervals % (aggrStateSize / intervalsRatio)
-	} else {
-		flushIndex = flushIntervals % aggrStateSize
-		dedupIndex = flushIndex
+		dedupIdx = getStateIdx(a.dedupInterval.Milliseconds(), dedupTime.Add(-a.dedupInterval).UnixMilli())
 	}
-	return dedupIndex, flushIndex
+	return dedupIdx, flushIdx
+}
+
+func getStateIdx(interval int64, ts int64) int {
+	return int(ts/interval) % aggrStateSize
 }
 
 func (a *aggregator) dedupFlush(deleteDeadline int64, dedupIdx, flushIdx int) {
@@ -1000,7 +996,6 @@ func (a *aggregator) Push(tss []prompbmarshal.TimeSeries, matchIdxs []byte) {
 				continue
 			}
 			if ignoreOldSamples && s.Timestamp < minTimestamp {
-				a.ignoredOldSamples.Inc()
 				// Skip old samples outside the current aggregation interval
 				a.ignoredOldSamples.Inc()
 				continue
@@ -1010,7 +1005,7 @@ func (a *aggregator) Push(tss []prompbmarshal.TimeSeries, matchIdxs []byte) {
 				maxLagMsec = lagMsec
 			}
 			if ignoreOldSamples {
-				flushIdx = int((s.Timestamp)/a.tickInterval+1) % aggrStateSize
+				flushIdx = getStateIdx(a.tickInterval, s.Timestamp)
 			}
 			samples[flushIdx] = append(samples[flushIdx], pushSample{
 				key:       key,
@@ -1019,20 +1014,20 @@ func (a *aggregator) Push(tss []prompbmarshal.TimeSeries, matchIdxs []byte) {
 			})
 		}
 	}
-	if len(samples) > 0 {
-		a.matchedSamples.Add(len(samples))
-		a.samplesLag.Update(float64(maxLagMsec) / 1_000)
-	}
+
 	ctx.samples = samples
 	ctx.buf = buf
 
+	pushSamples := a.pushSamples
 	if a.da != nil {
-		for idx, s := range samples {
-			a.da.pushSamples(s, idx)
-		}
-	} else {
-		for idx, s := range samples {
-			a.pushSamples(s, deleteDeadlineMsec, idx)
+		pushSamples = a.da.pushSamples
+	}
+
+	for idx, s := range samples {
+		if len(s) > 0 {
+			a.samplesLag.Update(float64(maxLagMsec) / 1_000)
+			a.matchedSamples.Add(len(s))
+			pushSamples(s, deleteDeadlineMsec, idx)
 		}
 	}
 }

lib/streamaggr/unique_samples.go

@@ -79,10 +79,8 @@ func (as *uniqueSamplesAggrState) flushState(ctx *flushCtx) {
 		state := len(sv.state[ctx.idx])
 		sv.state[ctx.idx] = make(map[float64]struct{})
 		sv.mu.Unlock()
-		if state > 0 {
-			key := k.(string)
-			ctx.appendSeries(key, "unique_samples", float64(state))
-		}
+		key := k.(string)
+		ctx.appendSeries(key, "unique_samples", float64(state))
 		return true
 	})
 }