From 0fa9ca54b36b2ff03238eafbfa83e1943f7ff756 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Wed, 19 Jul 2023 20:25:41 +0200 Subject: [PATCH] Change how we cache the keys in backend.Reporter This fixes a memory leak caused by hashicorp/golang-lru#141. --- lib/backend/report.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/backend/report.go b/lib/backend/report.go index 86214c5835fff..783f8ce444fae 100644 --- a/lib/backend/report.go +++ b/lib/backend/report.go @@ -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)