Skip to content

[Cache] Redis clear cursor error with Valkey #58660

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.

Sign up for GitHub

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

Jump to bottom
Closed
thomas-hiron opened this issue Oct 25, 2024 · 9 comments
Closed

[Cache] Redis clear cursor error with Valkey #58660

thomas-hiron opened this issue Oct 25, 2024 · 9 comments
Labels
Bug Cache Status: Needs Review

Comments

@thomas-hiron
Copy link
Contributor

Symfony version(s) affected

7.1.2

Description

I'm trying to flush doctrine metadata using Redis adapter with Valkey 8.0.
The RedisTrait sends the following query to Valkey: SCAN NULL on the first loop iteration, which leads to error ERR invalid cursor.

Shouldn't the cursor variable initialized to value 0?
I've patched the file with $cursor = 0; instead, and bin/console doctrine:cache:clear-metadata works as expected.

How to reproduce

I'm using the following configuration:

doctrine:
    orm:
        metadata_cache_driver:
            type: pool
            pool: doctrine_redis

framework:
    cache:
        pools:
            doctrine_redis:
                adapter: cache.adapter.redis
                provider: snc_redis.doctrine_metadata_cache

snc_redis:
    clients:
        doctrine_metadata_cache:
            type: predis
            alias: doctrine_metadata_cache
            dsn: "%env(REDIS_URL)%/3"
            options:
                connection_timeout: 10
                read_write_timeout: 30

And run bin/console doctrine:cache:clear-metadata to get the error.

You can also use directly valkey-cli SCAN NULL

Possible Solution

Initialize the cursor variable with 0.

Additional Context

No response
@thomas-hiron thomas-hiron mentioned this issue Oct 25, 2024
Merged
nicolas-grekas added a commit that referenced this issue Oct 25, 2024
@nicolas-grekas
bug #58661 [Cache] Initialize RedisAdapter cursor to 0 (thomas-hiron)
61b8de4 
This PR was submitted for the 7.1 branch but it was merged into the 5.4 branch instead.

Discussion
----------

[Cache] Initialize RedisAdapter cursor to 0

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #58660
| License       | MIT

See #58660 for details and reproducer.

Commits
-------

99a7160 initialize RedisAdapter cursor to 0
@nicolas-grekas
Copy link
Member

Looks like the fix didn't work.

@acelaya acelaya mentioned this issue Nov 2, 2024
Closed
@acelaya
Copy link

acelaya commented Nov 2, 2024

Just for context, this behavior is reproducible not only with Valkey, but also with Redis 7.4.

It works with older Redis versions though.

@nicolas-grekas
Copy link
Member

This might be an issue with the extension then. Did you try using Predis instead of ext-redis to see if there's a difference?

@acelaya
Copy link

acelaya commented Nov 2, 2024

I have only tested it with Predis actually.

Using Predis, the suggested $cursor = 0 approach works.

@nicolas-grekas
Copy link
Member

Can you try with the extension then? Maybe a bug to fill for Predis?

@acelaya
Copy link

acelaya commented Nov 2, 2024

Yeah, I guess that's a possibility.

I tried $predisClient->scan(null, ...) and $predisClient->scan(0, ...), and the latter works with all redis versions, while the former fails with redis 7.4

If it's a change in redis API, I guess I would expect Predis to handle it, but I don't know what would be their opinion.

I'll open an issue there and link it here. Then let's continue from there.

@thomas-hiron did you experience this while using Predis, or the redis extension?

@thomas-hiron
Copy link
Contributor Author

Hi, I'm using Predis indeed!
I moved from Redis 7.0 to Valkey 8.0, that can explain why I didn't have the error before!

I also confirm I don't have the issue with SncBundle being configured with the PHP Redis extension.

Thanks for your investigation!
I'm using the patch setting $cursor = 0 in production for a week without any issue for now.

@acelaya acelaya mentioned this issue Nov 3, 2024
Closed
@acelaya
Copy link

acelaya commented Nov 3, 2024

I reported the issue to predis predis/predis#1488

@acelaya acelaya mentioned this issue Nov 3, 2024
Merged
@nicolas-grekas nicolas-grekas mentioned this issue Nov 4, 2024
Merged
@nicolas-grekas
Copy link
Member

Thanks.
For Symfony, let's go with #58753

nicolas-grekas added a commit that referenced this issue Nov 4, 2024
@nicolas-grekas
bug #58753 [Cache] Fix clear() when using Predis (nicolas-grekas)
5ebc4c3 
This PR was merged into the 5.4 branch.

Discussion
----------

[Cache] Fix clear() when using Predis

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #58660
| License       | MIT

Commits
-------

fc796de [Cache] Fix clear() when using Predis
@PatNowak PatNowak mentioned this issue Jan 7, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Cache Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nicolas-grekas @acelaya @thomas-hiron @carsonbot and others