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 Cluster Support? #59

Open
BigBorg opened this issue Nov 24, 2017 · 6 comments
Open

Redis Cluster Support? #59

BigBorg opened this issue Nov 24, 2017 · 6 comments

Comments

@BigBorg
Copy link

BigBorg commented Nov 24, 2017

Does this package support Redis Cluster? I find that to make atomic operation work with multiple keys, {} brackets would be needed, which makes "lock:resource" as "lock:{resource}" and "lock-signal:resource" as "lock-signal:{resource}".

The Redis Cluster document is as follows:

Redis Cluster supports multiple key operations as long as all the keys involved into a single command execution (or whole transaction, or Lua script execution) all belong to the same hash slot. The user can force multiple keys to be part of the same hash slot by using a concept called hash tags.

Hash tags are documented in the Redis Cluster specification, but the gist is that if there is a substring between {} brackets in a key, only what is inside the string is hashed, so for example this{foo}key and another{foo}key are guaranteed to be in the same hash slot, and can be used together in a command with multiple keys as arguments.

@ionelmc
Copy link
Owner

ionelmc commented Nov 24, 2017

Perhaps you meant {lock}resource and {lock-signal}resource?

@BigBorg
Copy link
Author

BigBorg commented Nov 24, 2017

No, only content inside {} bracket is used to generate hash slot. If you use {lock}resource and {lock-signal}resource, they might end up in different slot since hash result of "lock" and "lock-signal" is different. Meanwhile all the lock key for different resources end up in the same slot which does not meet the purpose of Redis Cluster (to shard keys).

But if you use lock:{resource} and lock-signal:{resource}, the two keys for the same resource will end up in the same slot making atomic operation possible with Redis Cluster. Also, since resource differs, this will spread out lock and lock-signal keys for different resources to different slots.

@ionelmc
Copy link
Owner

ionelmc commented Nov 24, 2017

Well I guess we could change it but then a new release would break everything for most people. A better way would be a way to allow users to override the formatting (so you could do your RC stuff in your lock subclass).

@ionelmc
Copy link
Owner

ionelmc commented Nov 24, 2017

You can already do it right now, albeit a bit ugly:

class RedisClusterLock(Lock):
  def __init__(self, *args, **kwargs):
    super(RedisClusterLock, self).__init__(*args, **kwargs)
    self._name = 'lock:{%s}' % name
    self._signal = 'lock-signal:{%s}' % name

@amielboim
Copy link

@ionelmc
Any news about this issue?
Maybe need updates to the above solution for the 3.7.0 version (it's not working for us)?
We have troubles with same issue in the production environment...
Thanks 🙏

@ionelmc
Copy link
Owner

ionelmc commented Oct 18, 2022

@amielboim isn't that subclass working for you? I mean I could add a "cluster=False" option so ppl can opt-in to the new key names if you want something simpler. I can't simply change the lock names by default since it would make upgrading the library troublesome.

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