Set cluster slot for scan commands, rather than random
#2623
Conversation
pete-woods
force-pushed
the
use-hash-tag-in-scan-commands
branch
7 times, most recently
from
36a52f2
to
f39ee1e
Compare
pete-woods
changed the title
pete-woods
changed the title
scan commands, rather than random
pete-woods
force-pushed
the
use-hash-tag-in-scan-commands
branch
2 times, most recently
from
c2c460f
to
3898d71
Compare
|
Hi @vmihailenco. Is there any chance you could have a look over this, and let me know if this is a change you'd be interested in? Thanks! |
… client - At present, the `scan` command is dispatched to a random slot. - As far as I can tell, the scanX family of commands are not cluster aware (e.g. don't redirect the client to the correct slot). - You can see [here](https://github.com/redis/jedis/blob/869dc0bb6625b85c8bf15bf1361bde485a304338/src/main/java/redis/clients/jedis/ShardedCommandObjects.java#L101), the Jedis client calling `processKey` on the match argument, and this is what this PR also does. We've had this patch running in production, and it seems to work well for us. For further thought: - Continuing looking at other Redis clients (e.g. Jedis), they outright [reject as invalid](https://github.com/redis/jedis/blob/869dc0bb6625b85c8bf15bf1361bde485a304338/src/main/java/redis/clients/jedis/ShardedCommandObjects.java#L98) any scan command that does not include a hash-tag. Presumably this has the advantage of users not being surprised when their scan produces no results when a random server is picked. - Perhaps it would be sensible for go-redis to do the same also?
pete-woods
force-pushed
the
use-hash-tag-in-scan-commands
branch
from
3898d71
to
e0fbcea
Compare
|
Hi @vmihailenco. Just following up on this pr. Is it a change that you would consider? |
|
@chayim Hi! This is a change we've been carrying in a fork for about 6 months now. Do you have any time to have a look over and see if this is something this project would be willing to accept? The idea is basically lifted from the Redis clients for other languages. |
|
Would it be an option for scan to implicitly run over all shards if no hash-tag is included? |
|
The Java clients actually error if you don't provide a hash tag. I kinda think that behaviour is better, as it avoids the surprising result of your scan not really working at all, but it would be a breaking change for this library. |
scancommand is dispatched to a random slot.processKeyon the match argument, and this is what this PR also does.We've had this patch running in production, and it seems to work well for us.
For further thought: