Skip to content

Commit

Permalink
Deleted entries should not be removed by expire eviction (#219)
Browse files Browse the repository at this point in the history
Co-authored-by: bachhh <[email protected]>
  • Loading branch information
bachhh and bachhh authored May 6, 2020
1 parent 37b9eb2 commit d572104
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 0 deletions.
61 changes: 61 additions & 0 deletions bigcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,67 @@ func TestOnRemoveFilter(t *testing.T) {
assertEqual(t, true, onRemoveInvoked)
}

func TestOnRemoveFilterExpired(t *testing.T) {
// t.Parallel()

// given
clock := mockedClock{value: 0}
onRemoveDeleted, onRemoveExpired := false, false
var err error
onRemove := func(key string, entry []byte, reason RemoveReason) {
switch reason {

case Deleted:
onRemoveDeleted = true
case Expired:
onRemoveExpired = true

}
}
c := Config{
Shards: 1,
LifeWindow: 3 * time.Second,
CleanWindow: 0,
MaxEntriesInWindow: 10,
MaxEntrySize: 256,
OnRemoveWithReason: onRemove,
}

cache, err := newBigCache(c, &clock)
assertEqual(t, err, nil)

// case 1: key is deleted AFTER expire
// when
onRemoveDeleted, onRemoveExpired = false, false
clock.set(0)

cache.Set("key", []byte("value"))
clock.set(5)
cache.cleanUp(uint64(clock.epoch()))

err = cache.Delete("key")

// then
assertEqual(t, err, ErrEntryNotFound)
assertEqual(t, false, onRemoveDeleted)
assertEqual(t, true, onRemoveExpired)

// case 1: key is deleted BEFORE expire
// when
onRemoveDeleted, onRemoveExpired = false, false
clock.set(0)

cache.Set("key2", []byte("value2"))
err = cache.Delete("key2")
clock.set(5)
cache.cleanUp(uint64(clock.epoch()))
// then

assertEqual(t, err, nil)
assertEqual(t, true, onRemoveDeleted)
assertEqual(t, false, onRemoveExpired)
}

func TestOnRemoveGetEntryStats(t *testing.T) {
t.Parallel()

Expand Down
4 changes: 4 additions & 0 deletions shard.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,10 @@ func (s *cacheShard) removeOldestEntry(reason RemoveReason) error {
oldest, err := s.entries.Pop()
if err == nil {
hash := readHashFromEntry(oldest)
if hash == 0 {
// entry has been explicitly deleted with resetKeyFromEntry, ignore
return nil
}
delete(s.hashmap, hash)
s.onRemove(oldest, reason)
if s.statsEnabled {
Expand Down

0 comments on commit d572104

Please sign in to comment.