Skip to content
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

Jump to bottom

Fix memory leak by checking triggers uniqueness properly #1640

Merged
merged 11 commits into from
Mar 1, 2021
Merged

Fix memory leak by checking triggers uniqueness properly #1640

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c45f41d
add metricName to postgres scaler
ycabrer Nov 25, 2020
69c0db3
fix typo in postgres scaler test
ycabrer Nov 25, 2020
f3ac4aa
close scalers after name check
ycabrer Mar 1, 2021
fa243e2
Update Changelog
ycabrer Mar 1, 2021
009221d
limit metricName validation to new or changed scaledObjects
ycabrer Mar 1, 2021
6914f97
remove validateMetricNameUniqueness
ycabrer Mar 1, 2021
eb315d4
add uniqueness check to getScaledObjectMetricSpecs
ycabrer Mar 1, 2021
d2ec27a
fix typo
ycabrer Mar 1, 2021
aac7880
remove redundent import
ycabrer Mar 1, 2021
b819c5e
formatting change
ycabrer Mar 1, 2021
265b737
update changelog
ycabrer Mar 1, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- Fix a memory leak in kafka client and close push scalers ([#1565](https://github.com/kedacore/keda/issues/1565))
- Add 'Metadata' header to AAD podIdentity request ([#1566](https://github.com/kedacore/keda/issues/1566))
- KEDA should make sure generate correct labels for HPA ([#1630](https://github.com/kedacore/keda/issues/1630))
- Close scalers after name check; fix memory leak ([#1636](https://github.com/kedacore/keda/issues/1636))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Close scalers after name check; fix memory leak ([#1636](https://github.com/kedacore/keda/issues/1636))
- Fix memory leak by checking triggers uniqueness properly ([#1640](https://github.com/kedacore/keda/pull/1640))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed! Good suggestion


### Breaking Changes

Expand Down
13 changes: 8 additions & 5 deletions controllers/scaledobject_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,6 @@ func (r *ScaledObjectReconciler) reconcileScaledObject(logger logr.Logger, scale
return "ScaledObject doesn't have correct scaleTargetRef specification", err
}

err = r.validateMetricNameUniqueness(logger, scaledObject)
if err != nil {
return "Error checking metric name uniqueness", err
}

// Create a new HPA or update existing one according to ScaledObject
newHPACreated, err := r.ensureHPAForScaledObjectExists(logger, scaledObject, &gvkr)
if err != nil {
Expand All @@ -224,6 +219,12 @@ func (r *ScaledObjectReconciler) reconcileScaledObject(logger logr.Logger, scale

// Notify ScaleHandler if a new HPA was created or if ScaledObject was updated
if newHPACreated || scaleObjectSpecChanged {
// Check for any duplicate names in scaledObject
zroubalik marked this conversation as resolved.
Show resolved Hide resolved
err = r.validateMetricNameUniqueness(logger, scaledObject)
if err != nil {
return "Error checking metric name uniqueness", err
}

if r.requestScaleLoop(logger, scaledObject) != nil {
return "Failed to start a new scale loop with scaling logic", err
}
Expand Down Expand Up @@ -260,6 +261,8 @@ func (r *ScaledObjectReconciler) validateMetricNameUniqueness(logger logr.Logger

observedMetricNames := make(map[string]struct{})
for _, scaler := range scalers {
defer scaler.Close()

for _, metric := range scaler.GetMetricSpecForScaling() {
// Only validate external metricNames
if metric.External == nil {
Expand Down