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

Handle automatic cache purge (avoid capacity overflow) #6678

Open
chillu opened this issue Mar 5, 2017 · 7 comments
Open

Handle automatic cache purge (avoid capacity overflow) #6678

chillu opened this issue Mar 5, 2017 · 7 comments

Comments

@chillu
Copy link
Member

chillu commented Mar 5, 2017

Overview

Followup to #6252: We're using a new cache library, and still defaulting system caches to Filesystem (although we're choosing APC/opcache if available). As opposed to the old Zend_Cache solution, there's no way to limit the size of the cache. This leads to potential server capcity issues when old cache keys accumulate, particularly if they're based on timestamps as part of the cache key. The server might run out of disk space, or exhaust its filesystem nodes.

This issue should be solved in symfony/cache itself, so I've filed it there: symfony/symfony#21764 (comment)

Depending on the outcome, we might need to do more work in the framework to ensure a new purge() API is used automatically.

Acceptance Criteria

  • Expired cache entries get purged
  • This is enforced automatically (in-process cache purge for expired entries)
  • In-process cache purges don't exhaust server resources (PHP exec or mem)
  • Devs can opt in to background cache purging via a cron task
  • Continues to support non-expiring cache entries (acknowledging that they might get garbage collected when file size limit is reached)
  • Continues to support caches which aren't filesystem based (and do their own garbage collection)
  • Cache behaviour is well documented

Notes

  • Talk to platform operations about impact to their hosting context
@kinglozzer
Copy link
Member

@chillu That functionality has been added now (3.4+), I’m guessing this still needs to be addressed?

@chillu
Copy link
Member Author

chillu commented Mar 14, 2018

@kinglozzer Looking at the commit, it seems that there's a manual prune task - which we'd need to advise SilverStripe devs to set up as a cron job. Until we both upgrade to Symfony 3.4 and add those docs, I'd keep this issue open.

@chillu
Copy link
Member Author

chillu commented Dec 20, 2018

Documented here: https://symfony.com/doc/3.4/components/cache/cache_pools.html#pruning-cache-items

I think we can't really rely on cron jobs, and should do it the Zend way of randomly calling this prune command when caches are accessed. This will lead to some instability in request times, but it's no better or worse than in 3.x - where people largely didn't even notice this was happening. Note that not all cache interfaces support automatic pruning, so we'll need to add that to our documentation on how caching works. And recommend for people to disable this behaviour in favour of a cron job.

@chillu
Copy link
Member Author

chillu commented Dec 20, 2018

Raising up in backlog because there's no easy workaround, other than completely wiping cache (incl. non-expired items), which can lead to cache stampedes

@dnsl48 dnsl48 assigned dnsl48 and unassigned dnsl48 Jan 6, 2019
@dnsl48 dnsl48 self-assigned this Jan 13, 2019
@dnsl48
Copy link
Contributor

dnsl48 commented Jan 13, 2019

Here's the initial implementation of the Symfony/Cache component PruneableInterface. symfony/symfony@44d1162

@dnsl48 dnsl48 removed their assignment Jan 15, 2019
@dnsl48 dnsl48 self-assigned this Feb 7, 2019
@dnsl48 dnsl48 removed their assignment Apr 14, 2019
@kinglozzer
Copy link
Member

We’ve been reminded about this again - on higher traffic sites, there are a few things that appear to grow unbounded in TEMP_FOLDER (plus any usercode which implements caching, which can be much more likely to cause problems):

  • ./Intervention_Manipulations - errors have an increasing TTL, so that part is ready for pruning. Dimensions are cached, but without a TTL, so I think we should consider adding one here even if it’s a really long one. Currently they’ve never cleared as far as I can tell, even when a file is deleted
  • ./ratelimiter - spammers are causing thousands of files to be created here for our client. These do have a TTL though, so are ready for pruning
  • ./Sha1FileHashingService - these don’t have a TTL. Not too familiar with how this API is used, so not sure if allowing the cache entries to expire would be appropriate
  • ./ThemeResourceLoader - I think this is a separate “bug”: entering random URLs will add cache entries, something to do with controller checking if a template exists for myrandomurlslug. Probably shouldn’t ever expire

The challenge I encountered is how to fetch a list of cache instances to prune. We rely heavily on Injector for configuring things like namespaces, so this is the best I could come up with:

app/src/Tasks/PruneCachesTask.php
<?php

namespace App\Tasks;

use Psr\SimpleCache\CacheInterface;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Dev\BuildTask;
use Symfony\Component\Cache\PruneableInterface;

class PruneCachesTask extends BuildTask
{
    private static $segment = 'PruneCachesTask';

    protected $title = 'Prune Caches Task';

    protected $description = 'Prunes expired cache entries from filesystem-based caches';

    public function run($request)
    {
        $injectorServices = Config::inst()->get(Injector::class) ?: [];
        $cacheServices = array_filter($injectorServices, function($value, $key) {
            if (is_array($value) && isset($value['class'])) {
                $class = $value['class'];
                $interfaces = class_implements($class);
                // A custom named service could point to a cache implementation directly
                if ($interfaces && in_array(CacheInterface::class, $interfaces)) {
                    return true;
                }
            } else {
                $class = $key;
            }

            // Most common case: named cache services start with "Psr\SimpleCache\CacheInterface."
            return strpos($class, CacheInterface::class) === 0;
        }, ARRAY_FILTER_USE_BOTH);

        foreach ($cacheServices as $cacheService => $spec) {
            if (!Injector::inst()->has($cacheService)) {
                continue;
            }

            $cacheInstance = Injector::inst()->get($cacheService);
            if ($cacheInstance instanceof PruneableInterface) {
                $cacheInstance->prune();
            }
        }
    }
}

It’s pretty naïve, but it only really needs to cover core code (which it does). We can’t cater for every possible way someone could configure/instantiate a cache, so as long as we document it then anyone not using the expected Injector notation should be able to figure it out and write their own task.

chillu added a commit to open-sausages/silverstripe-framework that referenced this issue Nov 25, 2020
@chillu
Copy link
Member Author

chillu commented Nov 25, 2020

Yeah, I think the reason this isn't more on our radar is that most of our own hosting treats servers like cattle, they tend to be short lived (days or weeks). It's still a problem worth fixing, just not high on our priority list right now. I've at least made an effort to better document this gotcha: #9782

./ThemeResourceLoader - I think this is a separate “bug”: entering random URLs will add cache entries, something to do with controller checking if a template exists for myrandomurlslug. Probably shouldn’t ever expire

I can't reproduce this on 4.x-dev - can through a couple of URL variations, and checkedfind . -type f <TEMP_FOLDER>/ThemeResourceLoader | wc -l in between, no change in counts.

The challenge I encountered is how to fetch a list of cache instances to prune. We rely heavily on Injector for configuring things like namespaces, so this is the best I could come up with:

Yeah we should make caches more discoverable...

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

No branches or pull requests

4 participants