From 65642c3f8927cacb2def8519e4cd0f97ec6bead6 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 8 Jun 2022 14:23:18 +0200 Subject: [PATCH 1/4] Define a reason why the fifocache has evicted an item Signed-off-by: Danny Kopping --- pkg/storage/chunk/cache/fifo_cache.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/pkg/storage/chunk/cache/fifo_cache.go b/pkg/storage/chunk/cache/fifo_cache.go index 20cf11120fa42..2e9e7aed33dec 100644 --- a/pkg/storage/chunk/cache/fifo_cache.go +++ b/pkg/storage/chunk/cache/fifo_cache.go @@ -86,7 +86,7 @@ type FifoCache struct { entriesAdded prometheus.Counter entriesAddedNew prometheus.Counter - entriesEvicted prometheus.Counter + entriesEvicted *prometheus.CounterVec entriesCurrent prometheus.Gauge totalGets prometheus.Counter totalMisses prometheus.Counter @@ -94,6 +94,13 @@ type FifoCache struct { memoryBytes prometheus.Gauge } +const ( + expiredReason string = "expired" + fullReason = "full" + tooBigReason = "too big" + stoppedReason = "stopped" +) + type cacheEntry struct { updated time.Time key string @@ -155,13 +162,13 @@ func NewFifoCache(name string, cfg FifoCacheConfig, reg prometheus.Registerer, l ConstLabels: prometheus.Labels{"cache": name}, }), - entriesEvicted: promauto.With(reg).NewCounter(prometheus.CounterOpts{ + entriesEvicted: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{ Namespace: "querier", Subsystem: "cache", Name: "evicted_total", Help: "The total number of evicted entries", ConstLabels: prometheus.Labels{"cache": name}, - }), + }, []string{"reason"}), entriesCurrent: promauto.With(reg).NewGauge(prometheus.GaugeOpts{ Namespace: "querier", @@ -237,7 +244,7 @@ func (c *FifoCache) pruneExpiredItems(ttl time.Duration) { delete(c.entries, k) c.currSizeBytes -= sizeOf(entry) c.entriesCurrent.Dec() - c.entriesEvicted.Inc() + c.entriesEvicted.WithLabelValues(expiredReason).Inc() } } } @@ -278,7 +285,7 @@ func (c *FifoCache) Stop() { close(c.done) - c.entriesEvicted.Add(float64(c.lru.Len())) + c.entriesEvicted.WithLabelValues(stoppedReason).Add(float64(c.lru.Len())) c.entries = make(map[string]*list.Element) c.lru.Init() @@ -314,7 +321,7 @@ func (c *FifoCache) put(key string, value []byte) { // Cannot keep this item in the cache. if ok { // We do not replace this item. - c.entriesEvicted.Inc() + c.entriesEvicted.WithLabelValues(tooBigReason).Inc() } c.memoryBytes.Set(float64(c.currSizeBytes)) return @@ -330,7 +337,7 @@ func (c *FifoCache) put(key string, value []byte) { delete(c.entries, evicted.key) c.currSizeBytes -= sizeOf(evicted) c.entriesCurrent.Dec() - c.entriesEvicted.Inc() + c.entriesEvicted.WithLabelValues(fullReason).Inc() } // Finally, we have space to add the item. From 34d6d99272e7d792a828b90be0036c0db211c82e Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 8 Jun 2022 14:51:06 +0200 Subject: [PATCH 2/4] Fixing tests Signed-off-by: Danny Kopping --- pkg/storage/chunk/cache/fifo_cache.go | 2 +- pkg/storage/chunk/cache/fifo_cache_test.go | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pkg/storage/chunk/cache/fifo_cache.go b/pkg/storage/chunk/cache/fifo_cache.go index 2e9e7aed33dec..781a6d5ae6989 100644 --- a/pkg/storage/chunk/cache/fifo_cache.go +++ b/pkg/storage/chunk/cache/fifo_cache.go @@ -95,7 +95,7 @@ type FifoCache struct { } const ( - expiredReason string = "expired" + expiredReason string = "expired" //nolint:staticcheck fullReason = "full" tooBigReason = "too big" stoppedReason = "stopped" diff --git a/pkg/storage/chunk/cache/fifo_cache_test.go b/pkg/storage/chunk/cache/fifo_cache_test.go index 4c391c56792b9..6bc7153b64bf4 100644 --- a/pkg/storage/chunk/cache/fifo_cache_test.go +++ b/pkg/storage/chunk/cache/fifo_cache_test.go @@ -55,9 +55,11 @@ func TestFifoCacheEviction(t *testing.T) { require.NoError(t, err) require.Len(t, c.entries, cnt) + reason := fullReason + assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(1)) assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt)) - assert.Equal(t, testutil.ToFloat64(c.entriesEvicted), float64(0)) + assert.Equal(t, testutil.ToFloat64(c.entriesEvicted.WithLabelValues(reason)), float64(0)) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt)) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries))) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len())) @@ -98,7 +100,7 @@ func TestFifoCacheEviction(t *testing.T) { assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(2)) assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt+evicted)) - assert.Equal(t, testutil.ToFloat64(c.entriesEvicted), float64(evicted)) + assert.Equal(t, testutil.ToFloat64(c.entriesEvicted.WithLabelValues(reason)), float64(evicted)) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt)) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries))) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len())) @@ -119,7 +121,7 @@ func TestFifoCacheEviction(t *testing.T) { assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(2)) assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt+evicted)) - assert.Equal(t, testutil.ToFloat64(c.entriesEvicted), float64(evicted)) + assert.Equal(t, testutil.ToFloat64(c.entriesEvicted.WithLabelValues(reason)), float64(evicted)) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt)) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries))) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len())) @@ -149,7 +151,7 @@ func TestFifoCacheEviction(t *testing.T) { assert.Equal(t, testutil.ToFloat64(c.entriesAdded), float64(3)) assert.Equal(t, testutil.ToFloat64(c.entriesAddedNew), float64(cnt+evicted)) - assert.Equal(t, testutil.ToFloat64(c.entriesEvicted), float64(evicted)) + assert.Equal(t, testutil.ToFloat64(c.entriesEvicted.WithLabelValues(reason)), float64(evicted)) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(cnt)) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(len(c.entries))) assert.Equal(t, testutil.ToFloat64(c.entriesCurrent), float64(c.lru.Len())) @@ -208,7 +210,8 @@ func TestFifoCacheExpiry(t *testing.T) { assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesAdded)) assert.Equal(t, float64(4), testutil.ToFloat64(c.entriesAddedNew)) - assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesEvicted)) + assert.Equal(t, float64(0), testutil.ToFloat64(c.entriesEvicted.WithLabelValues(expiredReason))) + assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesEvicted.WithLabelValues(fullReason))) assert.Equal(t, float64(3), testutil.ToFloat64(c.entriesCurrent)) assert.Equal(t, float64(len(c.entries)), testutil.ToFloat64(c.entriesCurrent)) assert.Equal(t, float64(c.lru.Len()), testutil.ToFloat64(c.entriesCurrent)) @@ -223,7 +226,8 @@ func TestFifoCacheExpiry(t *testing.T) { assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesAdded)) assert.Equal(t, float64(4), testutil.ToFloat64(c.entriesAddedNew)) - assert.Equal(t, float64(4), testutil.ToFloat64(c.entriesEvicted)) + assert.Equal(t, float64(3), testutil.ToFloat64(c.entriesEvicted.WithLabelValues(expiredReason))) + assert.Equal(t, float64(1), testutil.ToFloat64(c.entriesEvicted.WithLabelValues(fullReason))) assert.Equal(t, float64(0), testutil.ToFloat64(c.entriesCurrent)) assert.Equal(t, float64(len(c.entries)), testutil.ToFloat64(c.entriesCurrent)) assert.Equal(t, float64(c.lru.Len()), testutil.ToFloat64(c.entriesCurrent)) From 4d9a986715bba883f4b83f805987d9e18b66a18f Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 8 Jun 2022 15:47:41 +0200 Subject: [PATCH 3/4] Making reason more clear Signed-off-by: Danny Kopping --- pkg/storage/chunk/cache/fifo_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/chunk/cache/fifo_cache.go b/pkg/storage/chunk/cache/fifo_cache.go index 781a6d5ae6989..54e8789b03a75 100644 --- a/pkg/storage/chunk/cache/fifo_cache.go +++ b/pkg/storage/chunk/cache/fifo_cache.go @@ -97,7 +97,7 @@ type FifoCache struct { const ( expiredReason string = "expired" //nolint:staticcheck fullReason = "full" - tooBigReason = "too big" + tooBigReason = "object too big" stoppedReason = "stopped" ) From a4122414cafafdf24e532c331cbd7c5108ff4470 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 8 Jun 2022 16:12:32 +0200 Subject: [PATCH 4/4] Do not increment eviction counter on Stop() Signed-off-by: Danny Kopping --- pkg/storage/chunk/cache/fifo_cache.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/storage/chunk/cache/fifo_cache.go b/pkg/storage/chunk/cache/fifo_cache.go index 54e8789b03a75..f2722466f5365 100644 --- a/pkg/storage/chunk/cache/fifo_cache.go +++ b/pkg/storage/chunk/cache/fifo_cache.go @@ -98,7 +98,6 @@ const ( expiredReason string = "expired" //nolint:staticcheck fullReason = "full" tooBigReason = "object too big" - stoppedReason = "stopped" ) type cacheEntry struct { @@ -285,8 +284,6 @@ func (c *FifoCache) Stop() { close(c.done) - c.entriesEvicted.WithLabelValues(stoppedReason).Add(float64(c.lru.Len())) - c.entries = make(map[string]*list.Element) c.lru.Init() c.currSizeBytes = 0