Add TrackingSet method

This method reduces the likelihood of a race condition where
you can add a (tracked) item to the cache, and the item isn't
the item you thought it was.
This commit is contained in:
Sargun Dhillon 2020-08-13 10:43:38 -07:00
parent e9b7be5016
commit df91803297
9 changed files with 36 additions and 20 deletions

View file

@ -23,9 +23,9 @@ func (b *bucket) get(key string) *Item {
return b.lookup[key] return b.lookup[key]
} }
func (b *bucket) set(key string, value interface{}, duration time.Duration) (*Item, *Item) { func (b *bucket) set(key string, value interface{}, duration time.Duration, track bool) (*Item, *Item) {
expires := time.Now().Add(duration).UnixNano() expires := time.Now().Add(duration).UnixNano()
item := newItem(key, value, expires) item := newItem(key, value, expires, track)
b.Lock() b.Lock()
existing := b.lookup[key] existing := b.lookup[key]
b.lookup[key] = item b.lookup[key] = item

View file

@ -1,9 +1,10 @@
package ccache package ccache
import ( import (
. "github.com/karlseguin/expect"
"testing" "testing"
"time" "time"
. "github.com/karlseguin/expect"
) )
type BucketTests struct { type BucketTests struct {
@ -32,7 +33,7 @@ func (_ *BucketTests) DeleteItemFromBucket() {
func (_ *BucketTests) SetsANewBucketItem() { func (_ *BucketTests) SetsANewBucketItem() {
bucket := testBucket() bucket := testBucket()
item, existing := bucket.set("spice", TestValue("flow"), time.Minute) item, existing := bucket.set("spice", TestValue("flow"), time.Minute, false)
assertValue(item, "flow") assertValue(item, "flow")
item = bucket.get("spice") item = bucket.get("spice")
assertValue(item, "flow") assertValue(item, "flow")
@ -41,7 +42,7 @@ func (_ *BucketTests) SetsANewBucketItem() {
func (_ *BucketTests) SetsAnExistingItem() { func (_ *BucketTests) SetsAnExistingItem() {
bucket := testBucket() bucket := testBucket()
item, existing := bucket.set("power", TestValue("9001"), time.Minute) item, existing := bucket.set("power", TestValue("9001"), time.Minute, false)
assertValue(item, "9001") assertValue(item, "9001")
item = bucket.get("power") item = bucket.get("power")
assertValue(item, "9001") assertValue(item, "9001")

View file

@ -98,9 +98,15 @@ func (c *Cache) TrackingGet(key string) TrackedItem {
return item return item
} }
// Used when the cache was created with the Track() configuration option.
// Sets the item, and returns a tracked reference to it.
func (c *Cache) TrackingSet(key string, value interface{}, duration time.Duration) TrackedItem {
return c.set(key, value, duration, true)
}
// Set the value in the cache for the specified duration // Set the value in the cache for the specified duration
func (c *Cache) Set(key string, value interface{}, duration time.Duration) { func (c *Cache) Set(key string, value interface{}, duration time.Duration) {
c.set(key, value, duration) c.set(key, value, duration, false)
} }
// Replace the value if it exists, does not set if it doesn't. // Replace the value if it exists, does not set if it doesn't.
@ -127,7 +133,7 @@ func (c *Cache) Fetch(key string, duration time.Duration, fetch func() (interfac
if err != nil { if err != nil {
return nil, err return nil, err
} }
return c.set(key, value, duration), nil return c.set(key, value, duration, false), nil
} }
// Remove the item from the cache, return true if the item was present, false otherwise. // Remove the item from the cache, return true if the item was present, false otherwise.
@ -182,8 +188,8 @@ func (c *Cache) deleteItem(bucket *bucket, item *Item) {
c.deletables <- item c.deletables <- item
} }
func (c *Cache) set(key string, value interface{}, duration time.Duration) *Item { func (c *Cache) set(key string, value interface{}, duration time.Duration, track bool) *Item {
item, existing := c.bucket(key).set(key, value, duration) item, existing := c.bucket(key).set(key, value, duration, track)
if existing != nil { if existing != nil {
c.deletables <- existing c.deletables <- existing
} }

View file

@ -140,18 +140,21 @@ func (_ CacheTests) PromotedItemsDontGetPruned() {
} }
func (_ CacheTests) TrackerDoesNotCleanupHeldInstance() { func (_ CacheTests) TrackerDoesNotCleanupHeldInstance() {
cache := New(Configure().ItemsToPrune(10).Track()) cache := New(Configure().ItemsToPrune(11).Track())
for i := 0; i < 10; i++ { item0 := cache.TrackingSet("0", 0, time.Minute)
for i := 1; i < 11; i++ {
cache.Set(strconv.Itoa(i), i, time.Minute) cache.Set(strconv.Itoa(i), i, time.Minute)
} }
item := cache.TrackingGet("0") item1 := cache.TrackingGet("1")
time.Sleep(time.Millisecond * 10) time.Sleep(time.Millisecond * 10)
gcCache(cache) gcCache(cache)
Expect(cache.Get("0").Value()).To.Equal(0) Expect(cache.Get("0").Value()).To.Equal(0)
Expect(cache.Get("1")).To.Equal(nil) Expect(cache.Get("1").Value()).To.Equal(1)
item.Release() item0.Release()
item1.Release()
gcCache(cache) gcCache(cache)
Expect(cache.Get("0")).To.Equal(nil) Expect(cache.Get("0")).To.Equal(nil)
Expect(cache.Get("1")).To.Equal(nil)
} }
func (_ CacheTests) RemovesOldestItemWhenFull() { func (_ CacheTests) RemovesOldestItemWhenFull() {

View file

@ -1,8 +1,9 @@
package ccache package ccache
import ( import (
. "github.com/karlseguin/expect"
"testing" "testing"
. "github.com/karlseguin/expect"
) )
type ConfigurationTests struct{} type ConfigurationTests struct{}

View file

@ -52,18 +52,22 @@ type Item struct {
element *list.Element element *list.Element
} }
func newItem(key string, value interface{}, expires int64) *Item { func newItem(key string, value interface{}, expires int64, track bool) *Item {
size := int64(1) size := int64(1)
if sized, ok := value.(Sized); ok { if sized, ok := value.(Sized); ok {
size = sized.Size() size = sized.Size()
} }
return &Item{ item := &Item{
key: key, key: key,
value: value, value: value,
promotions: 0, promotions: 0,
size: size, size: size,
expires: expires, expires: expires,
} }
if track {
item.refCount = 1
}
return item
} }
func (i *Item) shouldPromote(getsPerPromote int32) bool { func (i *Item) shouldPromote(getsPerPromote int32) bool {

View file

@ -46,7 +46,7 @@ func (b *layeredBucket) set(primary, secondary string, value interface{}, durati
b.buckets[primary] = bkt b.buckets[primary] = bkt
} }
b.Unlock() b.Unlock()
item, existing := bkt.set(secondary, value, duration) item, existing := bkt.set(secondary, value, duration, false)
item.group = primary item.group = primary
return item, existing return item, existing
} }

View file

@ -16,7 +16,7 @@ func (s *SecondaryCache) Get(secondary string) *Item {
// Set the secondary key to a value. // Set the secondary key to a value.
// The semantics are the same as for LayeredCache.Set // The semantics are the same as for LayeredCache.Set
func (s *SecondaryCache) Set(secondary string, value interface{}, duration time.Duration) *Item { func (s *SecondaryCache) Set(secondary string, value interface{}, duration time.Duration) *Item {
item, existing := s.bucket.set(secondary, value, duration) item, existing := s.bucket.set(secondary, value, duration, false)
if existing != nil { if existing != nil {
s.pCache.deletables <- existing s.pCache.deletables <- existing
} }

View file

@ -1,10 +1,11 @@
package ccache package ccache
import ( import (
. "github.com/karlseguin/expect"
"strconv" "strconv"
"testing" "testing"
"time" "time"
. "github.com/karlseguin/expect"
) )
type SecondaryCacheTests struct{} type SecondaryCacheTests struct{}