Skip to content

[Cache] Allow purge of expired caches (avoid filesystem overflow) #21764

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

Closed
chillu opened this issue Feb 26, 2017 · 14 comments
Closed

[Cache] Allow purge of expired caches (avoid filesystem overflow) #21764

chillu opened this issue Feb 26, 2017 · 14 comments
Assignees

Comments

@chillu
Copy link

chillu commented Feb 26, 2017

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no
Symfony version 3.3.x

The FilesystemAdapter implementation for Symfony/Cache doesn't seem to have any mechanisms to constrain the used cache size, auto-purging cache entries by least-recently-used, or their expiry date. Most other adapters (e.g. PHP's opcache, memcache) handle this internally. The filesystem adapter clears out expired caches on access only. While you can technically avoid causing the webserver filesystem to overflow by designating a limited filesystem partition for your temp folder, that's not a very common practice - and hence will leave users of this library quite exposed to server capacity exhaustion.

The StashPHP cache library has implemented a purge() action for this purpose (implementation details):

The Purge function allows drivers to perform basic maintenance tasks, such as removing stale or expired items from storage. Not all drivers need this, as many interact with systems that handle that automatically. It's important that this function is not called from inside a normal request, as the maintenance tasks this allows can occasionally take some time.

Zend_Cache used to deal with this by randomly calling a purge action every 100 cache accesses through the library - which makes app performance less predictable, but means you don't need to worry about cron jobs etc.

In general, there's not enough metadata to create a least-recently-used purge. The filesystem adapter uses touch() to set expiry metadata, but doesn't deal with last access.

If you consider this to be out of scope for the library itself, would you be open to a pull request which documents this fairly significant gotcha for the filesystem driver?

@nicolas-grekas
Copy link
Member

I agree, it could be useful to have a "gc" method, that erases expired items.
LRU is something different so I wouldn't go into this direction for FilesystemAdapter.

@chillu
Copy link
Author

chillu commented Mar 5, 2017

Thanks for your response! How do you want to handle this? I'd suggest a Purgeable interface which adapters can choose to implement, with a purge() implementation similar to the one in Stash (just with a different way to retrieve the expiry date).

