Handle InitialScale != 1 when creating multiScaler and some tidy-ups

pkg/autoscaler/scaling/autoscaler.go

@@ -242,7 +242,7 @@ func (a *autoscaler) Scale(ctx context.Context, now time.Time) ScaleResult {
 
 	// Here we compute two numbers: excess burst capacity and number of activators
 	// for subsetting.
-	// - the excess burst capacity based on panic value, since we don't want to
+	// - the excess burst capacity is based on panic value, since we don't want to
 	//   be making knee-jerk decisions about Activator in the request path.
 	//   Negative EBC means that the deployment does not have enough capacity to serve
 	//   the desired burst off hand.
@@ -252,15 +252,15 @@ func (a *autoscaler) Scale(ctx context.Context, now time.Time) ScaleResult {
 	//   the default number.
 	//   if tbc > 0, then revision gets number of activators to support total capacity and
 	//   tbc additional units.
-	//   if tbc==-1, then revision gets to number of activators to support total capacity.
+	//   if tbc==-1, then revision gets the number of activators needed to support total capacity.
 	//   With default target utilization of 0.7, we're overprovisioning number of needed activators
 	//   by rate of 1/0.7=1.42.
 	excessBCF := -1.
 	numAct := int32(MinActivators)
 	switch {
 	case a.deciderSpec.TargetBurstCapacity == 0:
 		excessBCF = 0
-		// numAct stays 1, only needed to scale from 0.
+		// numAct stays at MinActivators, only needed to scale from 0.
 	case a.deciderSpec.TargetBurstCapacity > 0:
 		// Extra float64 cast disables fused multiply-subtract to force identical behavior on
 		// all platforms. See floating point section in https://golang.org/ref/spec#Operators.
@@ -272,7 +272,7 @@ func (a *autoscaler) Scale(ctx context.Context, now time.Time) ScaleResult {
 		numAct = int32(math.Max(MinActivators,
 			math.Ceil(float64(originalReadyPodsCount)*a.deciderSpec.TotalValue/a.deciderSpec.ActivatorCapacity)))
 	}
-	logger.Debugf("PodCount=%v Total1PodCapacity=%v ObsStableValue=%v ObsPanicValue=%v TargetBC=%v ExcessBC=%v NumActivators=%d",
+	logger.Debugf("PodCount=%d Total1PodCapacity=%0.3f ObsStableValue=%0.3f ObsPanicValue=%0.3f TargetBC=%0.3f ExcessBC=%0.3f NumActivators=%d",
 		originalReadyPodsCount, a.deciderSpec.TotalValue, observedStableValue,
 		observedPanicValue, a.deciderSpec.TargetBurstCapacity, excessBCF, numAct)
 

pkg/autoscaler/scaling/multiscaler.go

@@ -45,7 +45,7 @@ type Decider struct {
 	Status DeciderStatus
 }
 
-// DeciderSpec is the parameters in which the Revision should scaled.
+// DeciderSpec is the parameters by which the Revision should be scaled.
 type DeciderSpec struct {
 	MaxScaleUpRate   float64
 	MaxScaleDownRate float64
@@ -159,26 +159,27 @@ func (sr *scalerRunner) updateLatestScale(sRes ScaleResult) bool {
 		ret = true
 	}
 
-	// If sign has changed -- then we have to update KPA
+	// If sign has changed -- then we have to update KPA.
 	ret = ret || !sameSign(sr.decider.Status.ExcessBurstCapacity, sRes.ExcessBurstCapacity)
 
 	// Update with the latest calculation anyway.
 	sr.decider.Status.ExcessBurstCapacity = sRes.ExcessBurstCapacity
 	return ret
 }
 
-// MultiScaler maintains a collection of Uniscalers.
+// MultiScaler maintains a collection of UniScalers.
 type MultiScaler struct {
-	scalers       map[types.NamespacedName]*scalerRunner
-	scalersMutex  sync.RWMutex
+	scalersMutex sync.RWMutex
+	scalers      map[types.NamespacedName]*scalerRunner
+
 	scalersStopCh <-chan struct{}
 
 	uniScalerFactory UniScalerFactory
 
 	logger *zap.SugaredLogger
 
-	watcher      func(types.NamespacedName)
 	watcherMutex sync.RWMutex
+	watcher      func(types.NamespacedName)
 
 	tickProvider func(time.Duration) *time.Ticker
 }
@@ -234,7 +235,7 @@ func (m *MultiScaler) Create(ctx context.Context, decider *Decider) (*Decider, e
 	return scaler.decider, nil
 }
 
-// Update applied the desired DeciderSpec to a currently running Decider.
+// Update applies the desired DeciderSpec to a currently running Decider.
 func (m *MultiScaler) Update(_ context.Context, decider *Decider) (*Decider, error) {
 	key := types.NamespacedName{Namespace: decider.Namespace, Name: decider.Name}
 	m.scalersMutex.Lock()
@@ -324,10 +325,9 @@ func (m *MultiScaler) createScaler(ctx context.Context, decider *Decider) (*scal
 	case -1, 0:
 		d.Status.ExcessBurstCapacity = int32(tbc)
 	default:
-		// If TBC > Target * InitialScale (currently 1), then we know initial
+		// If TBC > Target * InitialScale, then we know initial
 		// scale won't be enough to cover TBC and we'll be behind activator.
-		// TODO(autoscale-wg): fix this when we switch to non "1" initial scale.
-		d.Status.ExcessBurstCapacity = int32(1*d.Spec.TotalValue - tbc)
+		d.Status.ExcessBurstCapacity = int32(float64(d.Spec.InitialScale)*d.Spec.TotalValue - tbc)
 	}
 
 	m.runScalerTicker(ctx, runner)

pkg/autoscaler/scaling/multiscaler_test.go

@@ -50,7 +50,7 @@ func watchFunc(ctx context.Context, ms *MultiScaler, decider *Decider, desiredSc
 			return
 		}
 		if got, want := m.Status.DesiredScale, int32(desiredScale); got != want {
-			errCh <- fmt.Errorf("Get() = %v, wanted %v", got, want)
+			errCh <- fmt.Errorf("DesiredScale = %v, wanted %v", got, want)
 			return
 		}
 		errCh <- nil
@@ -114,10 +114,10 @@ func TestMultiScalerScaling(t *testing.T) {
 		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
 	}
 	if got, want := d.Status.ExcessBurstCapacity, int32(0); got != want {
-		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
+		t.Errorf("Decider.Status.ExcessBurstCapacity = %d, want: %d", got, want)
 	}
 	if got, want := d.Status.NumActivators, int32(0); got != want {
-		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
+		t.Errorf("Decider.Status.NumActivators = %d, want: %d", got, want)
 	}
 
 	// Verify that we see a "tick"
@@ -135,10 +135,10 @@ func TestMultiScalerScaling(t *testing.T) {
 		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
 	}
 	if got, want := d.Status.ExcessBurstCapacity, int32(1); got != want {
-		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
+		t.Errorf("Decider.Status.ExcessBurstCapacity = %d, want: %d", got, want)
 	}
 	if got, want := d.Status.NumActivators, int32(2); got != want {
-		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
+		t.Errorf("Decider.Status.NumActivators = %d, want: %d", got, want)
 	}
 
 	// Change number of activators, keeping the other data the same. E.g. CM
@@ -158,10 +158,10 @@ func TestMultiScalerScaling(t *testing.T) {
 		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
 	}
 	if got, want := d.Status.ExcessBurstCapacity, int32(1); got != want {
-		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
+		t.Errorf("Decider.Status.ExcessBurstCapacity = %d, want: %d", got, want)
 	}
 	if got, want := d.Status.NumActivators, int32(3); got != want {
-		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
+		t.Errorf("Decider.Status.NumActivators = %d, want: %d", got, want)
 	}
 
 	// Verify that subsequent "ticks" don't trigger a callback, since
@@ -188,6 +188,7 @@ func TestMultiscalerCreateTBC42(t *testing.T) {
 	ms, _ := createMultiScaler(ctx, TestLogger(t))
 
 	decider := newDecider()
+	decider.Spec.InitialScale = 2
 	decider.Spec.TargetBurstCapacity = 42
 	decider.Spec.TotalValue = 25
 
@@ -202,8 +203,8 @@ func TestMultiscalerCreateTBC42(t *testing.T) {
 	if got, want := d.Status.DesiredScale, int32(-1); got != want {
 		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
 	}
-	if got, want := d.Status.ExcessBurstCapacity, int32(25-42); got != want {
-		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
+	if got, want := d.Status.ExcessBurstCapacity, int32(50-42); got != want {
+		t.Errorf("Decider.Status.ExcessBurstCapacity = %d, want: %d", got, want)
 	}
 	if err := ms.Delete(ctx, decider.Namespace, decider.Name); err != nil {
 		t.Errorf("Delete() = %v", err)
@@ -230,7 +231,7 @@ func TestMultiscalerCreateTBCMinus1(t *testing.T) {
 		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
 	}
 	if got, want := d.Status.ExcessBurstCapacity, int32(-1); got != want {
-		t.Errorf("Decider.Status.DesiredScale = %d, want: %d", got, want)
+		t.Errorf("Decider.Status.ExcessBurstCapacity = %d, want: %d", got, want)
 	}
 	if err := ms.Delete(ctx, decider.Namespace, decider.Name); err != nil {
 		t.Errorf("Delete() = %v", err)

pkg/reconciler/autoscaling/kpa/scaler.go

@@ -319,7 +319,7 @@ func (ks *scaler) scale(ctx context.Context, pa *pav1alpha1.PodAutoscaler, sks *
 	min, max := pa.ScaleBounds()
 	initialScale := kparesources.GetInitialScale(config.FromContext(ctx).Autoscaler, pa)
 	if initialScale > 1 {
-		// Ignore initial scale if minScale >= initialScale
+		// Ignore initial scale if minScale >= initialScale.
 		if min < initialScale {
 			logger.Debugf("Adjusting min to meet the initial scale: %d -> %d", min, initialScale)
 		}