Skip to content

Commit

Permalink
change expiration type from int64 to time.Time (#277)
Browse files Browse the repository at this point in the history
  • Loading branch information
Warashi authored May 21, 2021
1 parent 0bf2acd commit 9d4946d
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 25 deletions.
13 changes: 6 additions & 7 deletions cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ type Item struct {
Conflict uint64
Value interface{}
Cost int64
Expiration int64
Expiration time.Time
wg *sync.WaitGroup
}

Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
36 changes: 36 additions & 0 deletions cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
12 changes: 6 additions & 6 deletions store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
18 changes: 9 additions & 9 deletions ttl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -120,15 +120,15 @@ 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)
m.Unlock()

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
}

Expand Down

0 comments on commit 9d4946d

Please sign in to comment.