From 6127adaca92c443209bb4e50377ecd7fb21d6319 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Mon, 24 Jul 2023 11:18:05 +0200 Subject: [PATCH] [v11] Change how we cache the keys in backend.Reporter (#29332) * 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 --------- Co-authored-by: Alan Parra --- 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..6cf3ac485dc36 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)