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

Redis discoverer implements scan incorrectly for Cluster configurations #938

Open
JordanPawlett opened this issue Jun 9, 2021 · 9 comments

Comments

@JordanPawlett
Copy link
Contributor

const stream = this.client.scanStream({

this.client.scanStream is used in multiple locations through the custom redis discoverer. scanStream does not exist on client for Redis.Cluster. I believe must iterate over client.nodes('master') to perform a scan, similar to how the Redis' cacher is written.

@icebob
Copy link
Member

icebob commented Jun 9, 2021

Thanks.

@icebob
Copy link
Member

icebob commented Sep 20, 2021

Could you create a PR?

@icebob icebob added this to the 0.14.x milestone Sep 20, 2021
@intech intech self-assigned this Sep 21, 2021
@intech
Copy link
Member

intech commented Sep 21, 2021

Now I will make and add tests

@intech
Copy link
Member

intech commented Sep 21, 2021

The preliminary result is that simply sending the request through master will not work. I'm figuring out if this problem is related to my tests or it's a bug in ioredis

@intech
Copy link
Member

intech commented Sep 23, 2021

@JordanPawlett

  1. You could clarify how exactly your cluster is assembled through sentinel or redis cluster
  2. Its configuration is how many masters and slaves per master.
  3. And is there a helm or docker-compose with sh scripts to configure it? Or maybe it's some kind of DBaaS solution?

I am testing on 2 different cluster configurations and am getting 2 completely different problems.

@JordanPawlett
Copy link
Contributor Author

I've since moved to use etcd for service-discovery.
If i remember correctly the Redis discovery worked correctly with a Sentinel setup. However, breaks for redisCluster, as scanStream does not exist on that client.

Both redis-cluster and redis-sentinel were deployed in a k8s cluster using bitnami helm charts.

What problems specifically are you encountering?

@intech
Copy link
Member

intech commented Sep 23, 2021

If i remember correctly the Redis discovery worked correctly with a Sentinel setup. However, breaks for redisCluster, as scanStream does not exist on that client.

Sentinel works differently, unlike redisCluster, a similar problem does not occur there. With redisCluster, additional commands are sent to redis-server which results in an error.

scanStream exists in ioredis and scan command is supported in redis-server with cluster. Errors occur when trying to work with data, the scan command on the server in one case CROSSSLOT error, in another cluster All keys in the pipeline should belong to the same slots allocation group when requesting for any node (master/slave).

@icebob
Copy link
Member

icebob commented Nov 17, 2021

Any news about it?

@intech
Copy link
Member

intech commented Nov 17, 2021

Any news about it?

Alas, I don't have a solution yet. In testing, I didn't a solution with client.nodes ('master').
I will run tests again with the current versions of redis and ioredis to make a final conclusion where the problem is.

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

No branches or pull requests

3 participants