Alternatively, we could add purge() to the AdapterInterface, and just return true for adapters which handle this internally (which is what Stash does, e.g. for it's memcache implementation). The "downside" here is that we couldn't just fix the issue for the FileSystem adapter, but would have to evaluate case-by-case (e.g. implement it for Redis, which should have the same issues as FileSystem).

Either way, I'd then suggest adding some indications in the docs which adapters require manual purging, and make people aware of this issue.

Aside: As a PHP project (SilverStripe CMS), we ideally want cache to be plug-and-play, hence default to FileSystem (if APC and opcache aren't available). At the moment, no other system aspect requires SilverStripe devs to set up cron jobs for a base installation. So we'll likely implement randomised auto-purging as part of HTTP request execution. rather than complicating a baseline installation by having users figure out cron jobs on their hosting.

@sminnee
Copy link

sminnee commented Mar 5, 2017

LRU is something different so I wouldn't go into this direction for FilesystemAdapter.

Would you suggest that if someone wanted this feature that they create an LRUFilesystemAdapter?

LRU (or some other way of deleting a portion of "old" items to fit within a cache-size quota) becomes important when you have a cache-invalidation approach based on changing the lookup key when a cache is invalidated (e.g. putting a known last-modified-timestamp into the cache key). Did you have an alternative solution to this or is supporting this caching approach not a primary design goal of symfony/cache?

Would you see an LRU-based filesystem cache as being appropriate to including the symfony/cache package, or just make it a separate composer package?

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Mar 6, 2017

About vocabularies, I have a personal preference towards "gc" or "prune" for this. I'm not sure "purge" convey enough semantic difference from "delete" or "clear" (which just empty the pool). But I'm not a native English speaker so I might be wrong.

Adding a new let's say PruneableInterface looks like a good idea yes (we can't change AdapterInterface per our BC policy.)

About LRU, we might not need a new adapter, but maybe just a new FilesystemAdapter::trackAccessTime() method, that'd enable additional metadata writes on each read.
Note that I wouldn't recommend this since the filesystem is one of the slowest part of a computer. I'd better take an adapter that is more suited for the job. Maybe even the database as Drupal does? Anyway, if you'd like to go this way, PR(s) welcomed.

@sminnee
Copy link

sminnee commented Mar 7, 2017

I can't imagine that a relational database would be faster than a filesystem for this kind of thing (I haven't done benchmarks but I've been badly burned moving sessions from files to database in the past), but good point about recommending a faster (e.g. memory-based) system by default.

@robfrawley
Copy link
Contributor

robfrawley commented Mar 7, 2017

@sminnee Assuming you are using InnoDB with an adequate buffer pool size, your database lookups will remain in memory, which will absolutely be faster than the filesystem, specifically on high volume websites, even with network/socket negotiation taken into account. The filesystem-backed adapter is going to start trailing heavily once a high enough volume of concurrent users starts hitting the site. In fact, the InnoDB buffer pool uses an LRU algorithm to determine what remains in memory and what is garbage collected (and must be re-read from disk on request). Furthermore, in the case of a cache (the intended use of these adapters), you could always use the memory engine for MySQL, which will ensure the disk is never touched (at the cost of purging the entire table contents on a MySQL server restart). Of course, you do need to be dedicating a large chunk of your RAM to MySQL for any useful performance under load, regardless of the engine.

Anyway, if you want to control the cache size or implement LRU, the filesystem isn't going to be the fastest option for these features, unless you're mounting a tmpfs temporary filesystem at the cache root directory to create a volatile, in-memory filesystem.

@sminnee
Copy link

sminnee commented Mar 7, 2017

Thanks for the info, @robfrawley, good insight!

@nicolas-grekas nicolas-grekas self-assigned this Apr 17, 2017
@nicolas-grekas nicolas-grekas removed their assignment Jul 6, 2017
@nicolas-grekas
Copy link
Member

Would anyone want to give a try to a "gc" or "prune" method, that would remove all expired entries, with a console command to call it?
(LRU can come later in another PR, if )

@robfrawley
Copy link
Contributor

robfrawley commented Jul 7, 2017

@nicolas-grekas I'll take a stab, but to confirm, are you talking about just the FileSystem adapter or all adapters? The latter I don't have time for, but the former I'd be happy to try implementing. Also, such a PR should be against 3.4 branch, correct?

@nicolas-grekas
Copy link
Member

Only FS, the other ones already manage purging expired items.

@RichardBradley
Copy link
Contributor

RichardBradley commented Jul 11, 2017 via email

@RichardBradley
Copy link
Contributor

Here's my app-layer implementation of the above.
This has a) a Command to GC expired cache items and b) a configurable limit to prevent the cache storing items when the filesystem is approaching full.

To make this patch into a proper PR, you'll need to: 1) move the code into the framework 2) add tests etc. 3) add documentation.

Ideally, I think there would be a note in the main caching documentation suggesting that users set up a cron job to run the GC command nightly (or as needed), perhaps even with example code for how to do that.

commit cac10c5271153ae00901990e50ff8526dbd88071
Author: Richard Bradley <Richard.Bradley@softwire.com>
Date:   Wed Apr 5 10:16:47 2017 +0100

    Remove expired filesystem cache entries, disable cache writes when disk is full

diff --git a/app/config/services.yml b/app/config/services.yml
index afb92fb5..ac604ee2 100644
--- a/app/config/services.yml
+++ b/app/config/services.yml
@@ -160,7 +160,7 @@ services:
         class: "(some app service)"
         arguments:
             - "@http_client"
-            - "@doctrine_cache.providers.file_system_configuration_cache"
+            - "@file_system_cache"
             - []
             - "@logger"
             - "@debug.stopwatch"
@@ -347,6 +347,14 @@ services:
             - "@featured_collection_from_collection_pid_factory"
             - "@featured_collection_from_video_promo_factory"
 
+    file_system_cache:
+        class: AppBundle\Cache\FilesystemCacheWithSizeLimiting
+        arguments:
+            - "%kernel.cache_dir%/doctrine/cache/file_system"
+            - ""
+            - 0002
+            - "@logger"
+
     get_set_method_normalizer:
         class: Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer
         tags:
