Skip to content

Commit

Permalink
[disk] SizedLRU.Reserve should return a non-nil error if unable to re…
Browse files Browse the repository at this point in the history
…serve the required space

Relates to buchgr#649
  • Loading branch information
mostynb committed Mar 11, 2023
1 parent c5bf6e1 commit 4855aff
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 16 deletions.
5 changes: 4 additions & 1 deletion cache/disk/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,10 @@ func (c *diskCache) Put(ctx context.Context, kind cache.EntryKind, hash string,
ok, err := c.lru.Reserve(size)
if err != nil {
c.mu.Unlock()
return internalErr(err)
return &cache.Error{
Code: http.StatusInsufficientStorage,
Text: err.Error(),
}
}
if !ok {
c.mu.Unlock()
Expand Down
10 changes: 7 additions & 3 deletions cache/disk/lru.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,19 @@ func (c *SizedLRU) Reserve(size int64) (bool, error) {
return true, nil
}

if size < 0 || size > c.maxSize {
return false, nil
if size < 0 {
return false, fmt.Errorf("Invalid negative blob size: %d", size)
}

if size > c.maxSize {
return false, fmt.Errorf("Unable to reserve space for blob (size: %d) larger than cache size %d", size, c.maxSize)
}

if sumLargerThan(size, c.reservedSize, c.maxSize) {
// If size + c.reservedSize is larger than c.maxSize
// then we cannot evict enough items to make enough
// space.
return false, nil
return false, fmt.Errorf("INTERNAL ERROR: unable to reserve enough space for blob with size %d (undersized cache?)", size)
}

// Evict elements until we are able to reserve enough space.
Expand Down
15 changes: 3 additions & 12 deletions cache/disk/lru_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,7 @@ func TestReserveAtCapacity(t *testing.T) {
}

ok, err = lru.Reserve(1)
if err != nil {
t.Fatal(err)
}
if ok {
if ok || err == nil {
t.Fatal("Should not be able to reserve any space")
}
if lru.TotalSize() != math.MaxInt64 {
Expand All @@ -184,19 +181,13 @@ func TestReserveOverflow(t *testing.T) {
}

ok, err = lru.Reserve(math.MaxInt64)
if err != nil {
t.Fatal(err)
}
if ok {
if ok || err == nil {
t.Fatal("Expected overflow")
}

lru = NewSizedLRU(10, nil, 0)
ok, err = lru.Reserve(math.MaxInt64)
if err != nil {
t.Fatal(err)
}
if ok {
if ok || err == nil {
t.Fatal("Expected overflow")
}
}
Expand Down

0 comments on commit 4855aff

Please sign in to comment.