Updated RWLockImpl: Fixed suboptimacy (readers queuing up) + minor tweaks to names and comments (#10303)

* Readers queue correction
* Comments, renamings and cosmetics
This commit is contained in:
Alexander Kazakov 2020-04-16 21:56:21 +03:00 committed by GitHub
parent 80873d79e3
commit d5b3b2c7b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 17 additions and 24 deletions

View File

@ -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();

View File

@ -19,9 +19,15 @@ class RWLockImpl;
using RWLock = std::shared_ptr<RWLockImpl>;
/// 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;
};
}