From 16133c2b773a223468aae42466322b1920c55208 Mon Sep 17 00:00:00 2001 From: Joseph Burnett Date: Wed, 16 Dec 2020 16:55:44 +0100 Subject: [PATCH] Up and down scale stabilize with envelope. The HPA controller keeps a flat history of recommendations for stabilization. However when both up and down scale stabilization are configured, the interpretation of the history changes depending on the direction of movement. What we want is to keep the stabilized recommendation within the envelope of the minimum and maximum over configured stabilization windows. We should only move when the envelope forces a move. --- pkg/controller/podautoscaler/horizontal.go | 58 +++++--- .../podautoscaler/horizontal_test.go | 129 +++++++++++------- 2 files changed, 114 insertions(+), 73 deletions(-) diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go index 59893133e13..f83a50681ea 100644 --- a/pkg/controller/podautoscaler/horizontal.go +++ b/pkg/controller/podautoscaler/horizontal.go @@ -874,44 +874,58 @@ func (a *HorizontalController) storeScaleEvent(behavior *autoscalingv2.Horizonta // - replaces old recommendation with the newest recommendation, // - returns {max,min} of recommendations that are not older than constraints.Scale{Up,Down}.DelaySeconds func (a *HorizontalController) stabilizeRecommendationWithBehaviors(args NormalizationArg) (int32, string, string) { - recommendation := args.DesiredReplicas + now := time.Now() + foundOldSample := false oldSampleIndex := 0 - var scaleDelaySeconds int32 - var reason, message string - var betterRecommendation func(int32, int32) int32 + upRecommendation := args.DesiredReplicas + upDelaySeconds := *args.ScaleUpBehavior.StabilizationWindowSeconds + upCutoff := now.Add(-time.Second * time.Duration(upDelaySeconds)) - if args.DesiredReplicas >= args.CurrentReplicas { - scaleDelaySeconds = *args.ScaleUpBehavior.StabilizationWindowSeconds - betterRecommendation = min - reason = "ScaleUpStabilized" - message = "recent recommendations were lower than current one, applying the lowest recent recommendation" - } else { - scaleDelaySeconds = *args.ScaleDownBehavior.StabilizationWindowSeconds - betterRecommendation = max - reason = "ScaleDownStabilized" - message = "recent recommendations were higher than current one, applying the highest recent recommendation" - } + downRecommendation := args.DesiredReplicas + downDelaySeconds := *args.ScaleDownBehavior.StabilizationWindowSeconds + downCutoff := now.Add(-time.Second * time.Duration(downDelaySeconds)) - maxDelaySeconds := max(*args.ScaleUpBehavior.StabilizationWindowSeconds, *args.ScaleDownBehavior.StabilizationWindowSeconds) - obsoleteCutoff := time.Now().Add(-time.Second * time.Duration(maxDelaySeconds)) - - cutoff := time.Now().Add(-time.Second * time.Duration(scaleDelaySeconds)) + // Calculate the upper and lower stabilization limits. for i, rec := range a.recommendations[args.Key] { - if rec.timestamp.After(cutoff) { - recommendation = betterRecommendation(rec.recommendation, recommendation) + if rec.timestamp.After(upCutoff) { + upRecommendation = min(rec.recommendation, upRecommendation) } - if rec.timestamp.Before(obsoleteCutoff) { + if rec.timestamp.After(downCutoff) { + downRecommendation = max(rec.recommendation, downRecommendation) + } + if rec.timestamp.Before(upCutoff) && rec.timestamp.Before(downCutoff) { foundOldSample = true oldSampleIndex = i } } + + // Bring the recommendation to within the upper and lower limits (stabilize). + recommendation := args.CurrentReplicas + if recommendation < upRecommendation { + recommendation = upRecommendation + } + if recommendation > downRecommendation { + recommendation = downRecommendation + } + + // Record the unstabilized recommendation. if foundOldSample { a.recommendations[args.Key][oldSampleIndex] = timestampedRecommendation{args.DesiredReplicas, time.Now()} } else { a.recommendations[args.Key] = append(a.recommendations[args.Key], timestampedRecommendation{args.DesiredReplicas, time.Now()}) } + + // Determine a human-friendly message. + var reason, message string + if args.DesiredReplicas >= args.CurrentReplicas { + reason = "ScaleUpStabilized" + message = "recent recommendations were lower than current one, applying the lowest recent recommendation" + } else { + reason = "ScaleDownStabilized" + message = "recent recommendations were higher than current one, applying the highest recent recommendation" + } return recommendation, reason, message } diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go index c69aa7c268b..347b306acad 100644 --- a/pkg/controller/podautoscaler/horizontal_test.go +++ b/pkg/controller/podautoscaler/horizontal_test.go @@ -3848,6 +3848,7 @@ func TestStoreScaleEvents(t *testing.T) { } func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) { + now := time.Now() type TestCase struct { name string key string @@ -3868,22 +3869,22 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) { prenormalizedDesiredReplicas: 5, expectedStabilizedReplicas: 5, expectedRecommendations: []timestampedRecommendation{ - {5, time.Now()}, + {5, now}, }, }, { name: "simple scale down stabilization", key: "", recommendations: []timestampedRecommendation{ - {4, time.Now().Add(-2 * time.Minute)}, - {5, time.Now().Add(-1 * time.Minute)}}, + {4, now.Add(-2 * time.Minute)}, + {5, now.Add(-1 * time.Minute)}}, currentReplicas: 100, prenormalizedDesiredReplicas: 3, expectedStabilizedReplicas: 5, expectedRecommendations: []timestampedRecommendation{ - {4, time.Now()}, - {5, time.Now()}, - {3, time.Now()}, + {4, now}, + {5, now}, + {3, now}, }, scaleDownStabilizationWindowSeconds: 60 * 3, }, @@ -3891,15 +3892,15 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) { name: "simple scale up stabilization", key: "", recommendations: []timestampedRecommendation{ - {4, time.Now().Add(-2 * time.Minute)}, - {5, time.Now().Add(-1 * time.Minute)}}, + {4, now.Add(-2 * time.Minute)}, + {5, now.Add(-1 * time.Minute)}}, currentReplicas: 1, prenormalizedDesiredReplicas: 7, expectedStabilizedReplicas: 4, expectedRecommendations: []timestampedRecommendation{ - {4, time.Now()}, - {5, time.Now()}, - {7, time.Now()}, + {4, now}, + {5, now}, + {7, now}, }, scaleUpStabilizationWindowSeconds: 60 * 5, }, @@ -3907,15 +3908,15 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) { name: "no scale down stabilization", key: "", recommendations: []timestampedRecommendation{ - {1, time.Now().Add(-2 * time.Minute)}, - {2, time.Now().Add(-1 * time.Minute)}}, + {1, now.Add(-2 * time.Minute)}, + {2, now.Add(-1 * time.Minute)}}, currentReplicas: 100, // to apply scaleDown delay we should have current > desired prenormalizedDesiredReplicas: 3, expectedStabilizedReplicas: 3, expectedRecommendations: []timestampedRecommendation{ - {1, time.Now()}, - {2, time.Now()}, - {3, time.Now()}, + {1, now}, + {2, now}, + {3, now}, }, scaleUpStabilizationWindowSeconds: 60 * 5, }, @@ -3923,15 +3924,15 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) { name: "no scale up stabilization", key: "", recommendations: []timestampedRecommendation{ - {4, time.Now().Add(-2 * time.Minute)}, - {5, time.Now().Add(-1 * time.Minute)}}, + {4, now.Add(-2 * time.Minute)}, + {5, now.Add(-1 * time.Minute)}}, currentReplicas: 1, // to apply scaleDown delay we should have current > desired prenormalizedDesiredReplicas: 3, expectedStabilizedReplicas: 3, expectedRecommendations: []timestampedRecommendation{ - {4, time.Now()}, - {5, time.Now()}, - {3, time.Now()}, + {4, now}, + {5, now}, + {3, now}, }, scaleDownStabilizationWindowSeconds: 60 * 5, }, @@ -3939,46 +3940,46 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) { name: "no scale down stabilization, reuse recommendation element", key: "", recommendations: []timestampedRecommendation{ - {10, time.Now().Add(-10 * time.Minute)}, - {9, time.Now().Add(-9 * time.Minute)}}, + {10, now.Add(-10 * time.Minute)}, + {9, now.Add(-9 * time.Minute)}}, currentReplicas: 100, // to apply scaleDown delay we should have current > desired prenormalizedDesiredReplicas: 3, expectedStabilizedReplicas: 3, expectedRecommendations: []timestampedRecommendation{ - {10, time.Now()}, - {3, time.Now()}, + {10, now}, + {3, now}, }, }, { name: "no scale up stabilization, reuse recommendation element", key: "", recommendations: []timestampedRecommendation{ - {10, time.Now().Add(-10 * time.Minute)}, - {9, time.Now().Add(-9 * time.Minute)}}, + {10, now.Add(-10 * time.Minute)}, + {9, now.Add(-9 * time.Minute)}}, currentReplicas: 1, prenormalizedDesiredReplicas: 100, expectedStabilizedReplicas: 100, expectedRecommendations: []timestampedRecommendation{ - {10, time.Now()}, - {100, time.Now()}, + {10, now}, + {100, now}, }, }, { name: "scale down stabilization, reuse one of obsolete recommendation element", key: "", recommendations: []timestampedRecommendation{ - {10, time.Now().Add(-10 * time.Minute)}, - {4, time.Now().Add(-1 * time.Minute)}, - {5, time.Now().Add(-2 * time.Minute)}, - {9, time.Now().Add(-9 * time.Minute)}}, + {10, now.Add(-10 * time.Minute)}, + {4, now.Add(-1 * time.Minute)}, + {5, now.Add(-2 * time.Minute)}, + {9, now.Add(-9 * time.Minute)}}, currentReplicas: 100, prenormalizedDesiredReplicas: 3, expectedStabilizedReplicas: 5, expectedRecommendations: []timestampedRecommendation{ - {10, time.Now()}, - {4, time.Now()}, - {5, time.Now()}, - {3, time.Now()}, + {10, now}, + {4, now}, + {5, now}, + {3, now}, }, scaleDownStabilizationWindowSeconds: 3 * 60, }, @@ -3989,20 +3990,44 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) { name: "scale up stabilization, reuse one of obsolete recommendation element", key: "", recommendations: []timestampedRecommendation{ - {10, time.Now().Add(-100 * time.Minute)}, - {6, time.Now().Add(-1 * time.Minute)}, - {5, time.Now().Add(-2 * time.Minute)}, - {9, time.Now().Add(-3 * time.Minute)}}, + {10, now.Add(-100 * time.Minute)}, + {6, now.Add(-1 * time.Minute)}, + {5, now.Add(-2 * time.Minute)}, + {9, now.Add(-3 * time.Minute)}}, currentReplicas: 1, prenormalizedDesiredReplicas: 100, expectedStabilizedReplicas: 5, expectedRecommendations: []timestampedRecommendation{ - {100, time.Now()}, - {6, time.Now()}, - {5, time.Now()}, - {9, time.Now()}, + {100, now}, + {6, now}, + {5, now}, + {9, now}, }, scaleUpStabilizationWindowSeconds: 300, + }, { + name: "scale up and down stabilization, do not scale up when prenormalized rec goes down", + key: "", + recommendations: []timestampedRecommendation{ + {2, now.Add(-100 * time.Minute)}, + {3, now.Add(-3 * time.Minute)}, + }, + currentReplicas: 2, + prenormalizedDesiredReplicas: 1, + expectedStabilizedReplicas: 2, + scaleUpStabilizationWindowSeconds: 300, + scaleDownStabilizationWindowSeconds: 300, + }, { + name: "scale up and down stabilization, do not scale down when prenormalized rec goes up", + key: "", + recommendations: []timestampedRecommendation{ + {2, now.Add(-100 * time.Minute)}, + {1, now.Add(-3 * time.Minute)}, + }, + currentReplicas: 2, + prenormalizedDesiredReplicas: 3, + expectedStabilizedReplicas: 2, + scaleUpStabilizationWindowSeconds: 300, + scaleDownStabilizationWindowSeconds: 300, }, } for _, tc := range tests { @@ -4025,12 +4050,14 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) { } r, _, _ := hc.stabilizeRecommendationWithBehaviors(arg) assert.Equal(t, tc.expectedStabilizedReplicas, r, "expected replicas do not match") - if !assert.Len(t, hc.recommendations[tc.key], len(tc.expectedRecommendations), "stored recommendations differ in length") { - return - } - for i, r := range hc.recommendations[tc.key] { - expectedRecommendation := tc.expectedRecommendations[i] - assert.Equal(t, expectedRecommendation.recommendation, r.recommendation, "stored recommendation differs at position %d", i) + if tc.expectedRecommendations != nil { + if !assert.Len(t, hc.recommendations[tc.key], len(tc.expectedRecommendations), "stored recommendations differ in length") { + return + } + for i, r := range hc.recommendations[tc.key] { + expectedRecommendation := tc.expectedRecommendations[i] + assert.Equal(t, expectedRecommendation.recommendation, r.recommendation, "stored recommendation differs at position %d", i) + } } }) }