redis cluster - CROSSSLOT error

[obh]

I was trying out the library against a redis cluster. With the last change, I am able to connect to the cluster but when I initiate the consumer I get the following errors - consume error: rmq.ConsumeError (8): CROSSSLOT Keys in request don't hash to the same slot. I'm guessing we are using keys which get hashed to different slots in the cluster and which is causing this issue.

I haven't looked at the code in a lot of detail, but I want to know if it worked for @si3nloong since he had raised the last merged PR.

[wellle]

It seems that redis uses characters { and } to use part of the key for hashing. See https://redis.io/topics/cluster-spec#keys-hash-tags

In rmq we currently use [ and ] for that. See file redis_keys.go. What you could try is replacing those characters with { and } and see if it works then. I think it might.

Please let us know if that works for you ✌️

[obh]

Yes, it's working. I just replaced the [] with the {}, didn't fully grasp what all the keys were used for though :). do you want me to send a PR on this? I had noticed that the older changes are also still in master, any plans to cut a new version say a 4.0.3?
Anyway pushing to master should suffice as I plan to test some more. Will report back if I see any issues.

[obh]

just to report back on this, while this change is required we also need to move away from using a redis client to using a redis clusterclient. This is because with a cluster the key could be moved to a different shard. As a result the redis client throws the error MOVED 7652 10.0.4.210:6379.
The redis cluster client is equipped to follow this and connect with the right shard. I believe the underlying implementation of rediswrapper would need to change to support this. Anyways, I am forking out the repo and will change the redis client to cluster client. Will report back if I see any other issues. If none, maybe that's a path to still use this library with a redis cluster🤞

[NikhilRyan]

Yes, it's working. I just replaced the [] with the {}, didn't fully grasp what all the keys were used for though :). do you want me to send a PR on this? I had noticed that the older changes are also still in master, any plans to cut a new version say a 4.0.3? Anyway pushing to master should suffice as I plan to test some more. Will report back if I see any issues.

Is this fix merged? I'm getting the same error.
2023/06/21 12:30:44 consume error: rmq.ConsumeError (8970): CROSSSLOT Keys in request don't hash to the same slot

[wellle]

No it hasn't been merged. See the comments in the closed PR linked above. There are still a few things which were not addressed yet.

[NikhilRyan]

Thanks for your response. Just wanted to check if there is any work around until this issue got fixed?

[wellle]

You know what, I'll try to look at the PR in the next few days and see if I can finish it. In that case, would you be up for testing it before we merge it?

[NikhilRyan]

Yes, sure. That will be great. Thanks a lot

[wellle]

Hey, quick update: I started working on this today and made good progress. I should be able to open a PR soon.

(Here's the work in progress branch: 113-redis-cluster-support)

[NikhilRyan]

Hey, quick update: I started working on this today and made good progress. I should be able to open a PR soon.

(Here's the work in progress branch: 113-redis-cluster-support)

That's great. Thanks for picking this up. Looking forward to it.

[wellle]

Here's a new PR: #148

Please give it a try and let me know how it went! 🙏

[NikhilRyan]

Here's a new PR: #148

Please give it a try and let me know how it went! 🙏

Thanks for sharing. Let me test and revert with the update.

[NikhilRyan]

Hi @wellle
Thanks for your time, I've successfully tested #148 on a Redis Cluster environment. It is working for me.
Request you to proceed with your PR.
Also, if it is possible for you to provide a timeline on the same so that I can communicate the same to my team from when they'll be able to use the Redis Cluster client.

Apart from this I have a small doubt, What will happen to the existing messages in the queue once we restart the service. I didn't see the previous messages from the queue when I restart my service.

Once again, thanks for your efforts and time.

[wellle]

1. Thank you for testing it!
2. We're still working on making the tests pass properly for Redis cluster. @vearutop is working on this. We don't have a definitive timeline for merging this PR yet.
3. No messages should get lost due to restart of services. Once a message gets acked it is removed from the system. Each message is only delivered once. If you had unacked messages when the services goes down you need to run a queue cleaner as a separate instance (exactly one queue cleaner instance per queue system) to move unacked messages back to ready as consumers die. Please refer to the Readme for that.

[NikhilRyan]

Hi @wellle, I hope you're doing well. Just wanted to check if there's any update/progress here?

[wellle]

Hey, we're still working on making the tests work with Redis cluster.

[NikhilRyan]

Hi @wellle, I hope you're doing well. Just wanted to check if there's any update/progress here?
Team has done all the dev using this library and now we don't want to switch to some other library.

[NikhilRyan]

Hi @wellle, I can see #148 has passed all the checks and @vearutop has also approved the changes. What is the next step now?

[vearutop]

Perhaps I can go ahead and merge that PR, since @wellle is out of office for a few days.

[vearutop]

@NikhilRyan do you need a new version urgently?

[NikhilRyan]

Hi @vearutop, thanks for reverting.
Yes, one of my release is pending due to this. Would be great if this got merged.

[vearutop]

@NikhilRyan please try the new v5.2.0.

[NikhilRyan]

@vearutop Thanks a lot, let me test and revert. Lots of love guys.
Thanks

[NikhilRyan]

Hi @vearutop @wellle, tested the change, working fine for me. Once again thanks a lot for your time and effort.

One quick question, is there any documentation or sample code for best practice to use multiple queues with multiple workers?

[wellle]

Thank you for your support on this! And thanks to @vearutop too for finishing and releasing this version! 🙌

The main thing to watch out for is to have exactly once queue cleaner instance per queue system (usually exactly one). You should be free to create as many consumers as you like and use as many queues as you like.

[NikhilRyan]

Hi @wellle @vearutop, I hope you guys are doing well.
I've been experiencing an issue recently. I have two workers for a queue and it seems that consumer is picking half of the values only. Any idea why this could be happening?

[wellle]

That sounds like expected behavior. The published deliveries will be distributed between the two consumers. Each delivery will be consumed by exactly one consumer. So each consumer will consume roughly half of the published deliveries.

[NikhilRyan]

I think there is a confusion on my end on how to use it properly with multiple consumers. Is there any public repo or gist where I can see the implementation apart from the one provided in this repo.