Skip to content

Commit

Permalink
[v13] Change how we cache the keys in backend.Reporter (#29330)
Browse files Browse the repository at this point in the history
* Change how we cache the keys in backend.Reporter

This fixes a memory leak caused by hashicorp/golang-lru#141.

* Apply suggestions from code review

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

---------

Co-authored-by: Alan Parra <alan.parra@goteleport.com>
  • Loading branch information
espadolini and codingllama committed Jul 24, 2023
1 parent ccc1d93 commit 2bf4033
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions lib/backend/report.go
Expand Up @@ -372,11 +372,23 @@ func (s *Reporter) trackRequest(opType types.OpType, key []byte, endKey []byte)
rangeSuffix = teleport.TagTrue
}

s.topRequestsCache.Add(topRequestsCacheKey{
cacheKey := topRequestsCacheKey{
component: s.Component,
key: keyLabel,
isRange: rangeSuffix,
}, struct{}{})
}
// We need to do ContainsOrAdd and then Get because if we do Add we hit
// https://github.com/hashicorp/golang-lru/issues/141 which can cause a
// memory leak in certain workloads (where we keep overwriting the same
// key); it's not clear if Add to overwrite would be the correct thing to do
// here anyway, as we use LRU eviction to delete unused metrics, but
// overwriting might cause an eviction of the same metric we are about to
// bump up in freshness, which is obviously wrong
if ok, _ := s.topRequestsCache.ContainsOrAdd(cacheKey, struct{}{}); ok {
// Refresh the key's position in the LRU cache, if it was already in it.
s.topRequestsCache.Get(cacheKey)
}

counter, err := requests.GetMetricWithLabelValues(s.Component, keyLabel, rangeSuffix)
if err != nil {
log.Warningf("Failed to get counter: %v", err)
Expand Down

0 comments on commit 2bf4033

Please sign in to comment.