[disk] SizedLRU.Reserve should return a non-nil error if unable to reserve the required space

cache/disk/disk.go

@@ -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()

cache/disk/lru.go

@@ -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.

cache/disk/lru_test.go

@@ -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 {
@@ -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")
 	}
 }