Fix some memory leaks in TTL implementation #358
Conversation
Woutifier
requested review from
joshua-goldstein,
mangalaman93,
harshil-goel and
all-seeing-code
as code owners
|
|
|
|
Woutifier
force-pushed
the
wouter/clear-expiry
branch
from
c5411c1 to
ff4f885
Compare
|
This PR has been stale for 60 days and will be closed automatically in 7 days. Comment to keep it open. |
|
Still relevant. Should this project be archived, or marked as deprecated? These are serious issues that have gone unaddressed for a very long time. |
|
Hi @Woutifier, Thanks for the PR and apologies for the delay. I am reviewing this PR on priority. Meanwhile, if you get a chance, feel free to merge/rebase with/on main. |
Expiry cleanup is not guaranteed to run at any interval. It attemps to run roughly twice per bucket duration, but may in fact not run. If the cleanup does not run at least one per bucket duration, one or more buckets may be leaked. This commit adds a field lastCleanedBucketNum to the expirationMap to keep track of the last bucket that was cleaned up. When cleanup is called will clean up all the buckets between the last bucket and the current bucket.
…o expiration If an item was already in the cache, adding it again will cause it to be added into the expiration map, even if the item has no expiration set. This effectively is a memory leak. There is no clean up of this "zero time" bucket, and it will keep growing.
mangalaman93
force-pushed
the
wouter/clear-expiry
branch
from
ff4f885 to
371c81a
Compare
|
Thanks for taking this PR into consideration :-). Let me know if there's anything I can do to help |
cleanup now does not try to clean an empty bucket selected by the cleanup algorithm this fixes a critical problem of slice growth when the algorithm incorrectly calculates the buckets that need to be cleared, let's say this behavior occurs at the time change described in issue: #426 this problem appeared after improving cleanup in this pr: #358 most likely the algorithm for calculating the cleaning bins also needs to be revised, but for me now it is important to fix the increase in memory consumption when changing the system time
The aim of this PR is to resolve three issues with the Ristretto TTL implementation: