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

[Question] Redis - Deleting Cached Items Once Expired #142

Open
benbelmour opened this issue Jun 9, 2021 · 2 comments
Open

[Question] Redis - Deleting Cached Items Once Expired #142

benbelmour opened this issue Jun 9, 2021 · 2 comments

Comments

@benbelmour
Copy link

I have my caching set up as such which works as expected:

$stack->push(
    new CacheMiddleware(
        new PrivateCacheStrategy(
            new LaravelCacheStorage(
                \Illuminate\Support\Facades\Cache::store('redis')
            )
        )
    ),
    'cache'
);

The one issue is, the items aren't being deleted from Redis.

The cache isn't being hit and requests are being sent once the TTL has expired which is expected, but as mentioned above the items aren't actually being deleted from Redis.

How would one set the items to delete automatically?

@benbelmour benbelmour changed the title Redis Expire and Delete Items [Question] Redis - Deleting Cached Items Once Expired Jun 9, 2021
@rhysemmerson
Copy link
Contributor

I think the problem is it only uses the ttl when fetching from the cache but not when saving to the cache. This is because some Cache Control rules allow serving from cache after the max-age has passed.

There might be a way to configure a default ttl, maybe a global max ttl config would be useful.

@rhysemmerson
Copy link
Contributor

We tackled this problem this week. The issue is that if the response has a validation header (Last-Modifed, etag etc) the LaravelCacheStorage class will use forever when saving to cache. This is because the response could be revalidated after it becomes stale.

This would be fine for a large file cache but when using redis the memory limits are more restrictive and the eviction policy doesn't work well with large amounts of entries with no ttl.

We've written our own implementation which unfortunately requires reflection. I'll submit a PR at some point that allows for a nicer way of doing this.

<?php

namespace Frontend\Http\Guzzle;

use Kevinrob\GuzzleCache\CacheEntry;
use Kevinrob\GuzzleCache\Storage\LaravelCacheStorage;
use ReflectionObject;
use ReflectionProperty;

class LaravelTtlAwareCacheStorage extends LaravelCacheStorage
{
    /**
     * {@inheritdoc}
     */
    public function save($key, CacheEntry $data)
    {
        try {
            $lifeTime = $this->getLifeTime($data);
            if ($lifeTime > 0) {
                return $this->cache->add(
                    $key,
                    serialize($data),
                    $lifeTime
                );
            }
        } catch (\Throwable $ignored) {
            report($ignored);
        }

        return false;
    }

    protected function getLifeTime(CacheEntry $data)
    {
        $ttl = $data->getTTL();

        if ($ttl < 0) {
            return $ttl;
        }

        $entry = $this->makeAccessible($data);

        $stateIfError = $entry['staleIfErrorTo']
            ? now()->diffInSeconds($entry['staleIfErrorTo']) : 0;
        $staleWhileRevalidate = $entry['staleWhileRevalidateTo']
            ? now()->diffInSeconds($entry['staleWhileRevalidateTo']) : 0;

        return max(now()->diffInSeconds($entry['staleAt']), $stateIfError, $staleWhileRevalidate);
    }

    private function makeAccessible(CacheEntry $entry)
    {
        return collect((new ReflectionObject($entry))->getProperties())
            ->each(fn (ReflectionProperty $property) => $property->setAccessible(true))
            ->keyBy(fn (ReflectionProperty $property) => $property->getName())
            ->map(fn (ReflectionProperty $property) => $property->getValue($entry));
    }
}

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