Closed WinstonPrivacy closed 4 years ago
I've fixed or improved a number of things related to this in my fork:
If ensureCacheSpaceWithLock removes the same key that it adds, the cache size is incorrectly calculated.
Updating a key with a smaller sized one incorrectly updates the cache size.
ensureCacheSpaceWithLock now clears out 15% of the cache when it exceeds the limit. Previously, it would clear just enough space to bring it under the limit, and the cache would fill up on the next new key. This causes a lot of CPU thrashing.
Nice! Would love to see a PR with these fixes (in isolation).
@WinstonPrivacy I am seeing this same issue. Can you please point me to your fork or raise a PR ? Thanks !
Here you go:
Thanks !
I got bitten by a panic in ensureCacheSpaceWithLock over the weekend, although I'm not clear what I did to trigger it. Any thoughts on building a PR? I look at look at WinstonPrivacy's fork but I'm afraid I can't be sure exactly which commits are relevant to this issue and which are for other things.
Edit: I've written a test to check it.
If variable sized keys are being used, then ensureCacheSpaceWithLock() can panic during the cache cleanup routine which loops through safe(). This occurs if the key being inserted is larger than the last key removed.
It would be better if either this key were inserted anyway (and thus exceeding the cache threshold) or the routine was modified so that it cleared some percent of the total cache size before proceeding.