From 10ca2b2b7faeb3a1e0f9df35904f7368243a9709 Mon Sep 17 00:00:00 2001 From: sewenew Date: Mon, 11 Nov 2019 23:45:49 +0800 Subject: [PATCH] 1. modify the interface of RedLock; 2. fix compiling errors --- src/sw/redis++/recipes/redlock.cpp | 52 ++++++++++------ src/sw/redis++/recipes/redlock.h | 90 ++++++++++++++++++---------- test/src/sw/redis++/redlock_test.hpp | 8 +-- 3 files changed, 94 insertions(+), 56 deletions(-) diff --git a/src/sw/redis++/recipes/redlock.cpp b/src/sw/redis++/recipes/redlock.cpp index 5d39ebdc..0f497ae1 100644 --- a/src/sw/redis++/recipes/redlock.cpp +++ b/src/sw/redis++/recipes/redlock.cpp @@ -45,43 +45,57 @@ std::chrono::milliseconds RedMutex::try_lock(const std::string &val, // Failed to lock more than half masters, or no time left for the lock. unlock(_resource); - throw Error("failed to lock: " + _resource); + return std::chrono::milliseconds(-1); } return time_left; } -bool RedMutex::try_lock(const std::string &val, +std::chrono::milliseconds RedMutex::try_lock(const std::string &val, const std::chrono::time_point &tp) { - try { - try_lock(val, RedLockUtils::ttl(tp)); - } catch (const Error &err) { - return false; - } - - return true; + return try_lock(val, RedLockUtils::ttl(tp)); } -bool RedMutex::extend_lock(const std::string &val, - const std::chrono::time_point &tp) { +std::chrono::milliseconds RedMutex::extend_lock(const std::string &val, + const std::chrono::milliseconds &ttl) { + // TODO: this method is almost duplicate with `try_lock`. I'll refactor it soon. + auto start = std::chrono::steady_clock::now(); + auto lock_cnt = 0U; for (auto &master : _masters) { - if (_extend_lock_master(master.get(), val, tp)) { + if (_extend_lock_master(master.get(), val, ttl)) { ++lock_cnt; } } - return lock_cnt >= _quorum(); + auto lock_ok = lock_cnt >= _quorum(); + + auto stop = std::chrono::steady_clock::now(); + auto elapse = stop - start; + + auto time_left = std::chrono::duration_cast(ttl - elapse); + + if (!lock_ok || time_left < std::chrono::milliseconds(0)) { + // Failed to lock more than half masters, or no time left for the lock. + unlock(_resource); + + return std::chrono::milliseconds(-1); + } + + return time_left; +} + +std::chrono::milliseconds RedMutex::extend_lock(const std::string &val, + const std::chrono::time_point &tp) { + return extend_lock(val, RedLockUtils::ttl(tp)); } bool RedMutex::_extend_lock_master(Redis &master, const std::string &val, - const std::chrono::time_point &tp) { + const std::chrono::milliseconds &ttl) { auto tx = master.transaction(true); auto r = tx.redis(); try { - auto ttl = RedLockUtils::ttl(tp); - r.watch(_resource); auto id = r.get(_resource); @@ -119,7 +133,7 @@ void RedMutex::_unlock_master(Redis &master, const std::string &val) { if (id && *id == val) { auto reply = tx.del(_resource).exec(); if (reply.get(0) != 1) { - throw Error("this should not happen"); + throw Error("Redis internal error: WATCH " + _resource + "failed"); } } } catch (const WatchError &err) { @@ -170,7 +184,7 @@ std::string RedLockUtils::lock_id() { } else if (idx < 10 + 26 + 26) { id.push_back('A' + idx - 10 - 26); } else { - assert(false); + assert(false && "Index out of range in RedLock::_lock_id()"); } } @@ -178,7 +192,7 @@ std::string RedLockUtils::lock_id() { } RedLockMutexVessel::RedLockMutexVessel(Redis& instance) : - RedLockMutexVessel({instance}) + RedLockMutexVessel({{instance}}) { } diff --git a/src/sw/redis++/recipes/redlock.h b/src/sw/redis++/recipes/redlock.h index 3aae9d0b..53a88f5f 100644 --- a/src/sw/redis++/recipes/redlock.h +++ b/src/sw/redis++/recipes/redlock.h @@ -58,10 +58,13 @@ class RedMutex { std::chrono::milliseconds try_lock(const std::string &val, const std::chrono::milliseconds &ttl); - bool try_lock(const std::string &val, + std::chrono::milliseconds try_lock(const std::string &val, const std::chrono::time_point &tp); - bool extend_lock(const std::string &val, + std::chrono::milliseconds extend_lock(const std::string &val, + const std::chrono::milliseconds &ttl); + + std::chrono::milliseconds extend_lock(const std::string &val, const std::chrono::time_point &tp); void unlock(const std::string &val); @@ -77,7 +80,7 @@ class RedMutex { bool _extend_lock_master(Redis &master, const std::string &val, - const std::chrono::time_point &tp); + const std::chrono::milliseconds &ttl); std::size_t _quorum() const { return _masters.size() / 2 + 1; @@ -96,56 +99,76 @@ class RedLock { RedLock(RedisInstance &mut, std::defer_lock_t) : _mut(mut), _lock_val(RedLockUtils::lock_id()) {} ~RedLock() { - if (_owned) { + if (owns_lock()) { unlock(); } } // Try to acquire the lock for *ttl* milliseconds. // Returns how much time still left for the lock, i.e. lock validity time. - std::chrono::milliseconds try_lock(const std::chrono::milliseconds &ttl) { + bool try_lock(const std::chrono::milliseconds &ttl) { auto time_left = _mut.try_lock(_lock_val, ttl); - _owned = true; - return time_left; + if (time_left <= std::chrono::milliseconds(0)) { + return false; + } + + _release_tp = std::chrono::steady_clock::now() + time_left; + + return true; } // Try to acquire the lock, and hold until *tp*. bool try_lock(const std::chrono::time_point &tp) { - if (_mut.try_lock(_lock_val, tp)) { - _owned = true; - } - - return _owned; + return try_lock(RedLockUtils::ttl(tp)); } // Try to extend the lock, and hold it until *tp*. - bool extend_lock(const std::chrono::time_point &tp) { - if (_mut.extend_lock(_lock_val, tp)) { - _owned = true; - } else { - _owned = false; + bool extend_lock(const std::chrono::milliseconds &ttl) { + // TODO: this method is almost duplicate with `try_lock`, and I'll refactor it soon. + auto time_left = _mut.extend_lock(_lock_val, ttl); + if (time_left <= std::chrono::milliseconds(0)) { + return false; } - return _owned; + _release_tp = std::chrono::steady_clock::now() + time_left; + + return true; + } + + bool extend_lock(const std::chrono::time_point &tp) { + return extend_lock(RedLockUtils::ttl(tp)); } void unlock() { try { _mut.unlock(_lock_val); - _owned = false; + _release_tp = std::chrono::time_point{}; } catch (const Error &err) { - _owned = false; + _release_tp = std::chrono::time_point{}; throw; } } -private: + bool owns_lock() const { + if (ttl() <= std::chrono::milliseconds(0)) { + return false; + } - RedisInstance &_mut; + return true; + } - bool _owned = false; + std::chrono::milliseconds ttl() const { + auto t = std::chrono::steady_clock::now(); + return std::chrono::duration_cast(_release_tp - t); + } + +private: + RedisInstance &_mut; std::string _lock_val; + + // The time point that we must release the lock. + std::chrono::time_point _release_tp{}; }; class RedLockMutexVessel @@ -241,33 +264,34 @@ class RedLockMutex RedLockMutex(RedLockMutex &&) = delete; RedLockMutex& operator=(RedLockMutex &&) = delete; - virtual ~RedLockMutex() = default; - std::chrono::milliseconds try_lock(const std::string& random_string, const std::chrono::milliseconds& ttl) { const auto lock_info = _redlock_mutex.lock(_resource, random_string, ttl, 1); if (!lock_info.locked) { - throw Error("failed to lock: " + _resource); + return std::chrono::milliseconds(-1); } return lock_info.time_remaining; } - bool try_lock(const std::string &random_string, + std::chrono::milliseconds try_lock(const std::string &random_string, const std::chrono::time_point &tp) { - const auto lock_info = _redlock_mutex.lock(_resource, random_string, RedLockUtils::ttl(tp), 1); - return lock_info.locked; + return try_lock(random_string, RedLockUtils::ttl(tp)); } - bool extend_lock(const std::string &random_string, - const std::chrono::time_point &tp) + std::chrono::milliseconds extend_lock(const std::string &random_string, + const std::chrono::milliseconds &ttl) { - const auto ttl = RedLockUtils::ttl(tp); const RedLockMutexVessel::LockInfo lock_info = {true, std::chrono::steady_clock::now(), ttl, _resource, random_string}; const auto result = _redlock_mutex.extend_lock(lock_info, ttl); - return result.locked; + return result.time_remaining; + } + + std::chrono::milliseconds extend_lock(const std::string &random_string, + const std::chrono::time_point &tp) { + return extend_lock(random_string, RedLockUtils::ttl(tp)); } void unlock(const std::string &random_string) diff --git a/test/src/sw/redis++/redlock_test.hpp b/test/src/sw/redis++/redlock_test.hpp index b7fa604f..a4e6b6d7 100644 --- a/test/src/sw/redis++/redlock_test.hpp +++ b/test/src/sw/redis++/redlock_test.hpp @@ -79,11 +79,11 @@ class RandomBuffer : public RandomBufferInterface #else // !USE_OPENSSL template -class RandomBuffer : public sw::redis::RandomBufferInterface +class RandomBuffer : public RandomBufferInterface { public: std::string get_updated_string() { - return RedLockMutexVessel::lock_id(); + return RedLockUtils::lock_id(); } }; @@ -150,7 +150,7 @@ void RedLockTest::run() { for (int i=0; i tp = std::chrono::system_clock::now() + multi_lock_ttl; - if (!(mutex_list.back()->try_lock(random_string, tp))) { + if (mutex_list.back()->try_lock(random_string, tp) < std::chrono::milliseconds(0)) { std::cout << "Num locks = " << i << std::endl;; REDIS_ASSERT(0, "unable to obtain a lock"); } @@ -172,7 +172,7 @@ void RedLockTest::run() { for (int i=0; i tp = std::chrono::system_clock::now() + multi_lock_ttl; mutex_list.push(new RedMutex(std::ref(_redis), RedLockUtils::lock_id())); - if (!(mutex_list.back()->try_lock(random_string, tp))) { + if (mutex_list.back()->try_lock(random_string, tp) < std::chrono::milliseconds(0)) { REDIS_ASSERT(0, "unable to obtain a lock"); } }