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

RedisCluster scan_iter does not return all keys from all nodes #2003

Closed
felixwang9817 opened this issue Feb 19, 2022 · 3 comments · Fixed by #2054
Closed

RedisCluster scan_iter does not return all keys from all nodes #2003

felixwang9817 opened this issue Feb 19, 2022 · 3 comments · Fixed by #2054

Comments

@felixwang9817
Copy link

Version: What redis-py and what redis version is the issue happening on?

redis-py==4.1.2 and redis-cli 6.2.6

Platform: What platform / version? (For example Python 3.5.1 on Windows 7 / Ubuntu 15.10 / Azure)

Python 3.7.12 on MacOS 11.4

Description: I started a local Redis cluster with 6 nodes (3 master nodes with ports 6001-6003, 3 slave nodes with ports 6004-6006). I wrote 19 total items to the Redis cluster: 1 is stored by 6001, 13 by 6002, and 5 by 6003. I confirmed this by doing

$ redis-cli -c -p 6001           
127.0.0.1:6001> KEYS *
1) "\x02\x00\x00\x00\x04\x00\x00\x00\x04\x00\x00\x00\x8b\x13\x00\x00integration_test_5d7072_1"
127.0.0.1:6001>

for each of the 3 master nodes.

However, when I call client.scan_iter(b"*"), I get a list containing only a single key, which is the key from node 6001. Here my client object was constructed as follows:

startup_nodes = [{'host': '127.0.0.1', 'port': '6001'}, {'host': '127.0.0.1', 'port': '6002'}, {'host': '127.0.0.1', 'port': '6003'}]
kwargs["startup_nodes"] = [
    ClusterNode(**node) for node in startup_nodes
]
self._client = RedisCluster(**kwargs)

This looks to me like there is an issue with scan_iter on RedisClusters, where it doesn't return all keys from all nodes.

@felixwang9817
Copy link
Author

Update: I did not realize that scan_iter would not automatically hit all nodes in the cluster. I fixed this problem by passing in target_nodes=RedisCluster.ALL_NODES as a kwarg. This usage is documented here, although I do feel like the documentation could be better (it took me a while to realize my mistake).

I have since found a related issue: scan_iter is not properly iterating through all keys. For example, the 6002 node has 14 keys. I can access the 6002 node by doing node = client.get_primaries()[1]. Then I call scan_iter on it by doing client.scan(cursor="0", target_nodes=[node]), which yields a list of length 10. This is wrong, however, as in the terminal I can confirm the node has more than 10 keys:

$ redis-cli -p 6002 SCAN 0
1) "3"
2)  1) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x8b\x13\x00\x00integration_test_6011b4_1"
    2) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x8e\x13\x00\x00integration_test_6011b4_1"
    3) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x8f\x13\x00\x00integration_test_6011b4_1"
    4) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x93\x13\x00\x00integration_test_6011b4_1"
    5) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x8c\x13\x00\x00integration_test_6011b4_1"
    6) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x99\x13\x00\x00integration_test_6011b4_1"
    7) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x96\x13\x00\x00integration_test_6011b4_1"
    8) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x9b\x13\x00\x00integration_test_6011b4_1"
    9) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x9a\x13\x00\x00integration_test_6011b4_1"
   10) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x90\x13\x00\x00integration_test_6011b4_1"
$ redis-cli -p 6002 SCAN 3
1) "0"
2) 1) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x8a\x13\x00\x00integration_test_6011b4_1"
   2) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x95\x13\x00\x00integration_test_6011b4_1"
   3) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x8d\x13\x00\x00integration_test_6011b4_1"
   4) "\x02\x00\x00\x00driver_id\x04\x00\x00\x00\x04\x00\x00\x00\x89\x13\x00\x00integration_test_6011b4_1"

I believe the source of this issue is that scan_iter relies on scan, which itself is incorrect (at least when specifying target_nodes). For example, calling result = client.scan(target_nodes=[node]) yields a tuple of the form (0, list of 10 keys), which indicates that the cursor being returned is a 0, which is not at all expected.

@kevjumba
Copy link

kevjumba commented Feb 28, 2022

Just as a followup, a rollback to redis-py-cluster resolved the issues that we were facing in our integration tests which is more evidence that there may be some implementation bug in the scanning mechanism on clusters of nodes.

This is using redis 3.5.3 and redis-py-cluster 2.1.3.

@chayim
Copy link
Contributor

chayim commented Mar 8, 2022

Update: I did not realize that scan_iter would not automatically hit all nodes in the cluster. I fixed this problem by passing in target_nodes=RedisCluster.ALL_NODES as a kwarg. This usage is documented here, although I do feel like the documentation could be better (it took me a while to realize my mistake).

I think you're right, the documentation could definitely be better. Generally cluster support is just started! While we worked with @Grokzen and @barshaul on moving this in, it's definitely not done! Any chance @felixwang9817 you'd like to contribute a docs PR that you think might better help everyone out?

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

Successfully merging a pull request may close this issue.

3 participants