fix: Force evict entries with the same expiration date (#28)

Author: viccon

cache_test.go

@@ -174,6 +174,86 @@ func TestForcedEvictions(t *testing.T) {
 	}
 }
 
+func TestForceEvictAllEntries(t *testing.T) {
+	t.Parallel()
+	capacity := 100
+	numShards := 1
+	ttl := time.Hour
+	evictionpercentage := 100
+	clock := sturdyc.NewTestClock(time.Now())
+	c := sturdyc.New[string](capacity, numShards, ttl, evictionpercentage,
+		sturdyc.WithClock(clock),
+	)
+
+	// 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++ {
+		c.Set(strconv.Itoa(i), strconv.Itoa(i))
+	}
+
+	// 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.
+	if c.Size() != 1 {
+		t.Errorf("expected cache size to be 0, got %d", c.Size())
+	}
+}
+
+func TestForceEvictionSameTime(t *testing.T) {
+	t.Parallel()
+	capacity := 100
+	numShards := 2
+	ttl := time.Hour
+	evictionpercentage := 50
+	clock := sturdyc.NewTestClock(time.Now())
+	c := sturdyc.New[string](capacity, numShards, ttl, evictionpercentage,
+		sturdyc.WithClock(clock),
+	)
+
+	// Now we're going to write 1000 records to the cache which should
+	// exceed its capacity and trigger a couple of forced evictions.
+	for i := 0; i < 1000; i++ {
+		c.Set(strconv.Itoa(i), strconv.Itoa(i))
+	}
+
+	// Assert that even though we're writing 1000
+	// records we never exceed the capacity of 100.
+	if c.Size() > 100 {
+		t.Errorf("exceeded the cache size of 100, got %d", c.Size())
+	}
+}
+
+func TestForceEvictionTwoDifferentTimes(t *testing.T) {
+	t.Parallel()
+	capacity := 100
+	numShards := 1
+	ttl := time.Hour
+	evictionpercentage := 10
+	clock := sturdyc.NewTestClock(time.Now())
+	c := sturdyc.New[string](capacity, numShards, ttl, evictionpercentage,
+		sturdyc.WithClock(clock),
+	)
+
+	// We're going to write 50 records, then move the clock forward
+	// and write another 50 to reach the capacity of the cache.
+	for i := 0; i < 50; i++ {
+		c.Set(strconv.Itoa(i), strconv.Itoa(i))
+	}
+	clock.Add(time.Hour)
+	for i := 0; i < 50; i++ {
+		c.Set(strconv.Itoa(i+50), strconv.Itoa(i+50))
+	}
+
+	// At this point, the cache should be at its capacity so
+	// adding another item should trigger a forced eviction.
+	// Given our eviction percentage of 10%, we expect the
+	// cache to first remove 10 items, and then write this
+	// record afterwards.
+	c.Set(strconv.Itoa(100), strconv.Itoa(100))
+	if c.Size() != 91 {
+		t.Errorf("expected cache size to be 91, got %d", c.Size())
+	}
+}
+
 func TestDisablingForcedEvictionMakesSetANoop(t *testing.T) {
 	t.Parallel()
 

quickselect_test.go

@@ -62,6 +62,55 @@ func TestCutoff(t *testing.T) {
 	}
 }
 
+func TestCutOffSameTime(t *testing.T) {
+	t.Parallel()
+	now := time.Now()
+	timestamps := make([]time.Time, 0, 100)
+	for i := 0; i < 100; i++ {
+		timestamps = append(timestamps, now)
+	}
+
+	// Given that we have a list where all the timestamps are the same, we
+	// should get that same timestamp back for every percentile.
+	cutoffOne := sturdyc.FindCutoff(timestamps, 0.1)
+	cutoffTwo := sturdyc.FindCutoff(timestamps, 0.3)
+	cutoffThree := sturdyc.FindCutoff(timestamps, 0.5)
+	if cutoffOne != now {
+		t.Errorf("expected cutoff to be %v, got %v", now, cutoffOne)
+	}
+	if cutoffTwo != now {
+		t.Errorf("expected cutoff to be %v, got %v", now, cutoffTwo)
+	}
+	if cutoffThree != now {
+		t.Errorf("expected cutoff to be %v, got %v", now, cutoffThree)
+	}
+}
+
+func TestCutOffTwoTimes(t *testing.T) {
+	t.Parallel()
+	timestamps := make([]time.Time, 0, 100)
+
+	firstTime := time.Now()
+	for i := 0; i < 50; i++ {
+		timestamps = append(timestamps, firstTime)
+	}
+
+	secondTime := time.Now().Add(time.Second)
+	for i := 0; i < 50; i++ {
+		timestamps = append(timestamps, secondTime)
+	}
+
+	firstCutoff := sturdyc.FindCutoff(timestamps, 0.49)
+	if firstCutoff != firstTime {
+		t.Errorf("expected cutoff to be %v, got %v", firstTime, firstCutoff)
+	}
+
+	secondCutoff := sturdyc.FindCutoff(timestamps, 0.51)
+	if secondCutoff != secondTime {
+		t.Errorf("expected cutoff to be %v, got %v", secondTime, secondCutoff)
+	}
+}
+
 func TestReturnsEmptyTimeIfArgumentsAreInvalid(t *testing.T) {
 	t.Parallel()
 

shard.go

@@ -64,17 +64,36 @@ func (s *shard[T]) evictExpired() {
 // based on the expiration time. Should be called with a lock.
 func (s *shard[T]) forceEvict() {
 	s.reportForcedEviction()
+
+	// Check if we should evict all entries.
+	if s.evictionPercentage == 100 {
+		s.entries = make(map[string]*entry[T])
+		s.reportEntriesEvicted(len(s.entries))
+		return
+	}
+
 	expirationTimes := make([]time.Time, 0, len(s.entries))
 	for _, e := range s.entries {
 		expirationTimes = append(expirationTimes, e.expiresAt)
 	}
 
-	cutoff := FindCutoff(expirationTimes, float64(s.evictionPercentage)/100)
+	// We could have a lumpy distribution of expiration times. As an example, we
+	// might have 100 entries in the cache but only 2 unique expiration times. In
+	// order to not over-evict when trying to remove 10%, we'll have to keep
+	// track of the number of entries that we've evicted.
+	percentage := float64(s.evictionPercentage) / 100
+	cutoff := FindCutoff(expirationTimes, percentage)
+	entriesToEvict := int(float64(len(expirationTimes)) * percentage)
 	entriesEvicted := 0
 	for key, e := range s.entries {
-		if e.expiresAt.Before(cutoff) {
+		// Here we're essentially saying: if e.expiresAt <= cutoff.
+		if !e.expiresAt.After(cutoff) {
 			delete(s.entries, key)
 			entriesEvicted++
+
+			if entriesEvicted == entriesToEvict {
+				break
+			}
 		}
 	}
 	s.reportEntriesEvicted(entriesEvicted)