allegro / bigcache

Efficient cache for gigabytes of data written in Go.
http://allegro.tech/2016/03/writing-fast-cache-service-in-go.html
Apache License 2.0
7.45k stars 593 forks source link

Disable eviction #150

Open larytet opened 5 years ago

larytet commented 5 years ago

Something like

diff --git a/bigcache.go b/bigcache.go
index b387926..32db182 100644
--- a/bigcache.go
+++ b/bigcache.go
@@ -171,7 +171,7 @@ func (c *BigCache) Iterator() *EntryInfoIterator {

 func (c *BigCache) onEvict(oldestEntry []byte, currentTimestamp uint64, evict func(reason RemoveReason) error) bool {
        oldestTimestamp := readTimestampFromEntry(oldestEntry)
-       if currentTimestamp-oldestTimestamp > c.lifeWindow {
+       if (int(c.lifeWindow) >= 0) && (currentTimestamp-oldestTimestamp > c.lifeWindow) {
                evict(Expired)
                return true
        }
diff --git a/config.go b/config.go
index 918908c..c5905f4 100644
--- a/config.go
+++ b/config.go
@@ -7,6 +7,7 @@ type Config struct {
        // Number of cache shards, value must be a power of two
        Shards int
        // Time after which entry can be evicted
+       // No eviction if LifeWindow < 0
        LifeWindow time.Duration
        // 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.
diff --git a/shard.go b/shard.go
index a31094f..6e5612a 100644
--- a/shard.go
+++ b/shard.go
@@ -134,7 +134,7 @@ func (s *cacheShard) del(key string, hashedKey uint64) error {

 func (s *cacheShard) onEvict(oldestEntry []byte, currentTimestamp uint64, evict func(reason RemoveReason) error) bool {
        oldestTimestamp := readTimestampFromEntry(oldestEntry)
-       if currentTimestamp-oldestTimestamp > s.lifeWindow {
+       if (int(s.lifeWindow) >= 0) && (currentTimestamp-oldestTimestamp > s.lifeWindow) {
                evict(Expired)
                return true
        }
janisz commented 5 years ago

LGTM, @cristaloleg what do you think?

cristaloleg commented 5 years ago

To be honest, I'm out of context. But what I've understood from code LGTM

larytet commented 5 years ago

The patch is not intended to be a production code. I tried to explain what I am missing in the API. The production system could introduce a configuration flag and save quite a lot of memory and code.

cristaloleg commented 5 years ago

Well, what I am missing in the API sounds strange to me, 'cause there is no changes in the API (unless 1 comment?).

Basically you want to say, that we need explicitly check LifeWindow to be non-negative.

I don't see any problems with such change, there is only 1 question about backward compatibility (will it be broken or not?)

larytet commented 5 years ago

The patch changes the API, the contract of what LifeWindow means. The patch breaks the backward compatibility for applications using large timeouts. I can imagine that some application could do something like LifeWindow := 292*365*24*time.Hour

cristaloleg commented 5 years ago

We might release v3 soon, with additional changes, so I think it's good to keep eye on this issue.

Thanks for sharing 😉

vtolstov commented 4 years ago

any chance to get this in v3 and also v3 release?

siennathesane commented 4 years ago

I think this is something which should make it into v3.

In the small scale, I don't see a difference, I think we'll see marginal performance improvements when working with millions of objects because we're not doing a double comparison.