From e6d8a225ec41f6a81074545ef19ca65bdd416ed5 Mon Sep 17 00:00:00 2001
From: George Robinson <george.robinson@grafana.com>
Date: Tue, 22 Apr 2025 11:22:47 +0100
Subject: [PATCH] fix: evict the limits cache once half the TTL has elapsed
 (#17337)

---
 pkg/limits/frontend/cache.go      | 16 ++++---
 pkg/limits/frontend/cache_test.go | 73 +++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/pkg/limits/frontend/cache.go b/pkg/limits/frontend/cache.go
index 8f3a8d8cfa..d68edd5201 100644
--- a/pkg/limits/frontend/cache.go
+++ b/pkg/limits/frontend/cache.go
@@ -32,9 +32,10 @@ func (i *item[V]) hasExpired(now time.Time) bool {
 
 // TTLCache is a simple, thread-safe cache with a single per-cache TTL.
 type TTLCache[K comparable, V any] struct {
-	items map[K]item[V]
-	ttl   time.Duration
-	mu    sync.RWMutex
+	items         map[K]item[V]
+	ttl           time.Duration
+	lastEvictedAt time.Time
+	mu            sync.RWMutex
 
 	// Used for tests.
 	clock quartz.Clock
@@ -73,7 +74,10 @@ func (c *TTLCache[K, V]) Set(key K, value V) {
 		value:     value,
 		expiresAt: now.Add(c.ttl),
 	}
-	c.removeExpiredItems(now)
+	if now.Sub(c.lastEvictedAt) > c.ttl/2 {
+		c.lastEvictedAt = now
+		c.evictExpired(now)
+	}
 }
 
 // Delete implements Cache.Delete.
@@ -90,8 +94,8 @@ func (c *TTLCache[K, V]) Reset() {
 	c.items = make(map[K]item[V])
 }
 
-// removeExpiredItems removes expired items.
-func (c *TTLCache[K, V]) removeExpiredItems(now time.Time) {
+// evictExpired evicts expired items.
+func (c *TTLCache[K, V]) evictExpired(now time.Time) {
 	for key, item := range c.items {
 		if item.hasExpired(now) {
 			delete(c.items, key)
diff --git a/pkg/limits/frontend/cache_test.go b/pkg/limits/frontend/cache_test.go
index 79b6c3bede..f6c7f61817 100644
--- a/pkg/limits/frontend/cache_test.go
+++ b/pkg/limits/frontend/cache_test.go
@@ -106,25 +106,90 @@ func TestTTLCache_Reset(t *testing.T) {
 	require.Equal(t, "qux", value)
 }
 
-func TestTTLCache_RemoveExpiredItems(t *testing.T) {
+func TestTTLCache_EvictExpired(t *testing.T) {
 	c := NewTTLCache[string, string](time.Minute)
 	clock := quartz.NewMock(t)
 	c.clock = clock
 	c.Set("foo", "bar")
 	_, ok := c.items["foo"]
 	require.True(t, ok)
-	// Advance the clock and update foo, it should not be removed.
+	// Advance the clock and update foo, it should not be evicted as its
+	// expiration time should be refreshed.
 	clock.Advance(time.Minute)
 	c.Set("foo", "bar")
 	_, ok = c.items["foo"]
 	require.True(t, ok)
-	// Advance the clock again but this time set bar, foo should be removed.
-	clock.Advance(time.Minute)
+	eviction1 := clock.Now()
+	require.Equal(t, eviction1, c.lastEvictedAt)
+	// Advance the clock 15 seconds. Since 15 seconds is less than half
+	// the TTL (30 seconds) since the last eviction, no eviction should
+	// be run.
+	clock.Advance(15 * time.Second)
 	c.Set("bar", "baz")
 	_, ok = c.items["foo"]
+	require.True(t, ok)
+	_, ok = c.items["bar"]
+	require.True(t, ok)
+	require.Equal(t, eviction1, c.lastEvictedAt)
+	// Advance the clock 16 seconds. Since 31 seconds is more than half the TTL
+	// since the last eviction, an eviction should be run, but no items should
+	// be evicted.
+	clock.Advance(16 * time.Second)
+	c.Set("baz", "qux")
+	_, ok = c.items["foo"]
+	require.True(t, ok)
+	_, ok = c.items["bar"]
+	require.True(t, ok)
+	_, ok = c.items["baz"]
+	require.True(t, ok)
+	eviction2 := clock.Now()
+	require.Equal(t, eviction2, c.lastEvictedAt)
+	// Advance the clock another 15 seconds. Again, since 15 seconds is less
+	// than half the TTL (seconds) since the last eviction, no eviction should
+	// be run.
+	clock.Advance(15 * time.Second)
+	c.Set("qux", "corge")
+	_, ok = c.items["foo"]
+	require.True(t, ok)
+	_, ok = c.items["bar"]
+	require.True(t, ok)
+	_, ok = c.items["baz"]
+	require.True(t, ok)
+	_, ok = c.items["qux"]
+	require.True(t, ok)
+	require.Equal(t, eviction2, c.lastEvictedAt)
+	// Advance the clock another 16 seconds. Since 31 seconds is more than
+	// half the TTL since the last eviction, an eviction should be run and
+	// this time foo should be evicted as it has expired.
+	clock.Advance(16 * time.Second)
+	c.Set("corge", "jorge")
+	_, ok = c.items["foo"]
 	require.False(t, ok)
 	_, ok = c.items["bar"]
 	require.True(t, ok)
+	_, ok = c.items["baz"]
+	require.True(t, ok)
+	_, ok = c.items["qux"]
+	require.True(t, ok)
+	_, ok = c.items["corge"]
+	require.True(t, ok)
+	eviction3 := clock.Now()
+	require.Equal(t, eviction3, c.lastEvictedAt)
+	// Advance the clock one whole minute. All items should be expired.
+	clock.Advance(time.Minute)
+	c.Set("foo", "bar")
+	_, ok = c.items["foo"]
+	require.True(t, ok)
+	_, ok = c.items["bar"]
+	require.False(t, ok)
+	_, ok = c.items["baz"]
+	require.False(t, ok)
+	_, ok = c.items["qux"]
+	require.False(t, ok)
+	_, ok = c.items["qux"]
+	require.False(t, ok)
+	eviction4 := clock.Now()
+	require.Equal(t, eviction4, c.lastEvictedAt)
 }
 
 func TestNopCache(t *testing.T) {