From 9d4946d9b973c8e860ae42944e07f5bbe28a506b Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada <6warashi9@gmail.com> Date: Sat, 22 May 2021 03:37:25 +0900 Subject: [PATCH] change expiration type from int64 to time.Time (#277) --- cache.go | 13 ++++++------- cache_test.go | 36 ++++++++++++++++++++++++++++++++++++ store.go | 12 ++++++------ store_test.go | 6 +++--- ttl.go | 18 +++++++++--------- 5 files changed, 60 insertions(+), 25 deletions(-) diff --git a/cache.go b/cache.go index 70226fd3..7226245b 100644 --- a/cache.go +++ b/cache.go @@ -152,7 +152,7 @@ type Item struct { Conflict uint64 Value interface{} Cost int64 - Expiration int64 + Expiration time.Time wg *sync.WaitGroup } @@ -258,7 +258,7 @@ func (c *Cache) SetWithTTL(key, value interface{}, cost int64, ttl time.Duration return false } - var expiration int64 + var expiration time.Time switch { case ttl == 0: // No expiration. @@ -267,7 +267,7 @@ func (c *Cache) SetWithTTL(key, value interface{}, cost int64, ttl time.Duration // Treat this a a no-op. return false default: - expiration = time.Now().Add(ttl).Unix() + expiration = time.Now().Add(ttl) } keyHash, conflictHash := c.keyToHash(key) @@ -335,18 +335,17 @@ func (c *Cache) GetTTL(key interface{}) (time.Duration, bool) { } expiration := c.store.Expiration(keyHash) - if expiration == 0 { + if expiration.IsZero() { // found but no expiration return 0, true } - ttl := time.Unix(expiration, 0) - if time.Now().After(ttl) { + if time.Now().After(expiration) { // found but expired return 0, false } - return time.Until(ttl), true + return time.Until(expiration), true } // Close stops all goroutines and closes all channels. diff --git a/cache_test.go b/cache_test.go index 455ac45f..840f132b 100644 --- a/cache_test.go +++ b/cache_test.go @@ -961,3 +961,39 @@ func newTestCache() (*Cache, error) { Metrics: true, }) } + +func TestCacheWithTTL(t *testing.T) { + // There may be a race condition, so run the test multiple times. + const try = 10 + + for i := 0; i < try; i++ { + t.Run(strconv.Itoa(i), func(t *testing.T) { + c, err := NewCache(&Config{ + NumCounters: 100, + MaxCost: 1000, + BufferItems: 64, + Metrics: true, + }) + + require.NoError(t, err) + + // Set initial value for key = 1 + insert := c.SetWithTTL(1, 1, 1, 800*time.Millisecond) + require.True(t, insert) + + time.Sleep(100 * time.Millisecond) + + // Get value from cache for key = 1 + val, ok := c.Get(1) + require.True(t, ok) + require.NotNil(t, val) + require.Equal(t, 1, val) + + time.Sleep(1200 * time.Millisecond) + + val, ok = c.Get(1) + require.False(t, ok) + require.Nil(t, val) + }) + } +} diff --git a/store.go b/store.go index 97ddc40d..e42a98b7 100644 --- a/store.go +++ b/store.go @@ -26,7 +26,7 @@ type storeItem struct { key uint64 conflict uint64 value interface{} - expiration int64 + expiration time.Time } // store is the interface fulfilled by all hash map implementations in this @@ -39,7 +39,7 @@ type store interface { // Get returns the value associated with the key parameter. Get(uint64, uint64) (interface{}, bool) // Expiration returns the expiration time for this key. - Expiration(uint64) int64 + Expiration(uint64) time.Time // Set adds the key-value pair to the Map or updates the value if it's // already present. The key-value pair is passed as a pointer to an // item object. @@ -82,7 +82,7 @@ func (sm *shardedMap) Get(key, conflict uint64) (interface{}, bool) { return sm.shards[key%numShards].get(key, conflict) } -func (sm *shardedMap) Expiration(key uint64) int64 { +func (sm *shardedMap) Expiration(key uint64) time.Time { return sm.shards[key%numShards].Expiration(key) } @@ -138,13 +138,13 @@ func (m *lockedMap) get(key, conflict uint64) (interface{}, bool) { } // Handle expired items. - if item.expiration != 0 && time.Now().Unix() > item.expiration { + if !item.expiration.IsZero() && time.Now().After(item.expiration) { return nil, false } return item.value, true } -func (m *lockedMap) Expiration(key uint64) int64 { +func (m *lockedMap) Expiration(key uint64) time.Time { m.RLock() defer m.RUnlock() return m.data[key].expiration @@ -193,7 +193,7 @@ func (m *lockedMap) Del(key, conflict uint64) (uint64, interface{}) { return 0, nil } - if item.expiration != 0 { + if !item.expiration.IsZero() { m.em.del(key, item.expiration) } diff --git a/store_test.go b/store_test.go index 052b427d..eea9440d 100644 --- a/store_test.go +++ b/store_test.go @@ -156,7 +156,7 @@ func TestStoreCollision(t *testing.T) { func TestStoreExpiration(t *testing.T) { s := newStore() key, conflict := z.KeyToHash(1) - expiration := time.Now().Add(time.Second).Unix() + expiration := time.Now().Add(time.Second) i := Item{ Key: key, Conflict: conflict, @@ -175,12 +175,12 @@ func TestStoreExpiration(t *testing.T) { _, ok = s.Get(key, conflict) require.False(t, ok) - require.Equal(t, int64(0), s.Expiration(key)) + require.True(t, s.Expiration(key).IsZero()) // missing item key, _ = z.KeyToHash(4340958203495) ttl = s.Expiration(key) - require.Equal(t, int64(0), ttl) + require.True(t, ttl.IsZero()) } func BenchmarkStoreGet(b *testing.B) { diff --git a/ttl.go b/ttl.go index 40a91bc1..337976ad 100644 --- a/ttl.go +++ b/ttl.go @@ -26,11 +26,11 @@ var ( bucketDurationSecs = int64(5) ) -func storageBucket(t int64) int64 { - return (t / bucketDurationSecs) + 1 +func storageBucket(t time.Time) int64 { + return (t.Unix() / bucketDurationSecs) + 1 } -func cleanupBucket(t int64) int64 { +func cleanupBucket(t time.Time) int64 { // The bucket to cleanup is always behind the storage bucket by one so that // no elements in that bucket (which might not have expired yet) are deleted. return storageBucket(t) - 1 @@ -51,13 +51,13 @@ func newExpirationMap() *expirationMap { } } -func (m *expirationMap) add(key, conflict uint64, expiration int64) { +func (m *expirationMap) add(key, conflict uint64, expiration time.Time) { if m == nil { return } // Items that don't expire don't need to be in the expiration map. - if expiration == 0 { + if expiration.IsZero() { return } @@ -73,7 +73,7 @@ func (m *expirationMap) add(key, conflict uint64, expiration int64) { b[key] = conflict } -func (m *expirationMap) update(key, conflict uint64, oldExpTime, newExpTime int64) { +func (m *expirationMap) update(key, conflict uint64, oldExpTime, newExpTime time.Time) { if m == nil { return } @@ -96,7 +96,7 @@ func (m *expirationMap) update(key, conflict uint64, oldExpTime, newExpTime int6 newBucket[key] = conflict } -func (m *expirationMap) del(key uint64, expiration int64) { +func (m *expirationMap) del(key uint64, expiration time.Time) { if m == nil { return } @@ -120,7 +120,7 @@ func (m *expirationMap) cleanup(store store, policy policy, onEvict itemCallback } m.Lock() - now := time.Now().Unix() + now := time.Now() bucketNum := cleanupBucket(now) keys := m.buckets[bucketNum] delete(m.buckets, bucketNum) @@ -128,7 +128,7 @@ func (m *expirationMap) cleanup(store store, policy policy, onEvict itemCallback for key, conflict := range keys { // Sanity check. Verify that the store agrees that this key is expired. - if store.Expiration(key) > now { + if store.Expiration(key).After(now) { continue }