diff --git a/bake-scripts/90-cron-filesystem-cache-gc.sh b/bake-scripts/90-cron-filesystem-cache-gc.sh
new file mode 100755
index 00000000..35d498e6
--- /dev/null
+++ b/bake-scripts/90-cron-filesystem-cache-gc.sh
@@ -0,0 +1,11 @@
+#! /bin/bash
+
+FILE=/etc/cron.hourly/myapp-filesystem-cache-gc.sh
+
+cat > $FILE <<'END'
+# Remove any expired cache entries.
+cd /var/www/my-app
+php app/console cache:gc
+END
+
+chmod +x $FILE
diff --git a/src/AppBundle/Cache/FilesystemCacheWithSizeLimiting.php b/src/AppBundle/Cache/FilesystemCacheWithSizeLimiting.php
new file mode 100644
index 00000000..81c983c4
--- /dev/null
+++ b/src/AppBundle/Cache/FilesystemCacheWithSizeLimiting.php
@@ -0,0 +1,136 @@
+<?php
+
+
+namespace AppBundle\Cache;
+
+use Doctrine\Common\Cache\FilesystemCache;
+use Monolog\Logger;
+
+/**
+ * Adds GC and size limiting (i.e. stop caching when the disk
+ * is nearly full) to the Doctrine FilesystemCache.
+ *
+ * See https://github.com/symfony/symfony/issues/21764
+ */
+class FilesystemCacheWithSizeLimiting extends FilesystemCache {
+
+    /**
+     * If the server has less than this amount of free disk space
+     * remaining, then new cache entries will not be written
+     * (to prevent a server crash).
+     *
+     * Update the alarm in service_monitoring.py if you change this.
+     */
+    const MIN_FREE_DISK_SPACE_BYTES = 500 * 1024 * 1024;
+    static $diskFreeSpace;
+    /** @var Logger */
+    private $logger;
+
+    /**
+     * The length, in bytes of a unix timestamp.
+     * This will be true until at least 2038.
+     */
+    const TIMESTAMP_LENGTH_BYTES = 10;
+
+    public function __construct($directory, $extension, $umask, $logger)
+    {
+        parent::__construct($directory, $extension, $umask);
+        $this->logger = $logger;
+    }
+
+    /**
+     * Scans all cache files and deletes any which have expired.
+     *
+     * This may be an expensive operation.
+     *
+     * @return string a human-readable summary of the cache state
+     */
+    public function gc() {
+        // See doFlush()
+
+        if ('' !== $this->getExtension()) {
+            // (So we don't have to copy 'isFilenameEndingWithExtension')
+            throw new \Exception("This class does not support the 'extension' mode");
+        }
+
+        $time = time();
+        $freeSpaceBefore = disk_free_space($this->directory);
+        $entryCount = 0;
+        $expiredRemovedCount = 0;
+        $neverExpireCount = 0;
+
+        foreach ($this->getIterator() as $name => $file) {
+            if ($file->isFile()) {
+                $entryCount++;
+
+                // See doFetch()
+                $resource = fopen($name, "r");
+
+                if (false !== ($line = fgets($resource, self::TIMESTAMP_LENGTH_BYTES + 1))) {
+                    $lifetime = (int) $line;
+                } else {
+                    $lifetime = PHP_INT_MAX;
+                }
+
+                fclose($resource);
+
+                if ($lifetime == 0) {
+                    $neverExpireCount++;
+                } else if ($lifetime < $time) {
+                    // There is a race condition here, in that a separate request
+                    // may have written a newer cache entry to this $name in the
+                    // time since we read the expiry time above.
+                    // However, nothing too bad will happen in that case; we will
+                    // just delete a valid cache entry, which is not fatal.
+                    @unlink($name);
+
+                    $expiredRemovedCount++;
+                }
+            }
+        }
+
+        $freeSpaceAfter = disk_free_space($this->directory);
+
+        return "Filesystem cache GC OK.
+Elapsed time: " . (time() - $time) . "s
+Examined $entryCount file(s)
+Removed $expiredRemovedCount expired file(s)
+There are $neverExpireCount cache item(s) set to never expire
+The filesystem had free space $freeSpaceBefore before and $freeSpaceAfter after";
+    }
+
+    /**
+     * This is copied from FileCache, where it has private access.
+     *
+     * @return \Iterator
+     */
+    private function getIterator()
+    {
+        return new \RecursiveIteratorIterator(
+            new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS),
+            \RecursiveIteratorIterator::CHILD_FIRST
+        );
+    }
+
+    protected function doSave($id, $data, $lifeTime = 0) {
+        $freeSpace = $this->estimateFreeSpaceRemaining();
+        if ($freeSpace < self::MIN_FREE_DISK_SPACE_BYTES) {
+            $this->logger->warn("Free disk space of $freeSpace is less than " .
+                "the min threshold of " . self::MIN_FREE_DISK_SPACE_BYTES . ". " .
+                "No new filesystem cache entries will be written.");
+            return false;
+        } else {
+            return parent::doSave($id, $data, $lifeTime);
+        }
+    }
+
+    private function estimateFreeSpaceRemaining() {
+        // See doGetStats()
+        // We cache this in a static var to avoid calling this function
+        // more than once per request.
+        if (!isset(self::$diskFreeSpace)) {
+            self::$diskFreeSpace = disk_free_space($this->directory);
+        }
+        return self::$diskFreeSpace;
+    }
+}
diff --git a/src/AppBundle/Command/FilesystemCacheGcCommand.php b/src/AppBundle/Command/FilesystemCacheGcCommand.php
new file mode 100644
index 00000000..6f8ffd9b
--- /dev/null
+++ b/src/AppBundle/Command/FilesystemCacheGcCommand.php
@@ -0,0 +1,56 @@
+<?php
+
+namespace AppBundle\Command;
+
+use AppBundle\Cache\FilesystemCacheWithSizeLimiting;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\DependencyInjection\ContainerAwareInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+ * Invokes the GC op on the filesystem cache
+ */
+class FilesystemCacheGcCommand extends Command  implements ContainerAwareInterface {
+
+    /**
+     * @var ContainerInterface
+     */
+    private $container;
+
+    protected function configure()
+    {
+        $this
+            ->setName('cache:gc')
+            ->setDescription('Deletes expired filesystem cache entries.')
+            ->setHelp('Deletes expired filesystem cache entries.');
+    }
+
+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
+        $container = $this->getContainer();
+        /** @var FilesystemCacheWithSizeLimiting $cacheProvider */
+        $cacheProvider = $container->get("filesystem_cache");
+
+        $summary = $cacheProvider->gc();
+
+        $output->writeln($summary);
+    }
+
+    /**
+     * @return \Symfony\Component\DependencyInjection\ContainerInterface
+     */
+    protected function getContainer()
+    {
+        return $this->container;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function setContainer(ContainerInterface $container = null)
+    {
+        $this->container = $container;
+    }
+}

