[Cache] Fix Redis6Proxy #51754
[Cache] Fix Redis6Proxy #51754
Conversation
nicolas-grekas
commented
Sep 26, 2023
| Q | A |
|---|---|
| Branch? | 6.3 |
| Bug fix? | yes |
| New feature? | no |
| Deprecations? | no |
| Tickets | - |
| License | MIT |
| Doc PR | - |
a39d71c to
c789603
Compare
c789603 to
6554077
Compare
| @@ -353,6 +353,7 @@ public function get(): ?array | |||
| $this->group, | |||
| $this->consumer, | |||
| [$this->stream => $messageId], | |||
| 1, | |||
| 1 | |||
There was a problem hiding this comment.
@alexander-schranz can you please confirm that this change makes sense to you? It adds explicit BLOCK 1 to the xreadgroup command. (ext-redis 5 & 6 don't have the same default for this argument apparently, I'm trying to figure out how we should write the code to deal with both versions.)
There was a problem hiding this comment.
This looks like the new default, what was the previous one? I try to understand / find the change inside the phpredis extension, did you already find that?
There was a problem hiding this comment.
The C code looks a but blurry to me. It looks like the behavior depends on the number of arguments, not on the actual values :'(
Is there a value of BLOCK that works for the need here? eg this 1?
There was a problem hiding this comment.
In generally I don't see a problem of a Block for 1ms to wait for any messages. The PHP Extension part for me between 5.3.7 and 6.0.1 did look the same so maybe its inside Redis itself? Or was just the redis extension updated? Another possibility would be that we check for the version via phpversion('redis'); but that just returns the extension version not the server version. I try to find out when the added noblock feature as I maybe think its related to that changes 🤔 .
There was a problem hiding this comment.
Let me merge this to make tests green but please let me know if you discover 1 isn't what we need.