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

Fix the way metrics are handled for deletions. #111

Merged
merged 6 commits into from
Dec 18, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,20 +266,19 @@ func (c *Cache) processItems() {
if added {
c.store.Set(i.key, i.conflict, i.value)
c.Metrics.add(keyAdd, i.key, 1)
c.Metrics.add(costAdd, i.key, uint64(i.cost))
}
for _, victim := range victims {
victim.conflict, victim.value = c.store.Del(victim.key, 0)
if c.onEvict != nil {
c.onEvict(victim.key, victim.conflict, victim.value, victim.cost)
}
c.Metrics.add(keyEvict, victim.key, 1)
c.Metrics.add(costEvict, victim.key, uint64(victim.cost))
}

case itemUpdate:
c.policy.Update(i.key, i.cost)

case itemDelete:
c.policy.Del(i.key)
c.policy.Del(i.key) // Deals with metrics updates.
c.store.Del(i.key, i.conflict)
}
case <-c.stop:
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ go 1.12
require (
github.com/cespare/xxhash v1.1.0
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
)
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@ github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko=
github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2 h1:tdlZCpZ/P9DhczCTSixgIKmwPv6+wP5DGjqLYw5SUiA=
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2/go.mod h1:SqUrOPUnsFjfmXRMNPybcSiG0BgUW2AuFH8PAnS2iTw=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72 h1:qLC7fQah7D6K1B0ujays3HV9gkFtllcxhzImRR7ArPQ=
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
15 changes: 14 additions & 1 deletion policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ func (p *defaultPolicy) Add(key uint64, cost int64) ([]*item, bool) {
}
// we don't need to go any further if the item is already in the cache
if has := p.evict.updateIfHas(key, cost); has {
return nil, true
// If we are just updating, we are not adding, so this should return a false.
return nil, false
}
// if we got this far, this key doesn't exist in the cache
//
Expand All @@ -137,6 +138,7 @@ func (p *defaultPolicy) Add(key uint64, cost int64) ([]*item, bool) {
// there's enough room in the cache to store the new item without
// overflowing, so we can do that now and stop here
p.evict.add(key, cost)
p.metrics.add(costAdd, key, uint64(cost))
return nil, true
}
// incHits is the hit count for the incoming item
Expand Down Expand Up @@ -169,6 +171,7 @@ func (p *defaultPolicy) Add(key uint64, cost int64) ([]*item, bool) {
}
// delete the victim from metadata
p.evict.del(minKey)

// delete the victim from sample
sample[minId] = sample[len(sample)-1]
sample = sample[:len(sample)-1]
Expand All @@ -180,6 +183,7 @@ func (p *defaultPolicy) Add(key uint64, cost int64) ([]*item, bool) {
})
}
p.evict.add(key, cost)
p.metrics.add(costAdd, key, uint64(cost))
return victims, true
}

Expand Down Expand Up @@ -272,6 +276,8 @@ func (p *sampledLFU) del(key uint64) {
}
p.used -= cost
delete(p.keyCosts, key)
p.metrics.add(costEvict, key, uint64(cost))
p.metrics.add(keyEvict, key, 1)
}

func (p *sampledLFU) add(key uint64, cost int64) {
Expand All @@ -284,6 +290,13 @@ func (p *sampledLFU) updateIfHas(key uint64, cost int64) bool {
// update the cost of an existing key, but don't worry about evicting,
// evictions will be handled the next time a new item is added
p.metrics.add(keyUpdate, key, 1)
if prev > cost {
diff := prev - cost
p.metrics.add(costAdd, key, ^uint64(uint64(diff)-1))
} else if cost > prev {
diff := cost - prev
p.metrics.add(costAdd, key, uint64(diff))
}
p.used += cost - prev
p.keyCosts[key] = cost
return true
Expand Down