Skip to content

Commit 1233dd1

Browse files
committed
Replace entry.Key interface with string
We only use non-string Keys in tests, so let's change the tests to use strings also and drop the interface{}.
1 parent 138ef30 commit 1233dd1

File tree

5 files changed

+49
-49
lines changed

5 files changed

+49
-49
lines changed

cache/disk/disk.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -149,20 +149,24 @@ func (c *diskCache) pollCacheAge() {
149149
func (c *diskCache) updateCacheAgeMetric() {
150150
c.mu.Lock()
151151

152-
key, value := c.lru.getTailItem()
152+
key, value, ok := c.lru.getTailItem()
153+
if !ok {
154+
// No items in the cache.
155+
c.mu.Unlock()
156+
return
157+
}
158+
153159
age := 0.0
154160
validAge := true
155161

156-
if key != nil {
157-
f := c.getElementPath(key, value)
158-
ts, err := atime.Stat(f)
162+
f := c.getElementPath(key, value)
163+
ts, err := atime.Stat(f)
159164

160-
if err != nil {
161-
log.Printf("ERROR: failed to determine time of least recently used cache item: %v, unable to stat %s", err, f)
162-
validAge = false
163-
} else {
164-
age = time.Since(ts).Seconds()
165-
}
165+
if err != nil {
166+
log.Printf("ERROR: failed to determine time of least recently used cache item: %v, unable to stat %s", err, f)
167+
validAge = false
168+
} else {
169+
age = time.Since(ts).Seconds()
166170
}
167171

168172
c.mu.Unlock()
@@ -172,8 +176,8 @@ func (c *diskCache) updateCacheAgeMetric() {
172176
}
173177
}
174178

175-
func (c *diskCache) getElementPath(key Key, value lruItem) string {
176-
ks := key.(string)
179+
func (c *diskCache) getElementPath(key string, value lruItem) string {
180+
ks := key
177181
hash := ks[len(ks)-sha256.Size*2:]
178182
var kind = cache.AC
179183
if strings.HasPrefix(ks, "cas") {

cache/disk/disk_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -454,10 +454,10 @@ func TestCacheExistingFiles(t *testing.T) {
454454
}
455455
testCache := testCacheI.(*diskCache)
456456

457-
evicted := []Key{}
457+
evicted := []string{}
458458
origOnEvict := testCache.lru.onEvict
459-
testCache.lru.onEvict = func(key Key, value lruItem) {
460-
evicted = append(evicted, key.(string))
459+
testCache.lru.onEvict = func(key string, value lruItem) {
460+
evicted = append(evicted, key)
461461
origOnEvict(key, value)
462462
}
463463

cache/disk/load.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ func (c *diskCache) loadExistingFiles(maxSizeBytes int64, cc CacheConfig) error
583583
// The eviction callback deletes the file from disk.
584584
// This function is only called while the lock is not held
585585
// by the current goroutine.
586-
onEvict := func(key Key, value lruItem) {
586+
onEvict := func(key string, value lruItem) {
587587
f := c.getElementPath(key, value)
588588
c.removeFile(f)
589589
}

cache/disk/lru.go

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,8 @@ import (
1111
"github.com/prometheus/client_golang/prometheus"
1212
)
1313

14-
// Key is the type used for identifying cache items. For non-test code,
15-
// this is a string of the form "<keyspace>/<hash>". Some test code uses
16-
// ints for simplicity.
17-
//
18-
// TODO: update the test code to use strings too, then drop all the
19-
// type assertions.
20-
type Key interface{}
21-
2214
// EvictCallback is the type of callbacks that are invoked when items are evicted.
23-
type EvictCallback func(key Key, value lruItem)
15+
type EvictCallback func(key string, value lruItem)
2416

2517
// SizedLRU is an LRU cache that will keep its total size below maxSize by evicting
2618
// items.
@@ -96,7 +88,10 @@ type SizedLRU struct {
9688
}
9789

9890
type entry struct {
99-
key Key
91+
// This is used to identify cache items. For non-test code,
92+
// this is a string of the form "<keyspace>/<hash>"
93+
key string
94+
10095
value lruItem
10196
}
10297

@@ -175,7 +170,7 @@ func (c *SizedLRU) RegisterMetrics() {
175170
// Note that this function rounds file sizes up to the nearest
176171
// BlockSize (4096) bytes, as an estimate of actual disk usage since
177172
// most linux filesystems default to 4kb blocks.
178-
func (c *SizedLRU) Add(key Key, value lruItem) (ok bool) {
173+
func (c *SizedLRU) Add(key string, value lruItem) (ok bool) {
179174

180175
roundedUpSizeOnDisk := roundUp4k(value.sizeOnDisk)
181176

@@ -237,7 +232,7 @@ func (c *SizedLRU) Add(key Key, value lruItem) (ok bool) {
237232
}
238233

239234
// Get looks up a key in the cache
240-
func (c *SizedLRU) Get(key Key) (value lruItem, ok bool) {
235+
func (c *SizedLRU) Get(key string) (value lruItem, ok bool) {
241236
if ele, hit := c.cache[key]; hit {
242237
c.ll.MoveToFront(ele)
243238
return ele.Value.(*entry).value, true
@@ -247,7 +242,7 @@ func (c *SizedLRU) Get(key Key) (value lruItem, ok bool) {
247242
}
248243

249244
// Remove removes a (key, value) from the cache
250-
func (c *SizedLRU) Remove(key Key) {
245+
func (c *SizedLRU) Remove(key string) {
251246
if ele, hit := c.cache[key]; hit {
252247
c.removeElement(ele)
253248
c.gaugeCacheLogicalBytes.Set(float64(c.uncompressedSize))
@@ -409,13 +404,13 @@ func roundUp4k(n int64) int64 {
409404
}
410405

411406
// Get the back item of the LRU cache.
412-
func (c *SizedLRU) getTailItem() (Key, lruItem) {
407+
func (c *SizedLRU) getTailItem() (string, lruItem, bool) {
413408
ele := c.ll.Back()
414409
if ele != nil {
415410
kv := ele.Value.(*entry)
416-
return kv.key, kv.value
411+
return kv.key, kv.value, true
417412
}
418-
return nil, lruItem{}
413+
return "", lruItem{}, false
419414
}
420415

421416
// Append an entry to the eviction queue. The entry must have been removed

cache/disk/lru_test.go

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"math"
55
"net/http"
66
"reflect"
7+
"strconv"
78
"testing"
89

910
testutils "github.com/buchgr/bazel-remote/v2/utils"
@@ -63,33 +64,33 @@ func TestBasics(t *testing.T) {
6364

6465
func TestEviction(t *testing.T) {
6566
// Keep track of evictions using the callback
66-
var evictions []int
67-
onEvict := func(key Key, value lruItem) {
68-
evictions = append(evictions, key.(int))
67+
var evictions []string
68+
onEvict := func(key string, value lruItem) {
69+
evictions = append(evictions, key)
6970
}
7071

7172
lru := NewSizedLRU(10*BlockSize, onEvict, 0)
7273

7374
expectedSizesNumItems := []struct {
7475
expBlocks int64
7576
expNumItems int
76-
expEvicted []int
77+
expEvicted []string
7778
}{
78-
{0, 1, []int{}}, // 0
79-
{1, 2, []int{}}, // 0, 1
80-
{3, 3, []int{}}, // 0, 1, 2
81-
{6, 4, []int{}}, // 0, 1, 2, 3
82-
{10, 5, []int{}}, // 0, 1, 2, 3, 4
83-
{9, 2, []int{0, 1, 2, 3}}, // 4, 5
84-
{6, 1, []int{4, 5}}, // 6
85-
{7, 1, []int{6}}, // 7
79+
{0, 1, []string{}}, // 0
80+
{1, 2, []string{}}, // 0, 1
81+
{3, 3, []string{}}, // 0, 1, 2
82+
{6, 4, []string{}}, // 0, 1, 2, 3
83+
{10, 5, []string{}}, // 0, 1, 2, 3, 4
84+
{9, 2, []string{"0", "1", "2", "3"}}, // 4, 5
85+
{6, 1, []string{"4", "5"}}, // 6
86+
{7, 1, []string{"6"}}, // 7
8687
}
8788

88-
var expectedEvictions []int
89+
var expectedEvictions []string
8990

9091
for i, thisExpected := range expectedSizesNumItems {
9192
item := lruItem{size: int64(i) * BlockSize, sizeOnDisk: int64(i) * BlockSize}
92-
ok := lru.Add(i, item)
93+
ok := lru.Add(strconv.Itoa(i), item)
9394
if !ok {
9495
t.Fatalf("Add: failed adding %d", i)
9596
}
@@ -163,13 +164,13 @@ func TestReserveAtCapacity(t *testing.T) {
163164

164165
func TestReserveAtEvictionQueueLimit(t *testing.T) {
165166

166-
lru := NewSizedLRU(BlockSize*2, func(Key, lruItem) {}, 0)
167+
lru := NewSizedLRU(BlockSize*2, func(string, lruItem) {}, 0)
167168
lru.maxSizeHardLimit = BlockSize * 3
168169

169-
blockSize1Key := 7777
170+
blockSize1Key := "7777"
170171
blockSize1Item := lruItem{size: BlockSize * 1, sizeOnDisk: BlockSize * 1}
171172

172-
blockSize2Key := 8888
173+
blockSize2Key := "8888"
173174
blockSize2Item := lruItem{size: BlockSize * 2, sizeOnDisk: BlockSize * 2}
174175

175176
// Add large item.

0 commit comments

Comments
 (0)