diff --git a/pkg/autoscaler/scaling/autoscaler.go b/pkg/autoscaler/scaling/autoscaler.go index 94d49caa88d7..4cd240a063c8 100644 --- a/pkg/autoscaler/scaling/autoscaler.go +++ b/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,7 +252,7 @@ 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. @@ -260,7 +260,7 @@ func (a *autoscaler) Scale(ctx context.Context, now time.Time) ScaleResult { 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) diff --git a/pkg/autoscaler/scaling/multiscaler.go b/pkg/autoscaler/scaling/multiscaler.go index 638b94ae6399..0e06342a21cf 100644 --- a/pkg/autoscaler/scaling/multiscaler.go +++ b/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,7 +159,7 @@ 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. @@ -167,18 +167,19 @@ func (sr *scalerRunner) updateLatestScale(sRes ScaleResult) bool { 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) diff --git a/pkg/autoscaler/scaling/multiscaler_test.go b/pkg/autoscaler/scaling/multiscaler_test.go index ac3748b416aa..058e78169935 100644 --- a/pkg/autoscaler/scaling/multiscaler_test.go +++ b/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) diff --git a/pkg/reconciler/autoscaling/kpa/scaler.go b/pkg/reconciler/autoscaling/kpa/scaler.go index afc5a81a2edc..4b75a7dacbb6 100644 --- a/pkg/reconciler/autoscaling/kpa/scaler.go +++ b/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) }