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

What happens when I set CleanWindow = 0 ? No cache will be cleared but will LifeWindow entries... #273

Open
hiqsociety opened this issue Mar 18, 2021 · 10 comments
Labels

Comments

@hiqsociety
Copy link

What happens when I set CleanWindow = 0 ? No cache will be cleared but will LifeWindow entries be able to Get?

Is CleanWindow a bit redundant? it's ok to leave expired entries in LifeWindow right? It just sits there and will not be gotten out.

@hiqsociety hiqsociety added the bug label Mar 18, 2021
@janisz
Copy link
Collaborator

janisz commented Mar 22, 2021

You are partially correct.
Setting clean window to 0 will disable periodical house keeping – removing expired entries. Theses entries could be removed when space is needed to insert new value.

It might look redundant but there was a valid use case for it #31 (comment)

@hixichen
Copy link

hixichen commented Dec 5, 2022

would be great if the doc on the LifeWindow could be more clear

Here is my current setting:

		// time after which entry can be evicted
		// Set to infinity.
		LifeWindow: time.Duration(math.MaxInt32) * time.Second,

		// Interval between removing expired entries (clean up).
		// If set to <= 0 then no action is performed.
		// Setting to < 1 second is counterproductive — bigcache has a one second resolution.
		CleanWindow: 0,

Issue:

Our desire is to have a forever in memory cache, with random failure, we happens to find that even clean window to 0, the item can be deleted.

And, this code made me very confusing: https://github.com/allegro/bigcache/blob/master/bigcache.go#L101

after verified with below test case, easy to catch:

--- FAIL: Test_BigCache_Lifewindow (1.03s)
    logger.go:130: 2022-12-02T23:58:58.002Z	ERROR	Cache Instance OnRemoveWithReason	{"Key": "test1", "Entry": "dGVzdC12YWx1ZS0x", "Reason": 1}
    key_cache_test.go:51:
        	Error Trace:	xxxxxxx key_cache_test.go:51
        	Error:      	Received unexpected error:
        	            	Entry not found
        	Test:       	Test_BigCache_Lifewindow

Test Code

package mytest

import (
	"strconv"
	"testing"
	"time"

	"go.uber.org/zap"
	"go.uber.org/zap/zaptest"

	"github.com/allegro/bigcache/v3"
	"github.com/stretchr/testify/require"
)

// This test was designed to verify when set cleanwindow to 0, what will happen if lifewindow expires.
// refer: https: //github.com/allegro/bigcache/issues/273
func Test_BigCache_Lifewindow(t *testing.T) {
	l := zaptest.NewLogger(t, zaptest.Level(zap.DebugLevel))
	onRemoveWithReason := func(key string, entry []byte, reason bigcache.RemoveReason) {
		l.Error("Cache Instance OnRemoveWithReason",
			zap.String("Key", key),
			zap.Reflect("Entry", entry),
			zap.Reflect("Reason", reason))
	}

	testConfig := bigcache.Config{
		Shards: 2,
		// time after which entry can be evicted
		// Set to infinity.
		LifeWindow: 1 * time.Second,
		// Interval between removing expired entries (clean up).
		// If set to <= 0 then no action is performed.
		// Setting to < 1 second is counterproductive — bigcache has a one second resolution.
		CleanWindow:        0,
		OnRemoveWithReason: onRemoveWithReason,
	}
	cacheInstance, err := bigcache.NewBigCache(testConfig)
	require.NoError(t, err)
	pk := "test1"
	value := []byte("test-value-1")
	require.NoError(t, cacheInstance.Set(pk, value))

	for i := 0; i < 500; i++ {
		getV, err := cacheInstance.Get(pk)
		require.NoError(t, err)
		// refer: https://github.com/allegro/bigcache/issues/31
		require.NoError(t, cacheInstance.Set("key"+strconv.Itoa(i), []byte("test")))
		require.Equal(t, string(value), string(getV))
		time.Sleep(10 * time.Millisecond)
	}
}

@janisz
Copy link
Collaborator

janisz commented Dec 6, 2022

we happens to find that even clean window to 0, the item can be deleted.

It's not deleted but evicted.

bigcache/bigcache.go

Lines 34 to 35 in 0b306aa

// Expired means the key is past its LifeWindow.
Expired = RemoveReason(1)

Have you tried reading it with

bigcache/bigcache.go

Lines 139 to 146 in 982ec3b

// GetWithInfo reads entry for the key with Response info.
// It returns an ErrEntryNotFound when
// no entry exists for the given key.
func (c *BigCache) GetWithInfo(key string) ([]byte, Response, error) {
hashedKey := c.hash.Sum64(key)
shard := c.getShard(hashedKey)
return shard.getWithInfo(key, hashedKey)
}

@SwarnaLathaNatarajan
Copy link

I'm running into the same problem. I'm looking to have a forever in-memory cache, but keys are getting removed even when Life window, Clean window and Hard max cache size are set to 0, 0 and 0 respectively. How do I go about this?

@janisz
Copy link
Collaborator

janisz commented May 17, 2023

Have you tired to set life window to max int?

@SwarnaLathaNatarajan
Copy link

yes, that resolves the issue. Would this guarantee a forever in-memory cache?

@janisz
Copy link
Collaborator

janisz commented May 18, 2023

I'd say yes but I think we should add more explicit way of saying that entries should never expire. Maybe new config option or treat -1 as never expire and then handle it differently in shard

bigcache/shard.go

Lines 284 to 289 in 982ec3b

func (s *cacheShard) isExpired(oldestEntry []byte, currentTimestamp uint64) bool {
oldestTimestamp := readTimestampFromEntry(oldestEntry)
if currentTimestamp <= oldestTimestamp { // if currentTimestamp < oldestTimestamp, the result will out of uint64 limits;
return false
}
return currentTimestamp-oldestTimestamp > s.lifeWindow

@SwarnaLathaNatarajan
Copy link

yea that would be very helpful. Thanks for the help.

Also, I noticed that the keys were being removed only sometimes when Life window, Clean window and Hard max cache size were set to 0, 0 and 0 respectively. Why was that not consistent?

@janisz
Copy link
Collaborator

janisz commented May 18, 2023

That's interesting? Are you sure it was how could it be reproduced?

@SwarnaLathaNatarajan
Copy link

yes, I executed the same code both locally and on a remote instance. The local cache worked, but the remote one ran into the key removal issue and this was consistent.

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