Skip to content

Scale zero activating timeout #4232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lvjing2 wants to merge 3 commits into from
Closed

Scale zero activating timeout #4232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
595a552
check activating timeout to scale zeor
lvjing2 Jun 4, 2019
a7f2118
not mark activating timeout
lvjing2 Jun 4, 2019
faa3dd5
check activating timeout for all
lvjing2 Jun 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/apis/autoscaling/v1alpha1/pa_lifecycle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ func (pas *PodAutoscalerStatus) CanMarkInactive(idlePeriod time.Duration) bool {
return pas.inStatusFor(corev1.ConditionTrue, idlePeriod)
}

// IsActivatingTimeout checks whether the pod autoscaler has been in activating
// for at least the specified timeout period.
func (pas *PodAutoscalerStatus) IsActivatingTimeout(activatingTimeout time.Duration) bool {
Copy link
Copy Markdown
Contributor

@vagababov vagababov Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably here and in the config-map, etc "Activation Timeout" is a more correct name.

return pas.inStatusFor(corev1.ConditionUnknown, activatingTimeout)
}

// inStatusFor returns true if the PodAutoscalerStatus's Active condition has stayed in
// the specified status for at least the specified duration. Otherwise it returns false,
// including when the status is undetermined (Active condition is not found.)
Expand Down
5 changes: 5 additions & 0 deletions pkg/autoscaler/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type Config struct {
TickInterval time.Duration

ScaleToZeroGracePeriod time.Duration
ActivatingTimeout time.Duration
}

