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

Proposition: RESP integer in hash tag bypasses CRC16 #5811

Open
thavlik opened this issue Jan 27, 2019 · 3 comments
Open

Proposition: RESP integer in hash tag bypasses CRC16 #5811

thavlik opened this issue Jan 27, 2019 · 3 comments

Comments

@thavlik
Copy link

thavlik commented Jan 27, 2019

I have not found an elegant way to ensure a keys end up on specific slots that a load balancer determines is most suitable for it. Currently, I generate a random prefix until the CRC16 is within the range of slots the node is serving. As my cluster grows, this process will take longer and longer.

It'd be great if specifying a RESP integer would bypass the CRC16 portion, but keep the modulo:

< CLUSTER KEYSLOT "{:1234}"
> "1234"
< CLUSTER KEYSLOT "{1234}"
> "...behaves identically to how it does now"
< CLUSTER KEYSLOT "{:16384}"
> "0"
@AngusP
Copy link
Contributor

AngusP commented Feb 5, 2019

The new RESP3 spec does introduce the Attribute Type which is a bit like a meta field... It could potentially be useful for providing this kind of info...

However, if I'm understanding correctly, it seems to me that the issue here is that the load balancer and the redis cluster aren't in agreement as-to which slot is where; It's important that the redis cluster is able to move slots between machines, though understandably it's not really doing so with load balancing in mind.

I'd have though it may instead be easier to make the load balancer actively aware of redis' slot assignments and to maintain balanced load by moving slots between cluster nodes if needs be, rather than having in effect two different sharding algorithms on-top of each-other? Redis clients are also generally 'supposed to' be aware of the slots and to connect to the correct redis server in the cluster for a given slot, rather than be redirected the whole time

@mgravell
Copy link

mgravell commented Feb 5, 2019

there's also a significant problem here in that this is a shard-breaking change; {:1234} already has a well-defined meaning of "compute the hash using :1234 " - which this would radically change. Likewise, any other key signature already has a well-defined meaning which would change.

as a terrible idea... you could brute force ahead of time a prefix for every possible slot (16384 of them), and store them in your codebase somewhere; not fun, but... technically possible.

I'm not sure that having your load balancer trying to define slots is a great idea, but that's a different topic.

@thavlik
Copy link
Author

thavlik commented Feb 5, 2019

In the event two distinct hash tags evaluate to the same slot, there is no way to move the Lua logic concerning each of them to different threads. The solution is to assign hash tags that are allowed to change - permitting an additional degree of scaling when Redis' coarse load balancing is insufficient. Keys can then be dumped and restored with the new hash tag to relocate them on a non-random slot.

there's also a significant problem here in that this is a shard-breaking change; {:1234} already has a well-defined meaning of "compute the hash using :1234 " - which this would radically change. Likewise, any other key signature already has a well-defined meaning which would change.

That's correct. Anyone who uses valid RESP integers within the range of 0...16384 as hash tags would have to do something like prepend another colon: CLUSTER KEYSLOT "{::1234}" -> ...same behavior as now... I'm not attached to this paradigm of fixing the problem I'm facing - it's just the first thing that came to mind as intuitive.

as a terrible idea... you could brute force ahead of time a prefix for every possible slot (16384 of them), and store them in your codebase somewhere; not fun, but... technically possible.

Actually... I think this might be an elegant solution for this strange problem. I'm trying to not modify my source code so it doesn't rule out Elasticache as an option.

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

3 participants