[REDIS] Use SCAN instead of KEYS to find all matching keys#13405
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13405 +/- ##
============================================
+ Coverage 90.94% 90.94% +<.01%
- Complexity 13997 14000 +3
============================================
Files 523 523
Lines 36318 36323 +5
============================================
+ Hits 33029 33034 +5
Misses 3289 3289
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #13405 +/- ##
============================================
+ Coverage 90.94% 90.94% +<.01%
- Complexity 13997 14000 +3
============================================
Files 523 523
Lines 36318 36325 +7
============================================
+ Hits 33029 33036 +7
Misses 3289 3289
Continue to review full report at Codecov.
|
|
I needed to remove the |
| $result = []; | ||
| foreach ($keys as $key) { | ||
| $result[] = $this->_Redis->del($key) > 0; | ||
| $this->_Redis->setOption(Redis::OPT_SCAN, Redis::SCAN_RETRY); |
There was a problem hiding this comment.
This is causing type errors in 4.x https://travis-ci.org/cakephp/cakephp/jobs/567855601#L822.
I am not sure what version of redis/predis travis is using. But even if it only occurs for older versions it could be an issue for our users.
There was a problem hiding this comment.
So the issue has been fixed phpredis/phpredis#1540. Not sure in which version.
There was a problem hiding this comment.
Fix is available in 5.0.0. So essentially one won't be able to use Redis 4.x with Cake 4.0.
There was a problem hiding this comment.
Do (string)Redis::SCAN_RETRY typecast would help?
There was a problem hiding this comment.
So essentially one won't be able to use Redis 4.x with Cake 4.0.
You mean the client library cannot be 4.x, not that you cannot use Redis 4.x.
There was a problem hiding this comment.
@garas Maybe, I haven't checked.
@josegonzalez Yes, I should have worded it better.
There was a problem hiding this comment.
Typecasting to string worked https://github.com/cakephp/cakephp/pull/13469/files#diff-61e014e894b3dcbac9bd2425ec47490bR237
This PR changes the way how cache is cleared. First the inefficient KEYS command was used to find all matching keys in Redis.
According to the Redis docs, this doesn't perform well and they advice to use the
SCANcommand.Fixes: #13404
Used sample code from PhpRedis https://github.com/phpredis/phpredis#scan