diff --git a/src/Common/RWLock.cpp b/src/Common/RWLock.cpp index d6b8cbd244f..a8dba490fac 100644 --- a/src/Common/RWLock.cpp +++ b/src/Common/RWLock.cpp @@ -154,23 +154,17 @@ RWLockImpl::getLock(RWLockImpl::Type type, const String & query_id, const std::c writers_queue.emplace_back(type); /// SM1: may throw (nothing to roll back) } else if (readers_queue.empty() || - (rdlock_owner == readers_queue.begin() && !writers_queue.empty())) + (rdlock_owner == readers_queue.begin() && readers_queue.size() == 1 && !writers_queue.empty())) { readers_queue.emplace_back(type); /// SM1: may throw (nothing to roll back) } GroupsContainer::iterator it_group = (type == Type::Write) ? std::prev(writers_queue.end()) : std::prev(readers_queue.end()); + /// Lock is free to acquire if (rdlock_owner == readers_queue.end() && wrlock_owner == writers_queue.end()) { - if (type == Type::Read) - { - rdlock_owner = it_group; /// SM2: nothrow - } - else - { - wrlock_owner = it_group; /// SM2: nothrow - } + (type == Read ? rdlock_owner : wrlock_owner) = it_group; /// SM2: nothrow } else { @@ -192,17 +186,10 @@ RWLockImpl::getLock(RWLockImpl::Type type, const String & query_id, const std::c /// Step 3a. Check if we must handle timeout and exit if (!wait_result) /// Wait timed out! { + /// Rollback(SM1): nothrow if (it_group->requests == 0) { - /// Roll back SM1 - if (type == Read) - { - readers_queue.erase(it_group); /// Rollback(SM1): nothrow - } - else - { - writers_queue.erase(it_group); /// Rollback(SM1): nothrow - } + (type == Read ? readers_queue : writers_queue).erase(it_group); } return nullptr; @@ -224,7 +211,7 @@ RWLockImpl::getLock(RWLockImpl::Type type, const String & query_id, const std::c /// Methods std::list<>::emplace_back() and std::unordered_map<>::emplace() provide strong exception safety /// We only need to roll back the changes to these objects: owner_queries and the readers/writers queue if (it_group->requests == 0) - eraseGroup(it_group); /// Rollback(SM1): nothrow + dropOwnerGroupAndPassOwnership(it_group); /// Rollback(SM1): nothrow throw; } @@ -272,11 +259,11 @@ void RWLockImpl::unlock(GroupsContainer::iterator group_it, const String & query /// If we are the last remaining referrer, remove this QNode and notify the next one if (--group_it->requests == 0) /// SM: nothrow - eraseGroup(group_it); + dropOwnerGroupAndPassOwnership(group_it); } -void RWLockImpl::eraseGroup(GroupsContainer::iterator group_it) noexcept +void RWLockImpl::dropOwnerGroupAndPassOwnership(GroupsContainer::iterator group_it) noexcept { rdlock_owner = readers_queue.end(); wrlock_owner = writers_queue.end(); diff --git a/src/Common/RWLock.h b/src/Common/RWLock.h index 43366192cf8..ad0a3f139fc 100644 --- a/src/Common/RWLock.h +++ b/src/Common/RWLock.h @@ -19,9 +19,15 @@ class RWLockImpl; using RWLock = std::shared_ptr; -/// Implements shared lock with FIFO service +/// Implements Readers-Writers locking algorithm that serves requests in "Phase Fair" order. /// (Phase Fair RWLock as suggested in https://www.cs.unc.edu/~anderson/papers/rtsj10-for-web.pdf) -/// Can be acquired recursively (for the same query) in Read mode +/// It is used for synchronizing access to various objects on query level (i.e. Storages). +/// +/// In general, ClickHouse processes queries by multiple threads of execution in parallel. +/// As opposed to the standard OS synchronization primitives (mutexes), this implementation allows +/// unlock() to be called by a thread other than the one, that called lock(). +/// It is also possible to acquire RWLock in Read mode without waiting (FastPath) by multiple threads, +/// that execute the same query (share the same query_id). /// /// NOTE: it is important to allow acquiring the same lock in Read mode without waiting if it is already /// acquired by another thread of the same query. Otherwise the following deadlock is possible: @@ -82,6 +88,6 @@ private: private: RWLockImpl() = default; void unlock(GroupsContainer::iterator group_it, const String & query_id) noexcept; - void eraseGroup(GroupsContainer::iterator group_it) noexcept; + void dropOwnerGroupAndPassOwnership(GroupsContainer::iterator group_it) noexcept; }; }