Skip to content

Commit

Permalink
Change how we cache the keys in backend.Reporter
Browse files Browse the repository at this point in the history
This fixes a memory leak caused by hashicorp/golang-lru#141.
  • Loading branch information
espadolini authored and github-actions committed Jul 19, 2023
1 parent 48351ec commit bea2eb5
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions lib/backend/report.go
Expand Up @@ -368,11 +368,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 bea2eb5

Please sign in to comment.