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

Prefixed Redis Connections not working as expected for scan command #24257

Closed
ragingdave opened this issue May 20, 2018 · 10 comments
Closed

Prefixed Redis Connections not working as expected for scan command #24257

ragingdave opened this issue May 20, 2018 · 10 comments
Labels

Comments

@ragingdave
Copy link
Contributor

  • Laravel Version: 5.6.22
  • PHP Version: 7.1.17
  • Database Driver & Version: 4.0.2

Description:

In both PhpRedis and Predis connections when running a scan command, the prefix doesn't appear to function as expected. In a non-prefixed predis connection, scanning with a match of test on a prefilled redis database like:

test
test1
test2

I get the expected single result of test.
When running that same test will prefixed entries in the database like:

pre:test
pre:test1
pre:test2

and the same match condition, I get no results.

I would assume the expectation would be that any command run on a prefixed connection would only operate on that prefix and that the prefix usage would basically just be invisible like:

// Both prefixed and non-prefixed connections
$redis->set('key1', 'value');
$result = $redis->scan('key*');
// $result = ['key1'];

This is even backed up by the Predis client options (https://github.com/nrk/predis/wiki/Client-Options)

Specifies a string used to automatically prefix all the keys contained in a command issued to Redis.)

Steps To Reproduce:

  1. Create 2 connections, one prefixed and one not.
  2. Before each run of the scan command, prefill the redis database like shown above, where the prefix is never manually added other than in the connection options.
  3. Note that a scan command does not return the same results and scan calls to the prefixed connection yields no results.
@ragingdave ragingdave changed the title Prefixed Redis Connections not working as expected for certain commands Prefixed Redis Connections not working as expected for scan command May 20, 2018
@driesvints
Copy link
Member

@ragingdave gonna close this due to inactivity. Feel free to reply if this is still an issue.

@matt-allan
Copy link
Contributor

I just ran into this same issue. @driesvints could you reopen this please?

Reproduction steps using php artisan tinker and the phpredis driver:

>>> RedisFacade::set('example:123', '1')
=> true
>>> RedisFacade::scan('', 'example:*')
=> []

If you open redis-cli in another terminal and run MONITOR you can see the following commands being executed:

1582048092.501213 [0 127.0.0.1:57806] "SET" "laravel_database_example:123" "1"
1582048101.392400 [0 127.0.0.1:57806] "SCAN" "0" "MATCH" "example:*"

The same issue occurs when using the Redis class directly so I think we need to implement a similar solution to the ZADD command.

@driesvints driesvints reopened this Feb 20, 2020
@driesvints
Copy link
Member

@matt-allan does anything from the attempted PR help? #24299

@driesvints driesvints added the bug label Feb 20, 2020
@matt-allan
Copy link
Contributor

@driesvints thanks for reopening this. There have been a lot of changes since #24299 in both the connection class and the tests that should make fixing this easier. I will try and put a PR together today or early next week.

@AegirLeet
Copy link
Contributor

There's also a PhpRedis issue for this at phpredis/phpredis#548. Might be better to fix it there instead of creating a workaround that breaks as soon as PhpRedis starts respecting the prefix.

@AegirLeet
Copy link
Contributor

See phpredis/phpredis#548 (comment).

@ragingdave
Copy link
Contributor Author

That comment would seem to suggest that we would then need versioned phpredis connections to account for that difference?

@AegirLeet
Copy link
Contributor

It sounds like it's going to be an optional feature you have to specifically opt-in to (for BC reasons), so we don't have to take it into account. Although it might make sense to add a version check and delegate prefix handing to PhpRedis in the future.

@driesvints
Copy link
Member

Closing this as this seems like a PhpRedis issue rather than a Laravel issue.

@AegirLeet
Copy link
Contributor

FYI PhpRedis has merged support for a new SCAN_PREFIX option which will automatically prepend the prefix to SCAN commands. Once a version with this feature is released, Laravel could add (optional) support for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants