Skip to content

Commit

Permalink
Fix scaler leak during cache refresh (kedacore#5807)
Browse files Browse the repository at this point in the history
Signed-off-by: Guillaume Jacquet <guillaume.jacquet@gmail.com>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
  • Loading branch information
gjacquet authored and JorTurFer committed Jul 30, 2024
1 parent 8386a88 commit 4757459
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ Here is an overview of all new **experimental** features:
- **MongoDB Scaler**: MongoDB url parses correctly `+srv` scheme ([#5760](https://github.com/kedacore/keda/issues/5760))
- **New Relic Scaler**: Fix CVE-2024-6104 in github.com/hashicorp/go-retryablehttp ([#5944](https://github.com/kedacore/keda/issues/5944))
- **ScaledJob**: Fix ScaledJob ignores failing trigger(s) error ([#5922](https://github.com/kedacore/keda/issues/5922))
- **General**: Scalers are properly closed after being refreshed ([#5806](https://github.com/kedacore/keda/issues/5806))
- **MongoDB Scaler**: MongoDB url parses correctly `+srv` scheme ([#5760](https://github.com/kedacore/keda/issues/5760))

### Deprecations
Expand Down
23 changes: 10 additions & 13 deletions pkg/scaling/scale_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,23 +293,20 @@ func (h *scaleHandler) getScalersCacheForScaledObject(ctx context.Context, scale
// performGetScalersCache returns cache for input scalableObject, it is common code used by GetScalersCache() and getScalersCacheForScaledObject() methods
func (h *scaleHandler) performGetScalersCache(ctx context.Context, key string, scalableObject interface{}, scalableObjectGeneration *int64, scalableObjectKind, scalableObjectNamespace, scalableObjectName string) (*cache.ScalersCache, error) {
h.scalerCachesLock.RLock()
regenerateCache := false

if cache, ok := h.scalerCaches[key]; ok {
// generation was specified -> let's include it in the check as well
if scalableObjectGeneration != nil {
if cache.ScalableObjectGeneration == *scalableObjectGeneration {
h.scalerCachesLock.RUnlock()
return cache, nil
}
// object was found in cache, but the generation is not correct,
// we'll need to close scalers in the cache and
// proceed further to recreate the cache
regenerateCache = false
} else {
h.scalerCachesLock.RUnlock()
return cache, nil
}
}

h.scalerCachesLock.RUnlock()

if scalableObject == nil {
Expand Down Expand Up @@ -379,17 +376,17 @@ func (h *scaleHandler) performGetScalersCache(ctx context.Context, key string, s
default:
}

// Scalers Close() could be impacted by timeouts, blocking the mutex
// until the timeout happens. Instead of locking the mutex, we take
// the old cache item and we close it in another goroutine, not locking
// the cache: https://github.com/kedacore/keda/issues/5083
if regenerateCache {
oldCache := h.scalerCaches[key]
h.scalerCachesLock.Lock()
defer h.scalerCachesLock.Unlock()

if oldCache, ok := h.scalerCaches[key]; ok {
// Scalers Close() could be impacted by timeouts, blocking the mutex
// until the timeout happens. Instead of locking the mutex, we take
// the old cache item and we close it in another goroutine, not locking
// the cache: https://github.com/kedacore/keda/issues/5083
go oldCache.Close(ctx)
}

h.scalerCachesLock.Lock()
defer h.scalerCachesLock.Unlock()
h.scalerCaches[key] = newCache
return h.scalerCaches[key], nil
}
Expand Down

0 comments on commit 4757459

Please sign in to comment.