From afc36b4cd4a91f170236e6df338a5e635bb34a57 Mon Sep 17 00:00:00 2001 From: Ville Vesilehto Date: Fri, 14 Feb 2025 22:35:20 +0200 Subject: [PATCH] fix(shard): ensure correct evicted entries count When evicting all entries (100% eviction), store the count before clearing the map. Previously we were reporting the length of an empty map, resulting in incorrect eviction metrics. The fix ensures accurate reporting of mass evictions for monitoring cache behavior. Added test coverage to verify both the evicted entries count and forced eviction events are correct. Signed-off-by: Ville Vesilehto --- cache_test.go | 28 ++++++++++++++++++++++++---- shard.go | 3 ++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/cache_test.go b/cache_test.go index 6c78e02..5cfda2c 100644 --- a/cache_test.go +++ b/cache_test.go @@ -181,20 +181,40 @@ func TestForceEvictAllEntries(t *testing.T) { ttl := time.Hour evictionpercentage := 100 clock := sturdyc.NewTestClock(time.Now()) + metricRecorder := newTestMetricsRecorder(numShards) c := sturdyc.New[string](capacity, numShards, ttl, evictionpercentage, sturdyc.WithClock(clock), + sturdyc.WithMetrics(metricRecorder), ) - // Now we're going to write 101 records to the cache which should - // exceed its capacity and trigger a forced eviction. - for i := 0; i < 101; i++ { + // Fill the cache to capacity + for i := 0; i < capacity; i++ { c.Set(strconv.Itoa(i), strconv.Itoa(i)) } + // Record metrics before eviction + preEvictionCount := metricRecorder.evictedEntries + + // Trigger eviction by adding one more entry // When the eviction is triggered by the 100th write, we expect the cache to // be emptied. Therefore, the 101th write should mean that the size is now 1. + c.Set("trigger", "value") + + // When the eviction is triggered, we expect the cache to be emptied + // and only contain the trigger value if c.Size() != 1 { - t.Errorf("expected cache size to be 0, got %d", c.Size()) + t.Errorf("expected cache size to be 1, got %d", c.Size()) + } + + // Verify eviction metrics + metricRecorder.Lock() + defer metricRecorder.Unlock() + evictedEntries := metricRecorder.evictedEntries - preEvictionCount + if evictedEntries != capacity { + t.Errorf("got %d evicted entries, want %d", evictedEntries, capacity) + } + if metricRecorder.forcedEvictions != 1 { + t.Errorf("got %d forced eviction events, want 1", metricRecorder.forcedEvictions) } } diff --git a/shard.go b/shard.go index 6f510de..82701af 100644 --- a/shard.go +++ b/shard.go @@ -67,8 +67,9 @@ func (s *shard[T]) forceEvict() { // Check if we should evict all entries. if s.evictionPercentage == 100 { + evictedCount := len(s.entries) s.entries = make(map[string]*entry[T]) - s.reportEntriesEvicted(len(s.entries)) + s.reportEntriesEvicted(evictedCount) return }