Added single Redis instance Redlock support. #32
Conversation
The definition of the Redlock (template) class is available in src/sw/redis++/redlock.* The tests are implemented in test/src/sw/redis++/redlock_test.h* The implementation is meant for a single Redis instance or for a single clustered Redis instance. The locking of multiple instances is not supported, but could be added.
Thanks to sewenew for pointing this out. Another test case was added to cover this situation.
The problem is that Redis is not really spot on with removing timed out keys. This means that a key might still be around a few milliseconds after it should have expired. The test TTL is now 200ms.
Thanks to sewenew for giving me this idea.
We now allow 2 ms for latency: - 1 ms for extend_lock() - 1 ms for Redis expire latency.
We now just call 'EVALSHA' and if it fails in cluster mode, we assume that the key moved to a node that has not got the script loaded yet. In this case we simply load the script and call 'EVALSHA' again.
|
|
||
| #include "redis++.h" | ||
| #include <unistd.h> | ||
| #include <openssl/rc4.h> |
There was a problem hiding this comment.
Since you don't use openssl stuff for the implementation of Redlock, you should not include this header. Otherwise, if the file is missing, i.e. openssl is not installed, your code won't compile.
There was a problem hiding this comment.
Thanks. This was a silly mistake from my side. The openssl.h include is now only in the test cases. See PR #33.
|
|
||
| namespace redis { | ||
|
|
||
| class RandomBufferInterface |
There was a problem hiding this comment.
You can use @antirez's method to generate random strings automatically.
There was a problem hiding this comment.
I think that it should be up to the user to decide if they want more speed or more randomness. The RandomBufferInterface's implementation is now moved to the test cases. See PR #33.
| Redlock(RedisInstance& instance, RandomBufferInterface& randomBuffer) : | ||
| _randomBuffer(randomBuffer), | ||
| _instance(instance), | ||
| _unlockScript("\ |
There was a problem hiding this comment.
You can try a new C++ feature: raw string literal
There was a problem hiding this comment.
Do you mean _unlockScript(R"...")?
If so, I could not get this right (lack of C++ skills), so could you please show me how to do it in RedLock::RedLock() in redlock.cpp of PR #33?
There was a problem hiding this comment.
You can try this: _unlockScript(R"(raw_characters )")
| const std::string _extendLockScript; | ||
| const std::string _unlockSHA1; | ||
| const std::string _extendLockSHA1; | ||
| std::map<std::string, std::string> _randomNumberMap; |
There was a problem hiding this comment.
Since you don't need the keys to be ordered, unordered_map might be better.
There was a problem hiding this comment.
Thanks, you're right. I however stopped tracking the locks in PR #33.
| virtual std::string getUpdatedString() = 0; | ||
| }; | ||
|
|
||
| template <typename RedisInstance> |
There was a problem hiding this comment.
It doesn't need to be a template. In fact, the original RedLock algorithm only try to solve the distributed lock problem with a single Redis instance or multiple independent Redis instances. If you do need to lock with Redis master. You can ask the user to get the Redis object with RedisCluster::redis(hash_tag) method, and pass it to Redlock.
There was a problem hiding this comment.
Thanks, you're right. I now have "RedLock" as a standard class in PR #33.
The definition of the Redlock (template) class is available in
src/sw/redis++/redlock.*
The tests are implemented in test/src/sw/redis++/redlock_test.h*
The implementation is meant for a single Redis instance or for a
single clustered Redis instance. The locking of multiple instances is
not supported, but could be added.