[10.x] Use Lua script in Redis to flush cache tags atomically

[zengbo]

Issue: #47903. Reproduce as follows:

1. Create a Laravel project: composer create-project laravel/laravel example-app
2. Add a controller: php artisan make:controller CacheController
3. Insert routes in routes/api.php
4. Set up Redis connection
5. Use attached program to replicate issue

The App\Http\Controllers\CacheController:

<?php

namespace App\Http\Controllers;

use Cache;
use Illuminate\Http\Request;
use Symfony\Component\HttpFoundation\Response;

class CacheController extends Controller
{
    public function create(Request $request): void
    {
        $key = $request->get("key");
        Cache::tags(['test_tag'])->remember($key, 86400, fn (): string => $key);
    }

    public function delete(): void
    {
        Cache::tags(['test_tag'])->flush();
    }

    public function check(): Response
    {
        /** @var \Illuminate\Cache\RedisTaggedCache $redis_tagged_cache */
        $redis_tagged_cache = Cache::tags(['test_tag']);
        /** @var \Illuminate\Cache\RedisTagSet redis_tag_set */
        $redis_tag_set = $redis_tagged_cache->getTags();

        $keys_in_tags = $redis_tag_set->entries();
        $cache_tagged_item_key_prefix = sha1($redis_tag_set->getNamespace());

        $cache_tagged = self::getTaggedCacheByPrefix($cache_tagged_item_key_prefix);

        if (count($cache_tagged) != $keys_in_tags->count()) {
            return response()->json(['result' => 'inconsistent']);
        } else {
            return response()->json(['result' => 'consistent']);
        }
    }

    private static function getTaggedCacheByPrefix(string $prefix): array
    {
        $redis_store = Cache::store('redis');
        $cursor = $defaultCursorValue = '0';
        $tagged_caches = [];
        do {
            [$cursor, $entries] = $redis_store->connection()->scan(
                $cursor,
                ['match' => '*' . $prefix . '*', 'count' => 1000]
            );

            if (! is_array($entries)) {
                break;
            }

            $entries = array_unique(array_keys($entries));

            if (count($entries) === 0) {
                continue;
            }

            foreach ($entries as $entry) {
                $tagged_caches[] = $entry;
            }
        } while (((string) $cursor) !== $defaultCursorValue);

        return $tagged_caches;
    }
}

The routes/api.php:

<?php

use Illuminate\Http\Request;
use Illuminate\Support\Facades\Route;

/*
|--------------------------------------------------------------------------
| API Routes
|--------------------------------------------------------------------------
|
| Here is where you can register API routes for your application. These
| routes are loaded by the RouteServiceProvider and all of them will
| be assigned to the "api" middleware group. Make something great!
|
*/


Route::post('/cache_tag', [\App\Http\Controllers\CacheController::class, 'create']);
Route::delete('/cache_tag', [\App\Http\Controllers\CacheController::class, 'delete']);
Route::get('/cache_tag/check', [\App\Http\Controllers\CacheController::class, 'check']);

To test on Mac iTerm:

./cache_tag_benchmark_mac_arm64 -apiPrefix=http://localhost/api -iterations=15

Output "Inconsistent result found" indicates issue reproduction.

cache_tag_benchmark.tar.gz

[taylorotwell]

Can you describe the cause of the issue and what steps you took to fix it? Please mark as ready for review when you have done so.

[zengbo]

Can you describe the cause of the issue and what steps you took to fix it? Please mark as ready for review when you have done so.

When we call Cache::tags(['test_tag'])->flush();, the process operates as follows:

1. Retrieve keys from the cache tag.
2. Delete these keys.
3. Remove the cache tag key.

Since these three operations are not atomic, a problem may arise: if new keys are added to the cache tag after step 2 but before step 3, these keys become orphaned and unmanageable by the tag key. Consequently, in the next call to flush, the new keys cannot be retrieved, and thus, cannot be deleted.

To resolve this, I modified the logic:

1. In step 3, only remove the deleted keys in the cache tag, without removing the cache tag key.
2. Use a Lua script to make steps 2 and 3 atomic.

[taylorotwell]

@zengbo can't new keys be added after you gather them but before you invoke the Lua script here?

[zengbo]

@zengbo can't new keys be added after you gather them but before you invoke the Lua script here?

@taylorotwell New keys can be added after they are gathered. The Lua script only deletes the previously gathered keys, not the tag key. Therefore, the new keys remain in the cache tag key and can be retrieved during the next flush call..

[zengbo]

@zengbo can't new keys be added after you gather them but before you invoke the Lua script here?

@taylorotwell New keys can be added after they are gathered. The Lua script only deletes the previously gathered keys, not the tag key. Therefore, the new keys remain in the cache tag key and can be retrieved during the next flush call..

This means that the deletion of the key and the removal of the key's reference in the tag key are atomic operations. However, I do not delete the tag key.

[taylorotwell]

So how does the tag key ever get deleted? Can the removal of all tag entires and the key be combined into a single Lua script?

[zengbo]

So how does the tag key ever get deleted? Can the removal of all tag entires and the key be combined into a single Lua script?

So how does the tag key **ever** get deleted?
If all the tag entries in the tag are deleted, the tag key is also deleted by Redis. In other words, deleting tag entries without concurrent requests is equivalent to deleting the tag key.

Can the removal of all tag entires and the key be combined into a single Lua script?
No.
Getting all tag entries in a single Lua script is challenging, because we use zScan to retrieve them. I normally retrieve all tag entries, and then use a Lua script to delete both the keys and the tag entries.

[TimWolla]

I believe this line violates:

Important: to ensure the correct execution of scripts, both in standalone and clustered deployments, all names of keys that a script accesses must be explicitly provided as input key arguments. The script should only access keys whose names are given as input arguments. Scripts should never access keys with programmatically-generated names or based on the contents of data structures stored in the database.

Because the key name is taken from ARGV instead of KEYS.

[zengbo]

The KEYS and ARGV are not the same.
KEYS represent cache entries within the cache tag, while ARGV stands for cache tag keys.
I utilize key for KEYS and zkey for ARGV.

[TimWolla]

The arg parameter is the key that ZREM operates on and it is taken from ARGV.

[zengbo]

@taylorotwell
I've created two repositories on GitHub:

• https://github.com/zengbo/laravel-demo
  The origin-laravel-package branch reproduces the issue, while the using-lua-script-flush-cache-tag branch uses the modified package.
• https://github.com/zengbo/concurrent-client-demo
  This is the client that reproduces the issue, written in Golang.

Please test these, thanks.

[taylorotwell]

I personally don't have the knowledge to trust merging this sadly. I would prefer to either de-document cache tags or recommend their use with Memcached only. Cache tags and Redis have been a maintenance and complexity nightmare.

[AidasK]

@taylorotwell please reconsider fixing this concurrency issue since our application suffers a lot from inconsistent cache around resources. Since tagged cache does not guarantee cache to be flushed successfully each time, we are forced to clear entire cache store. It's not an option for high load applications as it stresses out the database too much.

Opened a discussion #49159

[taylorotwell]

@AidasK are you able to test the content of this PR to see if it fixes your issue?

[AidasK]

Hi @taylorotwell, thanks for taking a look at this issue again. Personally I have not tried this solution as in production we are using 9.x laravel cache class. Our use case allows us to have some memory leaks.

You won't like it, but I suggest to use https://github.com/symfony/cache instead of developing your own solution. Under the hood they are already using lua scripts to make things work. Just look it up https://github.com/symfony/cache/blob/7.0/Adapter/RedisTagAwareAdapter.php

Since it's too hard to maintain proper cache adapters, it's better to use symfony ones which are well developed and focus on futures which truly matters. Of course it is easier said than done, but in the long term perspective it should be worth the effort. For laravel community it won't have any breaking changes besides of clearing all the database once if storage format is different, haven't checked.

[ARehmanMahi]

symfony/cache tags, recahe-locking, optimistic-recache are awesome built-in & default features.