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

Disk cache grows beyond the limit #45

Closed
toubib opened this issue Sep 16, 2015 · 6 comments
Closed

Disk cache grows beyond the limit #45

toubib opened this issue Sep 16, 2015 · 6 comments

Comments

@toubib
Copy link
Contributor

toubib commented Sep 16, 2015

I've done a load test on my installation with this configuration (there is some options from my fork):

/usr/bin/imageproxy -addr=localhost:8980 -cacheDir=/data/imageproxy/cache -cacheSize=1024 -log_dir=/data/imageproxy/log -filetypes=png,PNG,jpg,JPG,jpeg,JPEG,gif,GIF -signatureKey=xxx -logrotatecompatible=true

In front of that there is a nginx with a disk cache (who doesn't work the same than imageproxy cache as imageproxy still make a head request to the original server).

I've requested lot of different files and the imageproxy cache is now 3,2GiB with 56197 files. That's far beyond of the 1014MB given on the command line. Am I missing something ?

Since I already have a cache with nginx, it is possible to add an option to disable imageproxy cache ?

@toubib
Copy link
Contributor Author

toubib commented Sep 16, 2015

Another thing, the cache files are stored flat on the filesystem (that's the default option for the diskv
module), this can be a performance problem for large cache. Most of the cache systems use 2 or 3 levels of subdirectories.

@toubib
Copy link
Contributor Author

toubib commented Sep 21, 2015

I don't understand this code neither:

var cacheDir = flag.String("cacheDir", "", "directory to use for file cache")
var cacheSize = flag.Uint64("cacheSize", 100, "maximum size of file cache (in MB)")
...
    var c httpcache.Cache
    if *cacheDir != "" {
        d := diskv.New(diskv.Options{
            BasePath:     *cacheDir,
            CacheSizeMax: *cacheSize * 1024 * 1024,
        })
        c = diskcache.NewWithDiskv(d)
    } else if *cacheSize != 0 {
        c = httpcache.NewMemoryCache()
    }

@toubib
Copy link
Contributor Author

toubib commented Sep 21, 2015

Maybe to do something like this ?

    var c httpcache.Cache
    if *fileCache {
        if *cacheDir != "" {
            d := diskv.New(diskv.Options{
                BasePath:     *cacheDir,
                CacheSizeMax: *cacheSize * 1024 * 1024,
            })
            c = diskcache.NewWithDiskv(d)
        } //TODO else error ?
    } else if *memCache {
        c = httpcache.NewMemoryCache()
    } else {
        c = nil
    }

@toubib
Copy link
Contributor Author

toubib commented Sep 21, 2015

Done it there toubib@6a0c813, no cache mode added.

@willnorris
Copy link
Owner

I just filed #50 to track modifying the diskv structure. Making the no-cache mode simpler to use will be covered by #49. I'll leave this open to track the original reported issue of the disk cache growing too large.

@willnorris
Copy link
Owner

okay, so I figured out why this is happening. It's because I completely misunderstood what the CacheSizeMax option in the diskv library is for. 😞 diskv has its own in-memory cache of the files that it stores on disk, and CacheSizeMax controls the size of that cache, not the files on disk. Which, now that I think more about it, completely makes sense... diskv is designed to be an arbitrary key-value store, so you wouldn't want it randomly deleting files on disk.

This means that we don't actually have a way to limit the size of the image cache at this time for any of the backends. I'm going remove the cacheSize related code, since it's not doing anything meaningful. I'll leave the flag for the time being so that it doesn't cause panics, but it should no longer be used.

willnorris added a commit that referenced this issue Dec 8, 2015
this flag was never actually doing what I thought it was in the first
place.

Also fix up a few instances of cacheDir still be used in config files

fixes #45
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