Skip to content

Commit

Permalink
Implement a fix for peterbourgon#66, excessive memory use in siphon.
Browse files Browse the repository at this point in the history
The siphon will now stop writing to its internal buffer once the size of the buffer
exceeds the maximum cache size. Because we write until we *exceed* the max cache size,
we're safe to attempt the cache update even if the buffer only contains partial data,
because it's still over the limit & will be rejected.
  • Loading branch information
floren committed Aug 18, 2021
1 parent fc05534 commit 4498f7b
Showing 1 changed file with 13 additions and 6 deletions.
19 changes: 13 additions & 6 deletions diskv.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,15 +466,22 @@ func (s *siphon) Read(p []byte) (int, error) {
n, err := s.f.Read(p)

if err == nil {
return s.buf.Write(p[0:n]) // Write must succeed for Read to succeed
}

if err == io.EOF {
s.d.cacheWithoutLock(s.key, s.buf.Bytes()) // cache may fail
// Only write into the buffer if the buffer is not yet over the size of the cache.
// This logic guarantees that we'll write into the buffer until one of two things happens:
// 1. We write the entire contents of the source into the buffer.
// 2. We've read enough into the buffer to exceed the max cache size.
// If we read more than the max cache size into the buffer, our later attempt
// to update the cache will be rejected (because the value exceeds the max cache size);
// it does *not* cache a partial value.
if uint64(s.buf.Len()) < s.d.CacheSizeMax {
return s.buf.Write(p[0:n]) // Write must succeed for Read to succeed
}
} else if err == io.EOF {
// The cache will reject this if we've exceeded the maximum cache size
s.d.cacheWithoutLock(s.key, s.buf.Bytes())
if closeErr := s.f.Close(); closeErr != nil {
return n, closeErr // close must succeed for Read to succeed
}
return n, err
}

return n, err
Expand Down

0 comments on commit 4498f7b

Please sign in to comment.