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

Merge pull request #68356 from ClickHouse/fix-scheduler-data-race

Browse Source 
Fix data race in `DynamicResourceManager::updateConfiguration`
This commit is contained in:
Kseniia Sumarokova 2024-08-15 09:07:19 +00:00 committed by GitHub
commit 172d379a83
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 11 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/Common/Scheduler/Nodes/DynamicResourceManager.cpp
View File

@ -184,14 +184,20 @@ void DynamicResourceManager::updateConfiguration(const Poco::Util::AbstractConfi
// Resource update leads to loss of runtime data of nodes and may lead to temporary violation of constraints (e.g. limits)
// Try to minimise this by reusing "equal" resources (initialized with the same configuration).
std::vector<State::ResourcePtr> resources_to_attach;
for (auto & [name, new_resource] : new_state->resources)
{
if (auto iter = state->resources.find(name); iter != state->resources.end()) // Resource update
{
State::ResourcePtr old_resource = iter->second;
if (old_resource->equals(*new_resource))
{
new_resource = old_resource; // Rewrite with older version to avoid loss of runtime data
continue;
}
}
// It is new or updated resource
resources_to_attach.emplace_back(new_resource);
}
// Commit new state
@ -199,17 +205,14 @@ void DynamicResourceManager::updateConfiguration(const Poco::Util::AbstractConfi
state = new_state;
// Attach new and updated resources to the scheduler
for (auto & [name, resource] : new_state->resources)
for (auto & resource : resources_to_attach)
{
const SchedulerNodePtr & root = resource->nodes.find("/")->second.ptr;
if (root->parent == nullptr)
resource->attached_to = &scheduler;
scheduler.event_queue->enqueue([this, root]
{
resource->attached_to = &scheduler;
scheduler.event_queue->enqueue([this, root]
{
scheduler.attachChild(root);
});
}
scheduler.attachChild(root);
});
}
// NOTE: after mutex unlock `state` became available for Classifier(s) and must be immutable