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

Cache.expire() should cull items to respect size_limit #52

Closed
mrclary opened this issue Sep 13, 2017 · 11 comments
Closed

Cache.expire() should cull items to respect size_limit #52

mrclary opened this issue Sep 13, 2017 · 11 comments

Comments

@mrclary
Copy link

mrclary commented Sep 13, 2017

Grant,
I couldn't find a way to contact you regarding a question about DiskCache, except to file an issue.
I do not know how to cull items from the cache if the cache size is larger than the size_limit.

If I fill the cache to something larger than the size_limit, the cache does not report any items as expired, so calling the expire method does nothing. How is the size_limit parameter supposed to work?

@grantjenks
Copy link
Owner

The size_limit parameter should limit the size of the cache. It's a soft-limit though. How big is the cache getting?

Can you post some code to repro?

@grantjenks
Copy link
Owner

Also note that if you set the cull_limit to zero then the size_limit is ignored.

@grantjenks
Copy link
Owner

Another thing: expire() will only evict items that have expired from the cache. It won't evict for size constraints.

Maybe I understand your issue: do you have cull_limit set to zero? and are you calling expire() from a separate process to remove items? but observing that the cache is not obeying the size_limit? I could believe that's happening and that sounds like a bug.

@mrclary
Copy link
Author

mrclary commented Sep 13, 2017

That's exactly the issue I'm having. I expected expire() to trim the cache to the size_limit, using the eviction policy parameter. I do have the cull_limit set to zero.

What I would like to do is cull items manually, which is why I set cull_limit=0. I don't want items culled unless I call expire() explicitly. When I do cull items (by calling expire()) I'd like items removed according to the eviction policy (e.g. least-recently-stored) and meet both the expire time AND size_limit parameters.

Does a cache item only get flagged as expired if the expire time parameter is met?
How does the cache size "soft-limit" work?

@mrclary
Copy link
Author

mrclary commented Sep 13, 2017

I don't need a hard size limit. I just want to make sure I understand how it's supposed to work. I couldn't find anything in the documentation specifically addressing how the cache size limit is maintained, so I couldn't be sure if I was expecting the correct behavior.

@grantjenks
Copy link
Owner

I agree. That's a bug. I expected expire() to remove items to meet the size limit too.

I can best explain the "soft limit" with an example. Suppose the cache was initialized with its defaults: size_limit = 1GB and cull_limit = 10. Then suppose you fill the cache with one million 1KB items. That would put the cache right at the 1GB size limit. If you then store a 1MB item then to meet the size_limit, you need to evict one thousand 1KB items. But with the cull_limit set to 10, that won't happen. At least, not immediately. The cull_limit will limit the number of items removed during culling (which happens when an item is set() in the cache).

Does that make sense?

@grantjenks grantjenks changed the title Cache does not respect size_limit Cache.expire() should cull items to respect size_limit Sep 13, 2017
@grantjenks
Copy link
Owner

Renamed issue to capture the scenario. You can work-around the issue by doing something like the following:

def cull(cache):
    cache.cull_limit = 10  # Do NOT use reset()
    while cache.volume() > cache.size_limit:
        cache._cull()

@mrclary
Copy link
Author

mrclary commented Sep 13, 2017

did you mean the following?

def cull(cache):
    cache.cull_limit = 10  # Do NOT use reset()
    now = time.time()
    while cache.volume() > cache.size_limit:
        with cache._transact() as (sql, cleanup):
            cache._cull(now, sql, cleanup)

@grantjenks
Copy link
Owner

Yep, that looks right. That should work in a separate process. Careful about setting the cull_limit. Probably better to do:

def cull(path):
    with diskcache.Cache(path, timeout=60) as cache:
        cache.cull_limit = 10  # Do NOT use reset()
        now = time.time()
        while cache.volume() > cache.size_limit:
            with cache._transact() as (sql, cleanup):
                cache._cull(now, sql, cleanup)

@grantjenks
Copy link
Owner

I'm working on this with some other changes. Should be up on PyPI before the New Year.

@grantjenks
Copy link
Owner

Implementation complete at 434c128 will deploy in v3 to PyPI this week.

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

No branches or pull requests

2 participants