// TargetConcurrency calculates the target concurrency for a given container-concurrency
Expand Down Expand Up @@ -142,6 +143,10 @@ func NewConfigFromMap(data map[string]string) (*Config, error) {
key: "tick-interval",
field: &lc.TickInterval,
defaultValue: 2 * time.Second,
}, {
key: "activating-timeout",
field: &lc.ActivatingTimeout,
defaultValue: 2 * time.Minute,
}} {
if raw, ok := data[dur.key]; !ok {
*dur.field = dur.defaultValue
Expand Down
12 changes: 12 additions & 0 deletions pkg/autoscaler/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func TestNewConfig(t *testing.T) {
"tick-interval": "2s",
"panic-window-percentage": "10",
"panic-threshold-percentage": "200",
"activating-timeout": "2m",
},
want: &Config{
EnableScaleToZero: true,
Expand All @@ -91,6 +92,7 @@ func TestNewConfig(t *testing.T) {
TickInterval: 2 * time.Second,
PanicWindowPercentage: 10.0,
PanicThresholdPercentage: 200.0,
ActivatingTimeout: 2 * time.Minute,
},
}, {
name: "with toggles on",
Expand All @@ -104,6 +106,7 @@ func TestNewConfig(t *testing.T) {
"tick-interval": "2s",
"panic-window-percentage": "10",
"panic-threshold-percentage": "200",
"activating-timeout": "2m",
},
want: &Config{
EnableScaleToZero: true,
Expand All @@ -116,6 +119,7 @@ func TestNewConfig(t *testing.T) {
TickInterval: 2 * time.Second,
PanicWindowPercentage: 10.0,
PanicThresholdPercentage: 200.0,
ActivatingTimeout: 2 * time.Minute,
},
}, {
name: "with toggles on strange casing",
Expand All @@ -129,6 +133,7 @@ func TestNewConfig(t *testing.T) {
"tick-interval": "2s",
"panic-window-percentage": "10",
"panic-threshold-percentage": "200",
"activating-timeout": "2m",
},
want: &Config{
EnableScaleToZero: true,
Expand All @@ -141,6 +146,7 @@ func TestNewConfig(t *testing.T) {
TickInterval: 2 * time.Second,
PanicWindowPercentage: 10.0,
PanicThresholdPercentage: 200.0,
ActivatingTimeout: 2 * time.Minute,
},
}, {
name: "with toggles explicitly off",
Expand All @@ -154,6 +160,7 @@ func TestNewConfig(t *testing.T) {
"tick-interval": "2s",
"panic-window-percentage": "10",
"panic-threshold-percentage": "200",
"activating-timeout": "2m",
},
want: &Config{
ContainerConcurrencyTargetPercentage: 0.5,
Expand All @@ -165,6 +172,7 @@ func TestNewConfig(t *testing.T) {
TickInterval: 2 * time.Second,
PanicWindowPercentage: 10.0,
PanicThresholdPercentage: 200.0,
ActivatingTimeout: 2 * time.Minute,
},
}, {
name: "with explicit grace period",
Expand All @@ -179,6 +187,7 @@ func TestNewConfig(t *testing.T) {
"tick-interval": "2s",
"panic-window-percentage": "10",
"panic-threshold-percentage": "200",
"activating-timeout": "2m",
},
want: &Config{
ContainerConcurrencyTargetPercentage: 0.5,
Expand All @@ -190,6 +199,7 @@ func TestNewConfig(t *testing.T) {
TickInterval: 2 * time.Second,
PanicWindowPercentage: 10.0,
PanicThresholdPercentage: 200.0,
ActivatingTimeout: 2 * time.Minute,
},
}, {
name: "malformed float",
Expand All @@ -202,6 +212,7 @@ func TestNewConfig(t *testing.T) {
"tick-interval": "2s",
"panic-window-percentage": "10",
"panic-threshold-percentage": "200",
"activating-timeout": "2m",
},
wantErr: true,
}, {
Expand All @@ -215,6 +226,7 @@ func TestNewConfig(t *testing.T) {
"tick-interval": "2s",
"panic-window-percentage": "10",
"panic-threshold-percentage": "200",
"activating-timeout": "2m",
},
wantErr: true,
}}
Expand Down
6 changes: 6 additions & 0 deletions pkg/reconciler/autoscaling/kpa/kpa.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ func (c *Reconciler) reconcile(ctx context.Context, pa *pav1alpha1.PodAutoscaler
return perrors.Wrap(err, "error reporting metrics")
}

if pa.Status.IsActivating() {
timeout := config.FromContext(ctx).Autoscaler.ActivatingTimeout
logger.Infof("enqueue delay for check activating timeout after %v.", timeout)
Copy link
Copy Markdown
Contributor

@vagababov vagababov Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging statement.

Suggested change
logger.Infof("enqueue delay for check activating timeout after %v.", timeout)
logger.Infof("Enqueue with delay to check activation timeout after %v.", timeout)

c.scaler.enqueueCB(pa, timeout)
}

// computeActiveCondition decides if we need to change the SKS mode,
// and returns true if the status has changed.
if changed := computeActiveCondition(pa, want, got); changed {
Expand Down
12 changes: 9 additions & 3 deletions pkg/reconciler/autoscaling/kpa/scaler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,19 @@ func (ks *scaler) applyScale(ctx context.Context, pa *pav1alpha1.PodAutoscaler,
// Scale attempts to scale the given PA's target reference to the desired scale.
func (ks *scaler) Scale(ctx context.Context, pa *pav1alpha1.PodAutoscaler, desiredScale int32) (int32, error) {
logger := logging.FromContext(ctx)
autoscalerConfig := config.FromContext(ctx).Autoscaler

if desiredScale < 0 {
logger.Debug("Metrics are not yet being collected.")
return desiredScale, nil
// check is activating timeout
if pa.Status.IsActivatingTimeout(autoscalerConfig.ActivatingTimeout) {
Copy link
Copy Markdown
Contributor

@vagababov vagababov Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that this is correct. It should not really matter in the negative case at all.

logger.Infof("Activating pa timeout after %v", autoscalerConfig.ActivatingTimeout)
} else {
logger.Debug("Metrics are not yet being collected.")
return desiredScale, nil
}
}

desiredScale, shouldApplyScale := ks.handleScaleToZero(pa, desiredScale, config.FromContext(ctx).Autoscaler)
desiredScale, shouldApplyScale := ks.handleScaleToZero(pa, desiredScale, autoscalerConfig)
if !shouldApplyScale {
return desiredScale, nil
}
Expand Down