Hope this helps (sorry I don't have time to turn this into a proper PR for Symfony myself),

Rich

nicolas-grekas added a commit that referenced this issue Jul 22, 2017
…e) prune method and prune command (robfrawley)

This PR was merged into the 3.4 branch.

Discussion
----------

[Cache] Add (filesystem|phpfiles) cache (adapter|simple) prune method and prune command

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21764, #21764 (comment)
| License       | MIT
| Doc PR        | symfony/symfony-docs#8209

As requested in #21764 (comment), this PR adds a `prune()` method to [`FilesystemTrait`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Traits/FilesystemTrait.php). This placement seems reasonable as it exposes the method in [`FilesystemAdapter`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Adapter/FilesystemAdapter.php) and [`FilesystemCache`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Simple/FilesystemCache.php).

The return value is a `bool` representing either a partial or complete failure (when `false`) *or* complete success (when `true`).

Once the API for the `prune` method is confirmed, I'll introduce a documentation PR, as well.

---

*Stale-detection implementation:* The file modification time is used to determine if a cache item should be pruned. This seems reasonable, given the use of [`touch` in the common trait](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Traits/FilesystemCommonTrait.php#L90). Interestingly, though, the [`doFetch` method](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Traits/FilesystemTrait.php#L38) uses the timestamp saved at the top of the file itself to determine the stale state. Should this latter implementation be used for `prune` as well (or is the current one ok), for example:

```php
foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS), \RecursiveIteratorIterator::LEAVES_ONLY, \RecursiveIteratorIterator::CATCH_GET_CHILD) as $file) {
    if ($h = @fopen($file, 'rb')) {
        if ($time >= (int) $expiresAt = fgets($h)) {
            fclose($h);
            if (isset($expiresAt[0])) {
                $okay = (@Unlink($file) && !file_exists($file)) && $okay;
            }
        }
    }
}
```

Commits
-------

f0d0c5f add (filesystem|phpfiles) cache (adapter|simple) prune method and prune command
@nicolas-grekas nicolas-grekas self-assigned this Jul 28, 2017
@nicolas-grekas
Copy link
Member

Note: the recently merged pruning logic prunes only namespaces that are know. But when the namespace seed changes, all namespaces change, and aren't referenced anymore, so that they can't be pruned (nor cleared really) anymore.

@nicolas-grekas
Copy link
Member

Considering the previous comment, and since namespace seeds don't change randomly, I'm closing this one as fixed by the new 3.4 pruning logic (and cache:pool:prune command).

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

7 participants