#319 Fixed Redis Cluster Crossslot Issue

[SouradipPoddar]

Description

Issue Link: #319
The issue is occurring while renaming the temporary key with actual key because the hash slot for temporary key (with _tmp suffix) and actual key (without _tmp suffix) should be same. If they are not same, redis will throw an error:

(error) CROSSSLOT Keys in request don't hash to the same slot

To solve this,we need to force the temporary key to be in the same hash slot. We can do this by adding hashtag to the actual part of the temporary key. When the key contains a "{...}" pattern, only the substring between the braces, "{" and "}," is hashed to obtain the hash slot.

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

Checklist

• Linter passes correctly
• Add tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary

Does this introduce a breaking change?

• Yes
• No

Further comments

This change I have tested in my environment with 3 master node - 3 replica node redis cluster. It is working fine.

[mga-chka]

I'm surprised to see the failure, a better fix would be to see which operation generates the issue and see if we can avoid it because conceptually chproxy doesn't rely on bulk operations and each operation should deal with only one key.

If there is no solution then we could use the one you provide but you should add comments in the code to explain the meaning of the "{}" with a link the redis doc. But I'm afraid your fix wouldn't fix all situation because (again) conceptually we're not doing bulk operations in chproxy so I don't see why you couldn't have a bulk operation from multiple queries happening at the same time.

[edit] looking more at your description, you already said the issue comes during renaming (I'm guessing this one

chproxy/cache/redis_cache.go

Line 302 in 95a6a6d

r.client.Rename(ctxRename, stringKeyTmp, stringKey)

) which is seen as a bulk multikey operation by redis. I don't see how we can avoid this rename so your fix seems the good one.
Just add the comment I asked and we should be able to merge the PR.

[SouradipPoddar]

Thank you for the response. I have added the comment as per your suggestion. Please take a look.

[mga-chka]

FYI your fix will be in the next release that should be available before the end of the month