From 6610be80b608be73fb3b8775b91a8eae81e60146 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 4 Dec 2024 08:03:01 +0100 Subject: [PATCH 1/3] DependencyGraph: Allow lookups by parent & child dependencies --- lib/base/dependencygraph.cpp | 37 +++++++---------- lib/base/dependencygraph.hpp | 78 +++++++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 24 deletions(-) diff --git a/lib/base/dependencygraph.cpp b/lib/base/dependencygraph.cpp index 025eb3ea35a..c2cdbb75545 100644 --- a/lib/base/dependencygraph.cpp +++ b/lib/base/dependencygraph.cpp @@ -5,31 +5,28 @@ using namespace icinga; std::mutex DependencyGraph::m_Mutex; -std::map > DependencyGraph::m_Dependencies; +DependencyGraph::DependencyMap DependencyGraph::m_Dependencies; void DependencyGraph::AddDependency(Object *parent, Object *child) { std::unique_lock lock(m_Mutex); - m_Dependencies[child][parent]++; + if (auto [it, inserted] = m_Dependencies.insert(Edge(parent, child)); !inserted) { + m_Dependencies.modify(it, [](Edge& e) { e.count++; }); + } } void DependencyGraph::RemoveDependency(Object *parent, Object *child) { std::unique_lock lock(m_Mutex); - auto& refs = m_Dependencies[child]; - auto it = refs.find(parent); - - if (it == refs.end()) - return; - - it->second--; - - if (it->second == 0) - refs.erase(it); + if (auto it(m_Dependencies.find(Edge(parent, child))); it != m_Dependencies.end()) { + if (it->count == 1) { + m_Dependencies.erase(it); + return; + } - if (refs.empty()) - m_Dependencies.erase(child); + m_Dependencies.modify(it, [](Edge& e) { e.count--; }); + } } std::vector DependencyGraph::GetParents(const Object::Ptr& child) @@ -37,14 +34,10 @@ std::vector DependencyGraph::GetParents(const Object::Ptr& child) std::vector objects; std::unique_lock lock(m_Mutex); - auto it = m_Dependencies.find(child.get()); - - if (it != m_Dependencies.end()) { - typedef std::pair kv_pair; - for (const kv_pair& kv : it->second) { - objects.emplace_back(kv.first); - } - } + auto [begin, end] = m_Dependencies.get<2>().equal_range(child.get()); + std::transform(begin, end, std::back_inserter(objects), [](const Edge& edge) { + return Object::Ptr(edge.parent); + }); return objects; } diff --git a/lib/base/dependencygraph.hpp b/lib/base/dependencygraph.hpp index 51aa90ecad5..9579625fb87 100644 --- a/lib/base/dependencygraph.hpp +++ b/lib/base/dependencygraph.hpp @@ -5,7 +5,9 @@ #include "base/i2-base.hpp" #include "base/object.hpp" -#include +#include +#include +#include #include namespace icinga { @@ -25,8 +27,80 @@ class DependencyGraph private: DependencyGraph(); + /** + * Represents an undirected dependency edge between two objects. + * + * It allows to traverse the graph in both directions, i.e. from parent to child and vice versa. + */ + struct Edge + { + // Note, due the existing confusing implementation of the DependencyGraph class, the terms parent and + // child are flipped here. Meaning, the parent is the dependent object and the child is the dependency. + // Example: A service object is dependent on a host object, thus the service object is the *parent* and + // the host object is the *child* one :). + Object* parent; + Object* child; + // Is used to track the number of dependent edges of the current one. + // A number <= 1 means, this isn't referenced by anyone and might soon be erased from the container. + // Otherwise, each remove operation will only decrement this by 1 till it reaches 1 and causes the + // edge to completely be erased from the container. + int count; + + Edge(Object* parent, Object* child, int count = 1): parent(parent), child(child), count(count) + { + } + + struct HashPredicate + { + /** + * Generates a unique hash of the given Edge object. + * + * Note, the hash value is generated only by combining the hash values of the parent and child pointers. + * + * @param edge The Edge object to be hashed. + * + * @return size_t The resulting hash value of the given object. + */ + size_t operator()(const Edge& edge) const + { + size_t seed = 0; + boost::hash_combine(seed, edge.parent); + boost::hash_combine(seed, edge.child); + + return seed; + } + + /** + * Compares whether the two Edge objects contain the same parent and child pointers. + * + * Note, the member property count is not taken into account for equality checks. + * + * @param a The first Edge object to compare. + * @param b The second Edge object to compare. + * + * @return bool Returns true if the two objects are equal, false otherwise. + */ + bool operator()(const Edge& a, const Edge& b) const + { + return a.parent == b.parent && a.child == b.child; + } + }; + }; + + using DependencyMap = boost::multi_index_container< + Edge, // The value type we want to sore in the container. + boost::multi_index::indexed_by< + // The first indexer is used for lookups by the Edge object itself, thus it + // needs its own hash function and comparison predicate. + boost::multi_index::hashed_non_unique, Edge::HashPredicate, Edge::HashPredicate>, + // The second and third indexers are used for lookups by the parent and child pointers. + boost::multi_index::hashed_non_unique>, + boost::multi_index::hashed_non_unique> + > + >; + static std::mutex m_Mutex; - static std::map > m_Dependencies; + static DependencyMap m_Dependencies; }; } From 670d1954f98e825d232af6995212597fe8fd48f7 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 4 Dec 2024 08:06:54 +0100 Subject: [PATCH 2/3] DependencyGraph: Introduce `GetChildren()` method --- lib/base/dependencygraph.cpp | 20 ++++++++++++++++++++ lib/base/dependencygraph.hpp | 1 + 2 files changed, 21 insertions(+) diff --git a/lib/base/dependencygraph.cpp b/lib/base/dependencygraph.cpp index c2cdbb75545..28f7542014a 100644 --- a/lib/base/dependencygraph.cpp +++ b/lib/base/dependencygraph.cpp @@ -41,3 +41,23 @@ std::vector DependencyGraph::GetParents(const Object::Ptr& child) return objects; } + +/** + * Returns all the dependent objects of the given parent object. + * + * @param parent The parent object. + * + * @returns A list of the dependent objects. + */ +std::vector DependencyGraph::GetChildren(const Object::Ptr& parent) +{ + std::vector objects; + + std::unique_lock lock(m_Mutex); + auto [begin, end] = m_Dependencies.get<1>().equal_range(parent.get()); + std::transform(begin, end, std::back_inserter(objects), [](const Edge& edge) { + return Object::Ptr(edge.child); + }); + + return objects; +} diff --git a/lib/base/dependencygraph.hpp b/lib/base/dependencygraph.hpp index 9579625fb87..dfdcc7eae6b 100644 --- a/lib/base/dependencygraph.hpp +++ b/lib/base/dependencygraph.hpp @@ -23,6 +23,7 @@ class DependencyGraph static void AddDependency(Object *parent, Object *child); static void RemoveDependency(Object *parent, Object *child); static std::vector GetParents(const Object::Ptr& child); + static std::vector GetChildren(const Object::Ptr& parent); private: DependencyGraph(); From dd6f9b98ce05f2364a46743c666de26abd77aa49 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 16 Sep 2024 09:20:43 +0200 Subject: [PATCH 3/3] ApiListener: Sync runtime configs in order --- lib/remote/apilistener-configsync.cpp | 68 ++++++++++++++++++++++----- lib/remote/apilistener.hpp | 2 + 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/lib/remote/apilistener-configsync.cpp b/lib/remote/apilistener-configsync.cpp index 04436ad8b99..3848cad7156 100644 --- a/lib/remote/apilistener-configsync.cpp +++ b/lib/remote/apilistener-configsync.cpp @@ -5,11 +5,13 @@ #include "remote/configobjectutility.hpp" #include "remote/jsonrpc.hpp" #include "base/configtype.hpp" -#include "base/json.hpp" #include "base/convert.hpp" +#include "base/dependencygraph.hpp" +#include "base/json.hpp" #include "config/vmops.hpp" #include "remote/configobjectslock.hpp" #include +#include using namespace icinga; @@ -393,6 +395,48 @@ void ApiListener::UpdateConfigObject(const ConfigObject::Ptr& object, const Mess } } +/** + * Syncs the specified object and its direct and indirect parents to the provided client + * in topological order of their dependency graph recursively. + * + * Objects that the client does not have access to are skipped without going through their dependency graph. + * + * Please do not use this method to forward remote generated cluster updates; it should only be used to + * send local updates to that specific non-nullptr client. + * + * @param object The config object you want to sync. + * @param azone The zone of the client you want to send the update to. + * @param client The JsonRpc client you send the update to. + * @param syncedObjects Used to cache the already synced objects. + */ +void ApiListener::UpdateConfigObjectWithParents(const ConfigObject::Ptr& object, const Zone::Ptr& azone, + const JsonRpcConnection::Ptr& client, std::unordered_set& syncedObjects) +{ + if (syncedObjects.find(object.get()) != syncedObjects.end()) { + return; + } + + /* don't sync objects for non-matching parent-child zones */ + if (!azone->CanAccessObject(object)) { + return; + } + syncedObjects.emplace(object.get()); + + // Note, due to the confusing implementation of the DependencyGraph, the terms parent and child are flipped, + // thus we need to traverse the graph in the opposite direction. However, when changing the implementation + // of the DependencyGraph, this should be changed to `GetParents()` accordingly. + for (const auto& parent : DependencyGraph::GetChildren(object)) { + // Actually, the following dynamic cast should never fail, since the DependencyGraph class + // expects the types to always be of type Object::Ptr and such an object is supposed to always + // point to an instance of the specific derived ConfigObject class. See TypeHelper<>::GetFactory(). + if (auto parentObj = dynamic_pointer_cast(parent)) { + UpdateConfigObjectWithParents(parentObj, azone, client, syncedObjects); + } + } + + /* send the config object to the connected client */ + UpdateConfigObject(object, nullptr, client); +} void ApiListener::DeleteConfigObject(const ConfigObject::Ptr& object, const MessageOrigin::Ptr& origin, const JsonRpcConnection::Ptr& client) @@ -454,19 +498,17 @@ void ApiListener::SendRuntimeConfigObjects(const JsonRpcConnection::Ptr& aclient Log(LogInformation, "ApiListener") << "Syncing runtime objects to endpoint '" << endpoint->GetName() << "'."; + std::unordered_set syncedObjects; for (const Type::Ptr& type : Type::GetAllTypes()) { - auto *dtype = dynamic_cast(type.get()); - - if (!dtype) - continue; - - for (const ConfigObject::Ptr& object : dtype->GetObjects()) { - /* don't sync objects for non-matching parent-child zones */ - if (!azone->CanAccessObject(object)) - continue; - - /* send the config object to the connected client */ - UpdateConfigObject(object, nullptr, aclient); + if (auto *ctype = dynamic_cast(type.get())) { + for (const auto& object : ctype->GetObjects()) { + // All objects must be synced sorted by their dependency graph. + // Otherwise, downtimes/comments etc. might get synced before their respective Checkables, which will + // result in comments and downtimes being ignored by the other endpoint since it does not yet know + // about their checkables. Given that the runtime config updates event does not trigger a reload on the + // remote endpoint, these objects won't be synced again until the next reload. + UpdateConfigObjectWithParents(object, azone, aclient, syncedObjects); + } } } diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index fced0a8afb1..eae1fa03e61 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -247,6 +247,8 @@ class ApiListener final : public ObjectImpl /* configsync */ void UpdateConfigObject(const ConfigObject::Ptr& object, const MessageOrigin::Ptr& origin, const JsonRpcConnection::Ptr& client = nullptr); + void UpdateConfigObjectWithParents(const ConfigObject::Ptr& object, const Zone::Ptr& azone, + const JsonRpcConnection::Ptr& client, std::unordered_set& syncedObjects); void DeleteConfigObject(const ConfigObject::Ptr& object, const MessageOrigin::Ptr& origin, const JsonRpcConnection::Ptr& client = nullptr); void SendRuntimeConfigObjects(const JsonRpcConnection::Ptr& aclient);