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

Implement a fix for #66, excessive memory use in siphon. #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

floren
Copy link
Contributor

@floren floren commented Aug 18, 2021

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.

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.
@peterbourgon
Copy link
Owner

Would it be smarter to check both d.CacheSizeMax and fi.Size in readWithRLock, and only create and use the siphon if the file size could conceivably fit in the cache?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants