From 26f46fe021f8c993e9d9e1cf278180e6a0afef0c Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 7 Feb 2025 11:52:15 +0100 Subject: [PATCH] Simplify dependency group registration Co-Authored-By: Yonas Habteab --- lib/icinga/checkable-dependency.cpp | 90 +++++++++++++++--- lib/icinga/checkable.hpp | 7 +- lib/icinga/dependency-group.cpp | 140 +++++++++------------------- lib/icinga/dependency.cpp | 4 +- lib/icinga/dependency.hpp | 52 ++++++----- 5 files changed, 152 insertions(+), 141 deletions(-) diff --git a/lib/icinga/checkable-dependency.cpp b/lib/icinga/checkable-dependency.cpp index 0904c09cf..21cbcd71f 100644 --- a/lib/icinga/checkable-dependency.cpp +++ b/lib/icinga/checkable-dependency.cpp @@ -15,29 +15,91 @@ using namespace icinga; */ static constexpr int l_MaxDependencyRecursionLevel(256); -void Checkable::AddDependencyGroup(const DependencyGroup::Ptr& dependencyGroup) -{ - std::unique_lock lock(m_DependencyMutex); - m_DependencyGroups.insert(dependencyGroup); -} - -void Checkable::RemoveDependencyGroup(const DependencyGroup::Ptr& dependencyGroup) -{ - std::unique_lock lock(m_DependencyMutex); - m_DependencyGroups.erase(dependencyGroup); -} - std::vector Checkable::GetDependencyGroups() const { std::lock_guard lock(m_DependencyMutex); - return {m_DependencyGroups.begin(), m_DependencyGroups.end()}; + std::vector dependencyGroups; + for (const auto& [_, dependencyGroup] : m_DependencyGroups) { + dependencyGroups.emplace_back(dependencyGroup); + } + return dependencyGroups; +} + +/** + * Get the key for the provided dependency group. + * + * The key is either the parent Checkable object or the redundancy group name of the dependency object. + * This is used to uniquely identify the dependency group within a given Checkable object. + * + * @param dependency The dependency to get the key for. + * + * @return - Returns the key for the provided dependency group. + */ +static std::variant GetDependencyGroupKey(const Dependency::Ptr& dependency) +{ + if (auto redundancyGroup(dependency->GetRedundancyGroup()); !redundancyGroup.IsEmpty()) { + return std::move(redundancyGroup); + } + + return dependency->GetParent().get(); +} + +/** + * Add the provided dependency to the current Checkable list of dependencies. + * + * @param dependency The dependency to add. + */ +void Checkable::AddDependency(const Dependency::Ptr& dependency) +{ + std::lock_guard lock(m_DependencyMutex); + + auto dependencyGroupKey(GetDependencyGroupKey(dependency)); + std::set dependencies; + if (auto it(m_DependencyGroups.find(dependencyGroupKey)); it != m_DependencyGroups.end()) { + dependencies = DependencyGroup::Unregister(it->second, this); + m_DependencyGroups.erase(it); + } + + dependencies.emplace(dependency); + + m_DependencyGroups.emplace( + dependencyGroupKey, + DependencyGroup::Register(new DependencyGroup(dependency->GetRedundancyGroup(), dependencies)) + ); +} + +/** + * Remove the provided dependency from the current Checkable list of dependencies. + * + * @param dependency The dependency to remove. + */ +void Checkable::RemoveDependency(const Dependency::Ptr& dependency) +{ + std::lock_guard lock(m_DependencyMutex); + + auto dependencyGroupKey(GetDependencyGroupKey(dependency)); + auto it = m_DependencyGroups.find(dependencyGroupKey); + if (it == m_DependencyGroups.end()) { + return; + } + + std::set dependencies(DependencyGroup::Unregister(it->second, this)); + m_DependencyGroups.erase(it); + dependencies.erase(dependency); + + if (!dependencies.empty()) { + m_DependencyGroups.emplace( + dependencyGroupKey, + DependencyGroup::Register(new DependencyGroup(dependency->GetRedundancyGroup(), dependencies)) + ); + } } std::vector Checkable::GetDependencies() const { std::unique_lock lock(m_DependencyMutex); std::vector dependencies; - for (const auto& dependencyGroup : m_DependencyGroups) { + for (const auto& [_, dependencyGroup] : m_DependencyGroups) { auto tmpDependencies(dependencyGroup->GetDependenciesForChild(this)); dependencies.insert(dependencies.end(), tmpDependencies.begin(), tmpDependencies.end()); } diff --git a/lib/icinga/checkable.hpp b/lib/icinga/checkable.hpp index 2b9014143..2d29bbec0 100644 --- a/lib/icinga/checkable.hpp +++ b/lib/icinga/checkable.hpp @@ -18,6 +18,7 @@ #include #include #include +#include namespace icinga { @@ -185,9 +186,9 @@ public: bool IsFlapping() const; /* Dependencies */ - void AddDependencyGroup(const intrusive_ptr& dependencyGroup); - void RemoveDependencyGroup(const intrusive_ptr& dependencyGroup); std::vector> GetDependencyGroups() const; + void AddDependency(const intrusive_ptr& dependency); + void RemoveDependency(const intrusive_ptr& dependency); std::vector > GetDependencies() const; bool HasAnyDependencies() const; @@ -249,7 +250,7 @@ private: /* Dependencies */ mutable std::mutex m_DependencyMutex; - std::set> m_DependencyGroups; + std::map, intrusive_ptr> m_DependencyGroups; std::set > m_ReverseDependencies; void GetAllChildrenInternal(std::set& seenChildren, int level = 0) const; diff --git a/lib/icinga/dependency-group.cpp b/lib/icinga/dependency-group.cpp index 8b5aac92d..eabe69a0b 100644 --- a/lib/icinga/dependency-group.cpp +++ b/lib/icinga/dependency-group.cpp @@ -9,106 +9,52 @@ std::mutex DependencyGroup::m_RegistryMutex; DependencyGroup::RegistryType DependencyGroup::m_Registry; /** - * Refresh the global registry of dependency groups. + * Register the provided dependency group to the global dependency group registry. * - * Registers the provided dependency object to an existing dependency group with the same redundancy - * group name (if any), or creates a new one and registers it to the child Checkable and the registry. + * In case there is already an identical dependency group in the registry, the provided dependency group is merged + * with the existing one, and that group is returned. Otherwise, the provided dependency group is registered as is, + * and it's returned. * - * Note: This is a helper function intended for internal use only, and you should acquire the global registry mutex - * before calling this function. - * - * @param dependency The dependency object to refresh the registry for. - * @param unregister A flag indicating whether the provided dependency object should be unregistered from the registry. + * @param dependencyGroup The dependency group to register. */ -void DependencyGroup::RefreshRegistry(const Dependency::Ptr& dependency, bool unregister) +DependencyGroup::Ptr DependencyGroup::Register(const DependencyGroup::Ptr& dependencyGroup) { - auto registerRedundancyGroup = [](const DependencyGroup::Ptr& dependencyGroup) { - if (auto [it, inserted](m_Registry.insert(dependencyGroup.get())); !inserted) { - DependencyGroup::Ptr existingGroup(*it); - dependencyGroup->CopyDependenciesTo(existingGroup); - } - }; - - // Retrieve all the dependency groups with the same redundancy group name of the provided dependency object. - // This allows us to shorten the lookup for the _one_ optimal group to (un)register the dependency from/to. - auto [begin, end] = m_Registry.get<1>().equal_range(dependency->GetRedundancyGroup()); - for (auto it(begin); it != end; ++it) { - DependencyGroup::Ptr existingGroup(*it); - auto child(dependency->GetChild()); - if (auto dependencies(existingGroup->GetDependenciesForChild(child.get())); !dependencies.empty()) { - m_Registry.erase(existingGroup->GetCompositeKey()); // Will be re-registered when needed down below. - if (unregister) { - existingGroup->RemoveDependency(dependency); - // Remove the connection between the child Checkable and the dependency group if it has no members - // left or the above removed member was the only member of the group that the child depended on. - if (existingGroup->IsEmpty() || dependencies.size() == 1) { - child->RemoveDependencyGroup(existingGroup); - } - } - - size_t totalDependencies(existingGroup->GetDependenciesCount()); - // If the existing dependency group has an identical member already, or the child Checkable of the - // dependency object is the only member of it (totalDependencies == dependencies.size()), we can simply - // add the dependency object to the existing group. - if (!unregister && (existingGroup->HasParentWithConfig(dependency) || totalDependencies == dependencies.size())) { - existingGroup->AddDependency(dependency); - } else if (!unregister || (dependencies.size() > 1 && totalDependencies >= dependencies.size())) { - // The child Checkable is going to have a new dependency group, so we must detach the existing one. - child->RemoveDependencyGroup(existingGroup); - - Ptr replacementGroup(unregister ? nullptr : new DependencyGroup(existingGroup->GetRedundancyGroupName(), dependency)); - for (auto& existingDependency : dependencies) { - if (existingDependency != dependency) { - existingGroup->RemoveDependency(existingDependency); - if (replacementGroup) { - replacementGroup->AddDependency(existingDependency); - } else { - replacementGroup = new DependencyGroup(existingGroup->GetRedundancyGroupName(), existingDependency); - } - } - } - - child->AddDependencyGroup(replacementGroup); - registerRedundancyGroup(replacementGroup); - } - - if (!existingGroup->IsEmpty()) { - registerRedundancyGroup(existingGroup); - } - return; - } - } - - if (!unregister) { - // We couldn't find any existing dependency group to register the dependency to, so we must - // initiate a new one and attach it to the child Checkable and register to the global registry. - DependencyGroup::Ptr newGroup(new DependencyGroup(dependency->GetRedundancyGroup())); - newGroup->AddDependency(dependency); - dependency->GetChild()->AddDependencyGroup(newGroup); - registerRedundancyGroup(newGroup); + std::lock_guard lock(m_RegistryMutex); + if (auto [it, inserted] = m_Registry.insert(dependencyGroup); !inserted) { + dependencyGroup->CopyDependenciesTo(*it); + return *it; } + return dependencyGroup; } /** - * Register the provided dependency to the global dependency group registry. + * Detach the provided child Checkable from the specified dependency group. * - * @param dependency The dependency to register. + * Unregisters all the dependency objects the child Checkable depends on from the provided dependency group and + * removes the dependency group from the global registry if it becomes empty afterward. + * + * @param dependencyGroup The dependency group to unregister the child Checkable from. + * @param child The child Checkable to detach from the dependency group. + * + * @return - Returns the dependency objects of the child Checkable that were member of the provided dependency group. */ -void DependencyGroup::Register(const Dependency::Ptr& dependency) +std::set DependencyGroup::Unregister(const DependencyGroup::Ptr& dependencyGroup, const Checkable::Ptr& child) { std::lock_guard lock(m_RegistryMutex); - RefreshRegistry(dependency, false); -} + std::vector dependencies; + if (auto it(m_Registry.find(dependencyGroup)); it != m_Registry.end()) { + const auto& existingGroup(*it); + dependencies = existingGroup->GetDependenciesForChild(child.get()); -/** - * Unregister the provided dependency from the dependency group it was member of. - * - * @param dependency The dependency to unregister. - */ -void DependencyGroup::Unregister(const Dependency::Ptr& dependency) -{ - std::lock_guard lock(m_RegistryMutex); - RefreshRegistry(dependency, true); + for (const auto& dependency : dependencies) { + existingGroup->RemoveDependency(dependency); + } + + if (existingGroup->IsEmpty()) { + m_Registry.erase(it); + } + } + return {dependencies.begin(), dependencies.end()}; } /** @@ -126,6 +72,13 @@ DependencyGroup::DependencyGroup(String name): m_RedundancyGroupName(std::move(n { } +DependencyGroup::DependencyGroup(String name, const std::set& dependencies): m_RedundancyGroupName(std::move(name)) +{ + for (const auto& dependency : dependencies) { + AddDependency(dependency); + } +} + /** * Create a composite key for the provided dependency. * @@ -245,17 +198,10 @@ void DependencyGroup::CopyDependenciesTo(const DependencyGroup::Ptr& dest) VERIFY(this != dest); // Prevent from doing something stupid, i.e. deadlocking ourselves. std::lock_guard lock(m_Mutex); - DependencyGroup::Ptr thisPtr(this); // Just in case the Checkable below was our last reference. for (auto& [_, children] : m_Members) { - Checkable::Ptr previousChild; - for (auto& [checkable, dependency] : children) { - dest->AddDependency(dependency); - if (!previousChild || previousChild != checkable) { - previousChild = dependency->GetChild(); - previousChild->RemoveDependencyGroup(thisPtr); - previousChild->AddDependencyGroup(dest); - } - } + std::for_each(children.begin(), children.end(), [&dest](const auto& pair) { + dest->AddDependency(pair.second); + }); } } diff --git a/lib/icinga/dependency.cpp b/lib/icinga/dependency.cpp index 2f2482136..a9a7bf372 100644 --- a/lib/icinga/dependency.cpp +++ b/lib/icinga/dependency.cpp @@ -251,7 +251,7 @@ void Dependency::OnAllConfigLoaded() // InitChildParentReferences() has to be called before. VERIFY(m_Child && m_Parent); - DependencyGroup::Register(this); + m_Child->AddDependency(this); m_Parent->AddReverseDependency(this); } @@ -259,7 +259,7 @@ void Dependency::Stop(bool runtimeRemoved) { ObjectImpl::Stop(runtimeRemoved); - DependencyGroup::Unregister(this); + GetChild()->RemoveDependency(this); GetParent()->RemoveReverseDependency(this); } diff --git a/lib/icinga/dependency.hpp b/lib/icinga/dependency.hpp index 63bab92ad..0698a3e0a 100644 --- a/lib/icinga/dependency.hpp +++ b/lib/icinga/dependency.hpp @@ -8,9 +8,6 @@ #include "icinga/i2-icinga.hpp" #include "icinga/dependency-ti.hpp" #include "icinga/timeperiod.hpp" -#include -#include -#include #include #include #include @@ -136,9 +133,10 @@ public: using MembersMap = std::map; explicit DependencyGroup(String name); + DependencyGroup(String name, const std::set& dependencies); - static void Register(const Dependency::Ptr& dependency); - static void Unregister(const Dependency::Ptr& dependency); + static DependencyGroup::Ptr Register(const DependencyGroup::Ptr& dependencyGroup); + static std::set Unregister(const DependencyGroup::Ptr& dependencyGroup, const Checkable::Ptr& child); static size_t GetRegistrySize(); static CompositeKeyType MakeCompositeKeyFor(const Dependency::Ptr& dependency); @@ -176,7 +174,29 @@ protected: void RemoveDependency(const Dependency::Ptr& dependency); void CopyDependenciesTo(const DependencyGroup::Ptr& dest); - static void RefreshRegistry(const Dependency::Ptr& dependency, bool unregister); + struct Hash + { + size_t operator()(const DependencyGroup::Ptr& dependencyGroup) const + { + size_t hash = 0; + for (const auto& [key, group] : dependencyGroup->m_Members) { + boost::hash_combine(hash, key); + } + return hash; + } + }; + + struct Equal + { + bool operator()(const DependencyGroup::Ptr& lhs, const DependencyGroup::Ptr& rhs) const + { + return std::equal( + lhs->m_Members.begin(), lhs->m_Members.end(), + rhs->m_Members.begin(), rhs->m_Members.end(), + [](const auto& l, const auto& r) { return l.first == r.first; } + ); + } + }; private: mutable std::mutex m_Mutex; @@ -193,25 +213,7 @@ private: String m_RedundancyGroupName; MembersMap m_Members; - using RegistryType = boost::multi_index_container< - DependencyGroup*, // The type of the elements stored in the container. - boost::multi_index::indexed_by< - // This unique index allows to search/erase dependency groups by their composite key in an efficient manner. - boost::multi_index::hashed_unique< - boost::multi_index::mem_fun, - std::hash - >, - // This non-unique index allows to search for dependency groups by their name, and reduces the overall - // runtime complexity. Without this index, we would have to iterate over all elements to find the one - // with the desired members and since containers don't allow erasing elements while iterating, we would - // have to copy each of them to a temporary container, and then erase and reinsert them back to the original - // container. This produces way too much overhead, and slows down the startup time of Icinga 2 significantly. - boost::multi_index::hashed_non_unique< - boost::multi_index::const_mem_fun, - std::hash - > - > - >; + using RegistryType = std::unordered_set; // The global registry of dependency groups. static std::mutex m_RegistryMutex;