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

Cluster mode KEYS only searches one node by default #2560

Open
Kyle-sandeman-mrdfood opened this issue Jan 23, 2023 · 2 comments
Open

Cluster mode KEYS only searches one node by default #2560

Kyle-sandeman-mrdfood opened this issue Jan 23, 2023 · 2 comments

Comments

@Kyle-sandeman-mrdfood
Copy link

"KEYS",

The documentation provides an example: rc.keys(target_nodes=Redis.ALL_NODES). This looks acceptable as a default to me.
The KEYS command does not make sense in a cluster environment without checking at least the Redis.PRIMARIES.

Could KEYS either be moved to PRIMARIES/ALL_NODES or a note be added to the Clustering section of the documentation which explains this behaviour?

In redis-py-cluster, this command was given the flag all-nodes: https://github.com/Grokzen/redis-py-cluster/blob/8a8102a9d758d61a7ec1e2ac9050fcd34029ff3f/rediscluster/client.py#L183

As such rc.keys("...") breaks after switching to redis-py

@jonprindiville
Copy link

jonprindiville commented Jan 16, 2024

Oh, I'm not sure about the documentation example.

I can say, though, that generally KEYS is frowned upon in production and SCAN is often the suggested replacement.

That doesn't actually go towards your issue 🙃 but if you can be convinced to use SCAN, then in Grokzen/redis-py-cluster you can use scan_iter to transparently iterate the nodes and also the paged results of the SCAN. Seems like, yeah, it grabs all the primary nodes for that automatically.

In redis-py there's also a scan_iter... I just haven't used it first-hand to tell you that it behaves the same or not. You might have to bring your own list of target nodes.

@Kyle-sandeman-mrdfood
Copy link
Author

Hello, a year later!
I believe we've removed usages of KEYS from our system and migrated from redis-py-cluster to redis~=4.4.0.

In any case, the current code in master still exhibits this issue. KEYS should be configured to run against PRIMARIES rather than DEFAULT_NODE (I am not sure whether this requires any other code to change)

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

No branches or pull requests

2 participants