Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OnRemoveWithReason still receives deleted entry #126

Closed
santatic opened this issue May 15, 2019 · 5 comments
Closed

OnRemoveWithReason still receives deleted entry #126

santatic opened this issue May 15, 2019 · 5 comments
Labels

Comments

@santatic
Copy link

santatic commented May 15, 2019

I deleted a key from cache, OnRemoveWithReason receives entry with reason bigcache.Deleted
But then, at expire time OnRemoveWithReason still receives deleted entry with reason bigcache.Expired

My config:

var cache, _ = bigcache.NewBigCache(bigcache.Config{
		Shards:             16,
		LifeWindow:         time.Minute,
		CleanWindow:        500 * time.Millisecond,
		MaxEntriesInWindow: 5,
		MaxEntrySize:       1 * 1024 * 1024,
		Verbose:            true,
		HardMaxCacheSize:   5,
		OnRemove:           nil,
		OnRemoveWithReason: myRemoveWithReasonFunc,
		Logger:             bigcache.DefaultLogger(),
	})
  • My OS: Ubuntu 18.04
  • My go version: go1.12.2 linux/amd64
  • My bigcache version: v1.2.0
@siennathesane
Copy link
Collaborator

at expire time OnRemoveWithReason still receives deleted entry with reason bigcache.Expired

@santatic are you saying the entry isn't actually being deleted? Can you explain the problem/question a bit more?

@santatic
Copy link
Author

santatic commented May 17, 2019

@santatic are you saying the entry isn't actually being deleted?

Yes, that's my case.

My steps:

  1. I set key a = v
  2. I delete key a, OnRemoveWithReason response key a with reason bigcache.Deleted
  3. I wait for expire time LifeWindow, OnRemoveWithReason response key a with reason bigcache.Expired

@siennathesane
Copy link
Collaborator

Let me see if I can reproduce it with a test case and if I can, I'll get it fixed, if not I'll have to ask you for a reproduction so that we can ensure there are no regressions later.

@ilyaglow
Copy link

Here's the test that catches this case:

diff --git a/bigcache_test.go b/bigcache_test.go
index 580777d..2444dd5 100644
--- a/bigcache_test.go
+++ b/bigcache_test.go
@@ -275,6 +275,55 @@ func TestOnRemoveFilter(t *testing.T) {
        assert.True(t, onRemoveInvoked)
 }

+func TestOnRemoveFilterExpired(t *testing.T) {
+       t.Parallel()
+
+       // given
+       clock := mockedClock{value: 0}
+       onDeleteInvoked := false
+       onExpireInvoked := false
+       onRemove := func(key string, entry []byte, reason RemoveReason) {
+               switch reason {
+               case Deleted:
+                       onDeleteInvoked = true
+               case Expired:
+                       onExpireInvoked = true
+               }
+       }
+
+       c := Config{
+               Shards:             1,
+               LifeWindow:         2 * time.Second,
+               CleanWindow:        1 * time.Second,
+               MaxEntriesInWindow: 1,
+               MaxEntrySize:       256,
+               OnRemoveWithReason: onRemove,
+       }.OnRemoveFilterSet(Expired)
+
+       cache, _ := newBigCache(c, &clock)
+
+       // when
+       cache.Set("key", []byte("value"))
+       clock.set(5)
+       cache.Delete("key")
+
+       // then
+       assert.False(t, onDeleteInvoked)
+       assert.True(t, onExpireInvoked, "onExpireInvoke is false, but wants true")
+
+       // reset value
+       onExpireInvoked = false
+
+       // when
+       cache.Set("key2", []byte("value2"))
+       cache.Delete("key2")
+       clock.set(5)
+
+       // then
+       assert.False(t, onDeleteInvoked)
+       assert.False(t, onExpireInvoked, "onExpireInvoke is true, but wants false")
+}
+
 func TestCacheLen(t *testing.T) {
        t.Parallel()

It seems that OnRemoveFilterSet function is involved. Does your case include it @santatic?

@siennathesane
Copy link
Collaborator

Closed in #219.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants