PhpRedisConnector sends incorrect parameters to RedisCluster scan

[bentleyo]

Laravel Version

11.31.0

PHP Version

8.3.2

Database Driver & Version

Redis 7.1 (AWS OSS Redis Cache), phpredis 7.4

Description

When using redis clusters the PhpRedisConnection class fails to send the required key_or_address (node) parameter to the scan method of RedisCluster. See: https://phpredis.github.io/phpredis/RedisCluster.html#method_scan

bool|array scan(int|null $iterator, string|array $key_or_address, string|null $pattern = null, int $count = 0)

The code in PhpRedisConnection looks like this currently:

public function scan($cursor, $options = [])
{
    $result = $this->client->scan($cursor,
        $options['match'] ?? '*',
        $options['count'] ?? 10
    );

This seems to result in the scan method endlessly scanning over the redis database with the default count of 10 and the wrong value for match.

For large databases this is a very impractical count and causes performance issues.

I originally noticed this problem after seeing the cache:prune-stale-tags command consistently timeout with our production application.

This is happening as that command calls \Illuminate\Cache\RedisStore@currentTags which internally finds tags by calling scan.

When using cache with Vapor it puts redis into cluster mode, so this has been causing some headaches for us.

This might be the cause of these issues:

• cache:prune-stale-tags Not Functioning on Redis Cluster #50415
• cache:prune-stale-tags not available for Redis Cluster #48977

Steps To Reproduce

1. Setup the database config so that redis is configured with clusters (we're using AWS Redis OSS with Vapor)
2. Add more than 10 keys to the redis database
3. Make a call to scan like:

use Illuminate\Support\Facades\Redis;

$cursor = $defaultCursorValue = '0';
$scan = Redis::scan($cursor, ['match' => '*', 'count' => 100]);

dd($scan);

You will see that scan returns at most 10 keys, even if there are more.

[bentleyo]

I've been able to confirm that adding the missing parameter works by running the following snippet locally and then on Vapor via tinkerwell:

$cache = Cache::getStore();
$client = $cache->connection()->client();

$cursor = '0';

if ($client instanceof RedisCluster) {
    foreach ($client->_masters() as $master) {
        dump($client->scan($cursor, $master, '*', 100));
    }
} else if ($client instanceof Redis) {
    dump($client->scan($cursor, '*', 100));
}

Obviously it's not ideal to iterator over the _masters like this, but can confirm it's outputting 100 keys.

[crynobone]

@bentleyo Wouldn't it be possible to solve this by overriding scan() method in PhpRedisClusterConnection?

[bokanechase-digistorm]

@crynobone nice find! I wasn't aware of that class! Yep, it looks like that's the place it should be overridden. However, I'm not sure how you would approach scanning when there's multiple masters (as there would be multiple cursors etc.).

I think the solution is to only support one. Perhaps we could add a parameter for specifying the node and if it's note set default to using the first master. I have a few different ideas about how it could be solved.

I can look at putting in a PR for this.

[crynobone]

I not 100% familiar with Redis Cluster so PR are appreciated. We can get Vapor team to review it if needed.

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!