thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-11-30 03:22:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use executors snapshot

Browse Source
This commit is contained in:
Kruglov Pavel 2023-03-06 13:24:36 +01:00 committed by GitHub
parent 849fac672a
commit aa776d00fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 18 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/Interpreters/ProcessList.cpp
View File

@ -397,10 +397,15 @@ CancellationCode QueryStatus::cancelQuery(bool)
is_killed.store(true); is_killed.store(true);
std::unique_lock lock(executors_mutex); std::vector<ExecutorHolderPtr> executors_snapshot;
for (const auto & e : executors)
{ {
/// We should call cancel() with unlocked executors_mutex, because /// Create a snapshot of executors under a mutex.
std::lock_guard lock(executors_mutex);
executors_snapshot = executors;
}
/// We should call cancel() for each executor with unlocked executors_mutex, because
/// cancel() can try to lock some internal mutex that is already locked by query executing /// cancel() can try to lock some internal mutex that is already locked by query executing
/// thread, and query executing thread can call removePipelineExecutor and lock executors_mutex, /// thread, and query executing thread can call removePipelineExecutor and lock executors_mutex,
/// which will lead to deadlock. /// which will lead to deadlock.
@ -409,11 +414,9 @@ CancellationCode QueryStatus::cancelQuery(bool)
/// 1) We don't allow adding new executors while cancelling query in addPipelineExecutor /// 1) We don't allow adding new executors while cancelling query in addPipelineExecutor
/// 2) We don't actually remove executor holder from executors in removePipelineExecutor, /// 2) We don't actually remove executor holder from executors in removePipelineExecutor,
/// just mark that executor is invalid. /// just mark that executor is invalid.
/// So, it's safe to continue iteration over executors after subsequent mutex locking. /// So, it's ok to use a snapshot created above under a mutex, it won't be any differ from actual executors.
lock.unlock(); for (const auto & e : executors_shapshot)
e->cancel(); e->cancel();
lock.lock();
}
return CancellationCode::CancelSent; return CancellationCode::CancelSent;
} }