Fix possible overflow in unavailability estimator

gccontrol/interceptor.go

@@ -11,7 +11,8 @@ const (
 	waitForTrailers = 10 * time.Millisecond
 	// The arrival rate and the amount of resources to compute each request can vary
 	// a lot over time, keeping a long history might not improve the decision.
-	sampleSize = 5
+	sampleSize             = 5
+	unavailabilityPastSize = 5
 )
 
 // ShedResponse the response of processing a single request from GCInterceptor.
@@ -34,7 +35,7 @@ func New() *Interceptor {
 	debug.SetGCPercent(-1)
 	return &Interceptor{
 		sampler:   newSampler(sampleSize),
-		estimator: newUnavailabilityEstimator(),
+		estimator: newUnavailabilityEstimator(unavailabilityPastSize),
 		heap:      newHeap(),
 	}
 }

gccontrol/unavailability.go

@@ -5,30 +5,29 @@ import (
 	"time"
 )
 
-func newUnavailabilityEstimator() *unavailabilityEstimator {
+func newUnavailabilityEstimator(size int) *unavailabilityEstimator {
 	return &unavailabilityEstimator{
-		gcPast:  []time.Duration{0, 0, 0, 0, 0},
-		reqPast: []time.Duration{0, 0, 0, 0, 0},
+		gcPast:  make([]time.Duration, size),
+		reqPast: make([]time.Duration, size),
 	}
 }
 
 type unavailabilityEstimator struct {
-	gcCount      int
-	gcEstimation time.Duration
-	gcStart      time.Time
-	gcPast       []time.Duration
+	gcNext       int             // Next index in gcPast.
+	gcEstimation time.Duration   // Current estimation of the next GC duration.
+	gcStart      time.Time       // Last GC start time.
+	gcPast       []time.Duration // History of garbage collection duration estimations.
 
-	reqCount        int
-	reqPast         []time.Duration
-	reqMean, reqVar float64 // Request statistics in nanoseconds. Made them float to make math easier.
-	reqEstimation   time.Duration
+	reqCount        int             // Number of requests finished since last collection.
+	reqPast         []time.Duration // History of request duration estimations.
+	reqMean, reqVar float64         // Request statistics in nanoseconds. Made them float to make math easier.
+	reqEstimation   time.Duration   // Current request duration estimation.
 }
 
 func (u *unavailabilityEstimator) gcFinished(d time.Duration) {
 	// Estimate GC duration.
-	u.gcPast[u.gcCount%len(u.gcPast)] = d
+	u.gcPast[u.gcNext] = d
 	u.gcEstimation = maxDuration(u.gcPast)
-	u.gcCount++
 
 	// Estimate the time processing a request.
 	// Using 68–95–99.7 rule to have a good coverage on the request size.
@@ -37,11 +36,12 @@ func (u *unavailabilityEstimator) gcFinished(d time.Duration) {
 	if u.reqCount > 1 {
 		stdDev = math.Sqrt(u.reqVar) / float64(u.reqCount-1)
 	}
-	u.reqPast[u.gcCount%len(u.reqPast)] = time.Duration(u.reqMean+(3*stdDev)) * time.Nanosecond
+	u.reqPast[u.gcNext] = time.Duration(u.reqMean+(3*stdDev)) * time.Nanosecond
 	u.reqEstimation = maxDuration(u.reqPast)
 	u.reqCount = 0
 	u.reqMean = 0
 	u.reqVar = 0
+	u.gcNext = (u.gcNext + 1) % len(u.gcPast)
 }
 
 func (u *unavailabilityEstimator) estimate(queueSize int64) time.Duration {

gccontrol/unavailability_test.go

@@ -0,0 +1,30 @@
+package gccontrol
+
+import (
+	"testing"
+
+	"github.com/matryer/is"
+)
+import "time"
+
+func TestUnavailabilityEstimator(t *testing.T) {
+	is := is.New(t)
+	e := newUnavailabilityEstimator(3)
+
+	e.requestFinished(2 * time.Millisecond)
+	e.requestFinished(2 * time.Millisecond)
+	e.gcFinished(3 * time.Second)
+	is.Equal(3*time.Second+10*2*time.Millisecond, e.estimate(10))
+
+	// Estimations consider always the greater request and gc durations in the window (the previous one).
+	e.requestFinished(1 * time.Millisecond)
+	e.gcFinished(2 * time.Second)
+	is.Equal(3*time.Second+10*2*time.Millisecond, e.estimate(10))
+
+	// Past window has restarted.
+	for i := 0; i < 3; i++ {
+		e.requestFinished(1 * time.Millisecond)
+		e.gcFinished(1 * time.Second)
+	}
+	is.Equal(1*time.Second+5*1*time.Millisecond, e.estimate(5))
+}