diff --git a/score/launch_manager/src/daemon/src/control/control_client_channel.hpp b/score/launch_manager/src/daemon/src/control/control_client_channel.hpp index 032d49cf8..47eb32138 100644 --- a/score/launch_manager/src/daemon/src/control/control_client_channel.hpp +++ b/score/launch_manager/src/daemon/src/control/control_client_channel.hpp @@ -34,15 +34,15 @@ namespace internal /// @brief This is initially some ID provided by the Control Client library. When received /// by Control Client handler additional information is added - the state manager /// process originating the request. This can be given in the form of a function -/// group index and process index. +/// group index and process identifier. /// When the Control Client library receives a response, it must be able to extract /// the client ID, ignoring the state manager process identification. struct ControlClientID final { - uint16_t process_group_index_; ///< Process group containing the state manager process - uint16_t process_index_; ///< The process within the process group - uint32_t future_id_; ///< ID to match request and response - ControlClientID() : process_group_index_(0), process_index_(0), future_id_(0) + uint16_t process_group_index_; ///< Process group containing the state manager process + IdentifierHash process_identifier_; ///< The process within the process group + uint32_t future_id_; ///< ID to match request and response + ControlClientID() : process_group_index_(0), process_identifier_(""), future_id_(0) { } ///< For use by Control Client }; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/BUILD b/score/launch_manager/src/daemon/src/process_group_manager/details/BUILD index de31199ee..fd0a50f1d 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/BUILD +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/BUILD @@ -44,6 +44,7 @@ cc_library( strip_include_prefix = "/score/launch_manager/src/daemon/src/process_group_manager/details", visibility = ["//score:__subpackages__"], deps = [ + "//score/launch_manager/src/daemon/src/common:identifier_hash", "@score_baselibs//score/language/futurecpp", ], ) @@ -242,6 +243,7 @@ cc_library( strip_include_prefix = "/score/launch_manager/src/daemon/src/process_group_manager/details", visibility = ["//score/launch_manager/src/daemon/src/process_group_manager:__pkg__"], deps = [ + ":component_of", ":dependency_graph", "//score/launch_manager/src/daemon/src/common:constants", "//score/launch_manager/src/daemon/src/common/concurrency:fixed_size_queue", diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp index 848283ee3..b668a2373 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event.hpp @@ -26,32 +26,32 @@ namespace score::mw::lifecycle::internal /// @brief A node finished activating successfully. struct [[nodiscard]] ActivationSuccessful { - uint32_t node_index; + IdentifierHash node_identifier; }; /// @brief A node failed to activate. struct [[nodiscard]] ActivationFailed { - uint32_t node_index; + IdentifierHash node_identifier; IComponent::ComponentError reason; }; /// @brief A node finished deactivating. struct [[nodiscard]] DeactivationComplete { - uint32_t node_index; + IdentifierHash node_identifier; }; /// @brief A node terminated without having been requested to. struct [[nodiscard]] UnexpectedTermination { - uint32_t node_index; + IdentifierHash node_identifier; }; /// @brief A job was queued but cancelled by the time it was processed struct [[nodiscard]] JobSkipped { - uint32_t node_index; + IdentifierHash node_identifier; }; /// @brief Alive supervision has failed for the given process identifier. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp index 36b94c0f3..ee29c164e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_event_queue_UT.cpp @@ -40,7 +40,7 @@ TEST_F(ComponentEventQueueTest, WaitForEventsReturnsFalseOnEmptyQueue) TEST_F(ComponentEventQueueTest, WaitForEventsReturnsTrueAfterPush) { RecordProperty("Description", "Verify waitForEvents returns true once an event has been pushed."); - EXPECT_TRUE(queue_.push(ActivationSuccessful{7U})); + EXPECT_TRUE(queue_.push(ActivationSuccessful{IdentifierHash{"process"}})); EXPECT_TRUE(queue_.waitForEvents(std::chrono::milliseconds{0})); } @@ -53,13 +53,14 @@ TEST_F(ComponentEventQueueTest, GetNextEventReturnsNulloptWhenEmpty) TEST_F(ComponentEventQueueTest, GetNextEventReturnsPushedEventWithPayloadIntact) { RecordProperty("Description", "Verify a pushed event is returned by getNextEvent with its payload preserved."); - EXPECT_TRUE(queue_.push(ActivationFailed{3U, IComponent::ComponentError::kErrorBeforeReady})); + const IdentifierHash process_identifier{"payload"}; + EXPECT_TRUE(queue_.push(ActivationFailed{process_identifier, IComponent::ComponentError::kErrorBeforeReady})); auto event = queue_.getNextEvent(); ASSERT_TRUE(event.has_value()); ASSERT_TRUE(std::holds_alternative(*event)); const auto& failed = std::get(*event); - EXPECT_EQ(failed.node_index, 3U); + EXPECT_EQ(failed.node_identifier, process_identifier); EXPECT_EQ(failed.reason, IComponent::ComponentError::kErrorBeforeReady); } @@ -82,7 +83,7 @@ TEST_F(ComponentEventQueueTest, GetNextEventReturnsSupervisionFailureWithPayload TEST_F(ComponentEventQueueTest, GetOverflowStaysFalseUnderNormalUsage) { RecordProperty("Description", "Verify getOverflow() stays false when events are pushed and drained normally."); - EXPECT_TRUE(queue_.push(ActivationSuccessful{1U})); + EXPECT_TRUE(queue_.push(ActivationSuccessful{IdentifierHash{"process"}})); static_cast(queue_.getNextEvent()); EXPECT_FALSE(queue_.getOverflow()); } @@ -95,13 +96,13 @@ TEST_F(ComponentEventQueueTest, GetOverflowBecomesTrueOnceQueueIsFull) "mirroring how ProcessGroupManager::run() detects lost events."); for (std::size_t i = 0U; i < queue_.capacity(); ++i) { - EXPECT_TRUE(queue_.push(ActivationSuccessful{static_cast(i)})); + EXPECT_TRUE(queue_.push(ActivationSuccessful{IdentifierHash{"process"}})); } EXPECT_FALSE(queue_.getOverflow()); // One more push while the queue is already at capacity and nobody is draining it: this // push is dropped immediately, flagging overflow. - EXPECT_FALSE(queue_.push(ActivationSuccessful{9999U})); + EXPECT_FALSE(queue_.push(ActivationSuccessful{IdentifierHash{"process"}})); EXPECT_TRUE(queue_.getOverflow()); } @@ -111,7 +112,7 @@ TEST_F(ComponentEventQueueTest, StopFailsWaitForEventsOnEmptyQueue) "Description", "Verify stop() causes a subsequently-called waitForEvents() to return false, even if there's an event in the " "queue"); - EXPECT_TRUE(queue_.push(ActivationSuccessful{1})); + EXPECT_TRUE(queue_.push(ActivationSuccessful{IdentifierHash{"process"}})); queue_.stop(); EXPECT_FALSE(queue_.waitForEvents(std::chrono::milliseconds{0})); } @@ -122,7 +123,7 @@ TEST_F(ComponentEventQueueTest, GetNextEventStillDrainsQueuedEventsAfterStop) "Description", "Verify that events pushed before stop() was called are not silently discarded -- " "getNextEvent() must still be able to drain them during shutdown."); - EXPECT_TRUE(queue_.push(ActivationSuccessful{1U})); + EXPECT_TRUE(queue_.push(ActivationSuccessful{IdentifierHash{"process"}})); queue_.stop(); auto event = queue_.getNextEvent(); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/component_of.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/component_of.hpp index 203b5c8ef..3ea0a5135 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/component_of.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/component_of.hpp @@ -21,6 +21,7 @@ namespace score::mw::lifecycle::internal { + /// @brief Returns the IComponent reference from a variant type /// @details All types in the variant must implement the IComponent interface. inline IComponent& componentOf(std::variant& node) @@ -32,6 +33,13 @@ inline IComponent& componentOf(std::variant& node) node); } +/// @brief Returns the IComponent reference from an interface pointer. Useful for a template class that may take a +/// variant or a generic interface +inline IComponent& componentOf(IComponent* node) +{ + return *node; +} + } // namespace score::mw::lifecycle::internal #endif // SCORE_LCM_COMPONENT_OF_HPP_INCLUDED diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/dependency_graph.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/dependency_graph.hpp index bd59799f6..b5373c373 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/dependency_graph.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/dependency_graph.hpp @@ -18,114 +18,137 @@ #include #include +#include #include namespace score::mw::lifecycle { -/// @brief Index type used to identify nodes in the graph. -using GraphIndex = std::size_t; - /// @brief Stores a set of nodes as a directed acyclic graph (DAG) with edges representing dependencies between nodes. /// @details The class provides methods to create and traverse the graph. -template +template class DependencyGraph { private: + static_assert( + std::is_trivially_copyable_v, + "This class takes copies of keys so they should be trivially copyable"); + /// @brief Wrapper around objects in the graph to store information about dependencies. struct GraphNode { + /// @brief The underlying object stored in this graph. T value; - std::vector depends_on; - std::vector dependents; + /// @brief Nodes that this node needs to be ready before it can launch. + std::vector depends_on; + /// @brief Nodes that depend on this node being ready before they can launch. + std::vector dependents; + + /// @brief Temporary flag set when this node is traversed. + /// @warning This should be reset at the start of each traversal for valid results. + bool visited{false}; /// @brief Constructor to allow in-place construction of T. template - GraphNode(Args&&... args) : value(std::forward(args)...) + explicit GraphNode(Args&&... args) : value(std::forward(args)...) { } }; + using iterator = typename std::unordered_map::iterator; + public: /// @param count The exact number of nodes that will be added. /// /// @details The size of the internal traversal queue is either count - 1 or 1. This is because in each traversal /// one node is pushed to the queue and then popped. From then on, dependencies are pushed to the queue. - DependencyGraph(const std::size_t count) : traversal_queue(std::max(count, 2UL) - 1) + explicit DependencyGraph(const std::size_t count) : capacity_(count), traversal_queue(std::max(count, 2UL) - 1) { nodes.reserve(count); - visited.resize(count); } - /// @brief Construct a new node in-place. Returns the node's index, which equals the current size - /// before insertion (i.e. the first node is 0, second is 1, etc.). + /// @brief Construct a new node in-place. Returns the node's key. + /// @warning If the key is already present in the graph, the new node is not inserted. template - GraphIndex emplace(Args&&... args) + Key try_emplace(Key key, Args&&... args) { - nodes.emplace_back(std::forward(args)...); - return nodes.size() - 1; + std::pair res = nodes.try_emplace(key, std::forward(args)...); + SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE( + res.second, "Element was not inserted. This means that the key was already present in the map."); + return res.first->first; } /// @brief Add an edge: @p node depends on @p depends_on. /// During activation, depends_on will be started before node. /// During deactivation, node will be stopped before depends_on. - void addDependency(const GraphIndex node, const GraphIndex depends_on) + /// @pre @p node and @p depends_on must both be present in the graph + void addDependency(Key node, Key depends_on) { SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE( - nodes[node].depends_on.size() < capacity(), "More dependencies added than there are nodes in the graph"); + nodes.at(node).depends_on.size() < capacity(), "More dependencies added than there are nodes in the graph"); SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE( - nodes[depends_on].dependents.size() < capacity(), + nodes.at(depends_on).dependents.size() < capacity(), "More dependencies added than there are nodes in the graph"); - nodes[node].depends_on.push_back(depends_on); - nodes[depends_on].dependents.push_back(node); + nodes.at(node).depends_on.push_back(depends_on); + nodes.at(depends_on).dependents.push_back(node); } /// @return The number of nodes in the graph. - std::size_t size() const + std::size_t size() const noexcept { return nodes.size(); } /// @return The number of nodes this graph can hold without reallocating (the @c count /// reserved at construction). - std::size_t capacity() const + std::size_t capacity() const noexcept { - return nodes.capacity(); + return capacity_; } - T& operator[](GraphIndex index) + /// @brief Returns a mutable reference to the node at @p key + /// @pre @p key must be present in the graph. + T& operator[](Key key) { - return nodes[index].value; + return nodes.at(key).value; } - const T& operator[](const GraphIndex index) const + /// @brief Returns a constant reference to the node at @p key + /// @pre @p key must be present in the graph. + const T& operator[](const Key key) const { - return nodes[index].value; + return nodes.at(key).value; } - /// @return The nodes that @p index depends on. - const std::vector& dependsOn(GraphIndex index) const + /// @return The nodes that @p key depends on. + /// @pre @p key must be present in the graph. + const std::vector& dependsOn(Key key) const { - return nodes[index].depends_on; + return nodes.at(key).depends_on; } - /// @return The nodes that depend on @p index. - const std::vector& dependents(GraphIndex index) const + /// @return The nodes that depend on @p key. + /// @pre @p key must be present in the graph. + const std::vector& dependents(Key key) const { - return nodes[index].dependents; + return nodes.at(key).dependents; } /// @brief Traverse the graph, starting at @p start, performing @p per_node /// on each node and moving to the nodes provided by the return /// value from @p per_node. + /// @pre @p start must be present in the graph. template - void traverse(const GraphIndex start, PerNodeFn per_node) + void traverse(Key start, PerNodeFn per_node) { - visited.assign(visited.size(), false); + std::for_each(nodes.begin(), nodes.end(), [](std::pair& it) { + GraphNode& node = it.second; + node.visited = false; + }); auto push_res = traversal_queue.push(start); static_cast(push_res); SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE(push_res, "Traversal queue was already full"); - visited[start] = true; + nodes.at(start).visited = true; while (!traversal_queue.empty()) { const auto pop_res = traversal_queue.tryPop(); @@ -136,13 +159,13 @@ class DependencyGraph for (const auto neighbor : neighbors) { - if (visited[neighbor]) + if (nodes.at(neighbor).visited) { continue; } push_res = traversal_queue.push(neighbor); SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE(push_res, "Traversal queue was already full"); - visited[neighbor] = true; + nodes.at(neighbor).visited = true; } } } @@ -150,10 +173,10 @@ class DependencyGraph /// @brief Iterator over node values. struct ValueIterator { - typename std::vector::iterator it; - T& operator*() + iterator it; + std::pair operator*() { - return it->value; + return std::pair(it->first, it->second.value); } ValueIterator& operator++() @@ -166,6 +189,11 @@ class DependencyGraph { return it != other.it; } + + bool operator==(const ValueIterator& other) const + { + return it == other.it; + } }; /// @returns Iterator at the beginning of the nodes store. @@ -180,13 +208,19 @@ class DependencyGraph return ValueIterator{nodes.end()}; } + ValueIterator find(Key key) + { + return ValueIterator{nodes.find(key)}; + } + private: - std::vector nodes; + /// @brief The number of nodes the graph expects to hold + std::size_t capacity_; + + std::unordered_map nodes; /// @brief Presized queue reused by single-threaded traversals. - internal::FixedSizeQueue traversal_queue; - /// @brief Presized visited set reused by single-threaded traversals. - std::vector visited; + internal::FixedSizeQueue traversal_queue; }; } // namespace score::mw::lifecycle diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/dependency_graph_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/dependency_graph_UT.cpp index 5daf45459..a1de490da 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/dependency_graph_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/dependency_graph_UT.cpp @@ -25,30 +25,18 @@ namespace score::mw::lifecycle TEST(DependencyGraphTest, EmplaceAndAccessByIndex) { const std::string_view text = "AAAAA"; - DependencyGraph graph(1); - const auto res = graph.emplace(text); + DependencyGraph graph(1); + const auto res = graph.try_emplace(IdentifierHash{text}, text); auto& hash = graph[res]; - EXPECT_EQ(hash, IdentifierHash{text}); -} - -TEST(DependencyGraphTest, EmplaceReturnsSequentialIndices) -{ - DependencyGraph graph(3); - const auto first = graph.emplace("a"); - const auto second = graph.emplace("b"); - const auto third = graph.emplace("c"); - - EXPECT_EQ(first, 0U); - EXPECT_EQ(second, 1U); - EXPECT_EQ(third, 2U); + EXPECT_EQ(hash, text); } TEST(DependencyGraphTest, AddDependencyWiresDependsOnAndDependents) { - DependencyGraph graph(2); - const auto dep = graph.emplace("dep"); - const auto root = graph.emplace("root"); + DependencyGraph graph(2); + const auto dep = graph.try_emplace(IdentifierHash{"dep"}, "dep"); + const auto root = graph.try_emplace(IdentifierHash{"root"}, "root"); graph.addDependency(root, dep); EXPECT_THAT(graph.dependsOn(root), ::testing::ElementsAre(dep)); @@ -59,50 +47,48 @@ TEST(DependencyGraphTest, AddDependencyWiresDependsOnAndDependents) TEST(DependencyGraphTest, SizeReflectsNumberOfEmplacedNodes) { - DependencyGraph graph(2); + DependencyGraph graph(2); EXPECT_EQ(graph.size(), 0U); - graph.emplace("a"); + graph.try_emplace(IdentifierHash{"a"}, "a"); EXPECT_EQ(graph.size(), 1U); - graph.emplace("b"); + graph.try_emplace(IdentifierHash{"b"}, "b"); EXPECT_EQ(graph.size(), 2U); } TEST(DependencyGraphTest, TraverseVisitsWholeChainThroughDependsOn) { // root -> mid -> leaf (X -> Y means X depends_on Y) - DependencyGraph graph(3); - const auto leaf = graph.emplace("leaf"); - const auto mid = graph.emplace("mid"); - const auto root = graph.emplace("root"); + DependencyGraph graph(3); + const auto leaf = graph.try_emplace(IdentifierHash{"leaf"}, "leaf"); + const auto mid = graph.try_emplace(IdentifierHash{"mid"}, "mid"); + const auto root = graph.try_emplace(IdentifierHash{"root"}, "root"); graph.addDependency(root, mid); graph.addDependency(mid, leaf); - std::vector visited; - graph.traverse(root, [&](GraphIndex i) -> const std::vector& { + std::vector visited; + graph.traverse(root, [&](IdentifierHash i) -> const std::vector& { visited.push_back(graph[i]); return graph.dependsOn(i); }); - EXPECT_THAT( - visited, - ::testing::UnorderedElementsAre(IdentifierHash{"root"}, IdentifierHash{"mid"}, IdentifierHash{"leaf"})); + EXPECT_THAT(visited, ::testing::UnorderedElementsAre("leaf", "mid", "root")); } TEST(DependencyGraphTest, TraverseVisitsSharedDependencyExactlyOnce) { // Diamond: both a and b depend on shared; root depends on both a and b. - DependencyGraph graph(4); - const auto shared = graph.emplace("shared"); - const auto a = graph.emplace("a"); - const auto b = graph.emplace("b"); - const auto root = graph.emplace("root"); + DependencyGraph graph(4); + const auto shared = graph.try_emplace(IdentifierHash{"shared"}, "shared"); + const auto a = graph.try_emplace(IdentifierHash{"a"}, "a"); + const auto b = graph.try_emplace(IdentifierHash{"b"}, "b"); + const auto root = graph.try_emplace(IdentifierHash{"root"}, "root"); graph.addDependency(a, shared); graph.addDependency(b, shared); graph.addDependency(root, a); graph.addDependency(root, b); std::size_t shared_visits = 0; - graph.traverse(root, [&](GraphIndex i) -> const std::vector& { + graph.traverse(root, [&](IdentifierHash i) -> const std::vector& { if (i == shared) { ++shared_visits; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp index 2d8eeb15b..fc4788d10 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp @@ -39,25 +39,17 @@ namespace /// @param run_target_map Map to keep the translation between IDHash to Index /// @return A populated dependency graph with all components and run targets. void CreateDependencyGraph( - DependencyGraph& graph, + DependencyGraph& graph, configuration::Config& config, ProcessHandling process_handling, - std::unordered_map& run_target_map, std::chrono::milliseconds& off_state_transition_timeout) { - // this is a temporary (bad) implementation, all shall be cleandup - // on https://github.com/eclipse-score/lifecycle/issues/463 - // making the dep_graph a hash map would make this much cleaner as we - // wouldn't have to keep track of stuff... std::vector run_targets = config.takeRunTargets(); std::vector components = config.takeComponents(); - // map node names to their graph index, needed for deps - std::unordered_map name_to_index; - // dependencies can only be wired up once every node exists, so collect // them while creating the nodes - std::vector>> pending_dependencies; + std::vector>> pending_dependencies; pending_dependencies.reserve(graph.capacity()); // add all comps @@ -66,14 +58,10 @@ void CreateDependencyGraph( const auto name = component_config.name; auto depends_on = std::move(component_config.component_properties.depends_on); - const auto index = graph.emplace( - std::in_place_type, - std::move(component_config), - static_cast(graph.size()), - process_handling); + const auto index = graph.try_emplace( + IdentifierHash{name}, std::in_place_type, std::move(component_config), process_handling); - LM_LOG_DEBUG() << "Creating component node:" << name << "at index:" << index; - name_to_index[name] = index; + LM_LOG_DEBUG() << "Creating component node:" << name; pending_dependencies.emplace_back(index, std::move(depends_on)); } @@ -81,7 +69,8 @@ void CreateDependencyGraph( bool off_rt_defined = false; for (auto& run_target : run_targets) { - const auto index = graph.emplace(std::in_place_type, graph.size()); + const auto index = graph.try_emplace( + IdentifierHash{run_target.name}, std::in_place_type, IdentifierHash{run_target.name}); LM_LOG_DEBUG() << "Created RunTarget node:" << run_target.name << "at index" << index; if (run_target.name == Graph::off_state_name) @@ -89,36 +78,34 @@ void CreateDependencyGraph( off_rt_defined = true; off_state_transition_timeout = std::chrono::milliseconds(run_target.transition_timeout_ms); } - name_to_index[run_target.name] = index; - run_target_map.insert({IdentifierHash{run_target.name}.data(), index}); pending_dependencies.emplace_back(index, std::move(run_target.depends_on)); } // handle the off target if (!off_rt_defined) { - const auto off_index = graph.emplace(std::in_place_type, graph.size()); - run_target_map.insert({IdentifierHash{Graph::off_state_name}.data(), off_index}); + graph.try_emplace( + IdentifierHash{Graph::off_state_name}, + std::in_place_type, + IdentifierHash{Graph::off_state_name}); off_state_transition_timeout = internal::kDefaultOffStateTransitionTimeout; } // handle the fallback target - const auto fallback_index = graph.emplace(std::in_place_type, graph.size()); - run_target_map.insert({IdentifierHash{Graph::recovery_state_name}.data(), fallback_index}); - LM_LOG_DEBUG() << "fallback at index:" << fallback_index; + const auto fallback_index = graph.try_emplace( + IdentifierHash{Graph::recovery_state_name}, + std::in_place_type, + IdentifierHash{Graph::recovery_state_name}); pending_dependencies.emplace_back(fallback_index, config.fallbackRunTarget().depends_on); // wire up deps - for (const auto& [node_index, dependencies] : pending_dependencies) + for (const auto& [node_identifier, dependencies] : pending_dependencies) { for (const auto& dep_name : dependencies) { - const auto it = name_to_index.find(dep_name); - LM_LOG_DEBUG() << "Node" << node_index << "has dep to" << dep_name; - SCORE_LANGUAGE_FUTURECPP_PRECONDITION_MESSAGE( - it != name_to_index.end(), "Dependency not found in component list"); + LM_LOG_DEBUG() << "Node" << node_identifier << "has dep to" << dep_name; - graph.addDependency(node_index, it->second); + graph.addDependency(node_identifier, IdentifierHash{dep_name}); } } @@ -141,10 +128,10 @@ Graph::Graph( process_handling_(std::move(process_handling)), transition_result_receiver_(transition_result_receiver) { - last_state_manager_.process_index_ = 0xFFFFU; // an invalid state manager + last_state_manager_.process_identifier_ = IdentifierHash{""}; // an invalid state manager last_state_manager_.process_group_index_ = 0xFFFFU; cancel_message_.request_or_response_ = ControlClientCode::kNotSet; - CreateDependencyGraph(nodes_, configuration_, process_handling_, run_targets_, off_state_transition_timeout_); + CreateDependencyGraph(nodes_, configuration_, process_handling_, off_state_transition_timeout_); } Graph::~Graph() @@ -152,16 +139,6 @@ Graph::~Graph() LM_LOG_DEBUG() << "Graph destroyed"; } -int32_t Graph::getRunTargetIndex(IdentifierHash pg_state) const -{ - auto it = run_targets_.find(pg_state.data()); - if (it == run_targets_.end()) - { - return -1; - } - return static_cast(it->second); -} - bool Graph::setState(const GraphState new_state) { GraphState old_state = getState(); @@ -204,7 +181,7 @@ void Graph::updateRunTargetInPlace(RunTarget& run_target, ComponentTaskType task { run_target.deactivate(stop_source_.get_token()); } - current_transition_->onNodeFinished(run_target.getIndex()); + current_transition_->onNodeFinished(run_target.getIdentifier()); } void Graph::queueReadyNodes() @@ -262,9 +239,6 @@ void Graph::tryQueueNode(ComponentTask task) if (push_res) { jobs_in_progress_++; - // LM_LOG_DEBUG() << "Queued node " << task.component.get().getIndex() << " for " - // << (task.type == ComponentTaskType::kDeactivate ? "deactivation" : "activation") - // << " execution, jobs in progress:" << jobs_in_progress_; break; } else if (push_res.error() == ConcurrencyErrc::kTimeout) @@ -295,18 +269,16 @@ void Graph::startTransition(IdentifierHash pg_state) old_state_name = requested_state_.pg_state_name_; requested_state_.pg_state_name_ = pg_state; } - const int32_t target_node = getRunTargetIndex(requested_state_.pg_state_name_); SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE( - target_node >= 0, "RunTarget node not found for requested process group state"); + nodes_.find(pg_state) != nodes_.end(), "State name should be validated before it is passed to this method"); bool reached_transition = setState(GraphState::kInTransition); static_cast(reached_transition); // startTransition() should not be called while the graph is not in a final state SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE(reached_transition, "Setting state to kInTransition failed"); - const auto target = static_cast(target_node); - current_transition_ = &transition_builder_.createTransition(target); + current_transition_ = &transition_builder_.createTransition(pg_state); queueReadyNodes(); if (current_transition_->isFinished()) { @@ -348,15 +320,15 @@ void Graph::handleComponentEvent(const ComponentEvent& event) using T = std::decay_t; if constexpr (std::is_same_v || std::is_same_v) { - LM_LOG_DEBUG() << "Component " << data.node_index << " finished " + LM_LOG_DEBUG() << "Component " << data.node_identifier << " finished " << (std::is_same_v ? std::string_view("activation") : std::string_view("deactivation")) << " successfully"; - nodeExecuted(data.node_index, {}); + nodeExecuted(data.node_identifier, {}); } else if constexpr (std::is_same_v) { - nodeExecuted(data.node_index, score::cpp::make_unexpected(data.reason)); + nodeExecuted(data.node_identifier, score::cpp::make_unexpected(data.reason)); } else if constexpr (std::is_same_v) { @@ -365,7 +337,7 @@ void Graph::handleComponentEvent(const ComponentEvent& event) abort(1, error); // Need to clean up any leftover resources - IComponent& failingComponent = componentOf(nodes_[data.node_index]); + IComponent& failingComponent = componentOf(nodes_[data.node_identifier]); static_cast(failingComponent.deactivate({})); if (jobs_in_progress_ == 0) @@ -375,13 +347,13 @@ void Graph::handleComponentEvent(const ComponentEvent& event) } else if constexpr (std::is_same_v) { - nodeExecuted(data.node_index, {}); + nodeExecuted(data.node_identifier, {}); } }, event); } -void Graph::nodeExecuted(uint32_t node, score::cpp::expected_blank error) +void Graph::nodeExecuted(IdentifierHash node, score::cpp::expected_blank error) { bool was_last_in_queue = --jobs_in_progress_ == 0; @@ -478,7 +450,7 @@ void Graph::cancel() void Graph::forceKillProcesses() { - for (const auto& component : nodes_) + for (const auto [id, component] : nodes_) { if (const ProcessInfoNode* process = std::get_if(&component)) { @@ -510,9 +482,9 @@ void Graph::setStateManager(ControlClientID& control_client_id) last_state_manager_ = control_client_id; } -ProcessInfoNode* Graph::getProcessInfoNode(uint32_t process_index) +ProcessInfoNode* Graph::getProcessInfoNode(IdentifierHash process_index) { - if (process_index >= nodes_.size()) + if (nodes_.find(process_index) == nodes_.end()) { return nullptr; } @@ -538,17 +510,17 @@ IdentifierHash Graph::getProcessGroupState() const ProcessInfoNode* Graph::findControlClient() { - auto* pin = getProcessInfoNode(getStateManager().process_index_); + auto* pin = getProcessInfoNode(getStateManager().process_identifier_); if (pin && pin->getControlClientChannel()) { return pin; } - for (std::size_t i = 0; i < nodes_.size(); ++i) + for (const auto [id, node] : nodes_) { - if (const auto* node = std::get_if(&nodes_[i]); node && node->getControlClientChannel()) + if (const auto* process = std::get_if(&node); process && process->getControlClientChannel()) { - return node; + return process; } } diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp index 4cb0ec443..80e7580cc 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp @@ -181,8 +181,8 @@ class Graph final /// @brief Applies a ComponentEvent — produced by ProcessMonitor from worker/OS-handler thread /// callbacks and drained on the main thread — to this graph. /// @details Dispatches on the event's variant: - /// - ActivationSuccessful / DeactivationComplete: `nodeExecuted(node_index, {})` - /// - ActivationFailed: `nodeExecuted(node_index, make_unexpected(reason))` + /// - ActivationSuccessful / DeactivationComplete: `nodeExecuted(node_identifier, {})` + /// - ActivationFailed: `nodeExecuted(node_identifier, make_unexpected(reason))` /// - UnexpectedTermination: `abort(1, kErrorAfterReady)` — ProcessMonitor::terminated() only /// pushes this event once a process has already reached its ready condition, so it is /// always a post-ready crash. @@ -220,7 +220,7 @@ class Graph final /// @param process_index Index of the process node to retrieve. /// @return The ProcessInfoNode at the given index, or nullptr if out of bounds or if the node /// at that index is a RunTarget rather than a ProcessInfoNode. - ProcessInfoNode* getProcessInfoNode(uint32_t process_index); + ProcessInfoNode* getProcessInfoNode(IdentifierHash process_index); /// @return The identifier of the process group managed by this graph. IdentifierHash getProcessGroupName(); @@ -294,7 +294,7 @@ class Graph final private: /// @brief Reports that a node has finished executing, enqueuing successors or updating the graph state if a /// transition has finished. - void nodeExecuted(uint32_t node, score::cpp::expected_blank error); + void nodeExecuted(IdentifierHash node, score::cpp::expected_blank error); /// @brief Abort the current transition due to a process error. /// @deprecated @param code The execution error for the process that caused the abort. @@ -306,9 +306,6 @@ class Graph final /// @returns False if the requested state was not set bool setState(GraphState new_state); - /// @return The index of the RunTarget node for @p pg_state, or -1 if not found. - int32_t getRunTargetIndex(IdentifierHash pg_state) const; - /// @brief Pushes the given task onto the worker queue while the graph is in transition. /// Retries on timeout. /// @param task The task to enqueue. @@ -338,15 +335,13 @@ class Graph final /// @brief Nodes for all unique processes in this process group, plus a virtual RunTarget node /// per configured ProcessGroupState. - DependencyGraph nodes_; - - std::unordered_map run_targets_{}; + DependencyGraph nodes_; /// @brief Builder for creating the transition object for the current state transition. - TransitionBuilder transition_builder_; + TransitionBuilder transition_builder_; /// @brief The currently active transition or nullptr before the first one starts. - Transition* current_transition_{nullptr}; + Transition* current_transition_{nullptr}; /// @brief Current state of the graph. GraphState state_{GraphState::kSuccess}; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp index c61c6d35c..3843b68eb 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/graph_UT.cpp @@ -167,11 +167,11 @@ class GraphTest : public ::testing::Test executeJobSuccessfully(job->value()); if (job->value().type == ComponentTaskType::kActivate) { - graph_->handleComponentEvent(ActivationSuccessful{job->value().component.get().getIndex()}); + graph_->handleComponentEvent(ActivationSuccessful{job->value().component.get().getIdentifier()}); } else { - graph_->handleComponentEvent(DeactivationComplete{job->value().component.get().getIndex()}); + graph_->handleComponentEvent(DeactivationComplete{job->value().component.get().getIdentifier()}); } } @@ -228,7 +228,7 @@ TEST_F(GraphOrdinaryTransitionTest, correctJobDetails) const auto job = job_queue_->pop(); ASSERT_TRUE(job->has_value()) << "startTransition didn't push anything to the queue"; EXPECT_EQ(job->value().type, ComponentTaskType::kActivate); - EXPECT_EQ(job->value().component.get().getIndex(), 1); + EXPECT_EQ(job->value().component.get().getIdentifier(), IdentifierHash{process_name(1)}); } TEST_F(GraphOrdinaryTransitionTest, simpleActivationTransition) @@ -242,7 +242,7 @@ TEST_F(GraphOrdinaryTransitionTest, simpleActivationTransition) const auto job = job_queue_->pop(); executeJobSuccessfully(job->value()); - graph_->handleComponentEvent(ActivationSuccessful{0}); + graph_->handleComponentEvent(ActivationSuccessful{IdentifierHash{process_name(0)}}); ASSERT_EQ(graph_->getState(), GraphState::kSuccess); EXPECT_EQ(graph_->getProcessGroupState(), target); @@ -260,7 +260,7 @@ TEST_F(GraphOrdinaryTransitionTest, simpleDeactivationTransition) const auto job = job_queue_->pop(); executeJobSuccessfully(job->value()); - graph_->handleComponentEvent(DeactivationComplete{0}); + graph_->handleComponentEvent(DeactivationComplete{IdentifierHash{process_name(0)}}); ASSERT_EQ(graph_->getState(), GraphState::kSuccess); EXPECT_EQ(graph_->getProcessGroupState(), target); @@ -295,7 +295,8 @@ TEST_F(GraphInitialTransitionTest, jobFailure) const auto job = job_queue_->pop()->value(); failActivationJob(job); - graph_->handleComponentEvent(ActivationFailed{0, IComponent::ComponentError::kErrorBeforeReady}); + graph_->handleComponentEvent( + ActivationFailed{IdentifierHash{process_name(0)}, IComponent::ComponentError::kErrorBeforeReady}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState); } @@ -315,7 +316,7 @@ TEST_F(GraphInitialTransitionTest, cancel) const auto job = job_queue_->pop()->value(); executeJobSuccessfully(job); - graph_->handleComponentEvent(ActivationSuccessful{0}); + graph_->handleComponentEvent(ActivationSuccessful{IdentifierHash{process_name(0)}}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState); } @@ -341,7 +342,7 @@ TEST_F(GraphOffTransitionTest, normalShutdown) EXPECT_TRUE(graph_->isTransitioningToOff()); ASSERT_TRUE(job->has_value()); EXPECT_EQ(job.value()->type, ComponentTaskType::kDeactivate); - EXPECT_EQ(job->value().component.get().getIndex(), 0); + EXPECT_EQ(job->value().component.get().getIdentifier(), IdentifierHash{process_name(0)}); } TEST_F(GraphOffTransitionTest, shutdownDuringTransition) @@ -405,7 +406,7 @@ TEST_F(GraphImplicitOffTargetTest, offRunTargetIsCreatedWhenNotConfigured) ASSERT_TRUE(job->has_value()); EXPECT_EQ(job->value().type, ComponentTaskType::kDeactivate); executeJobSuccessfully(job->value()); - graph_->handleComponentEvent(DeactivationComplete{job->value().component.get().getIndex()}); + graph_->handleComponentEvent(DeactivationComplete{job->value().component.get().getIdentifier()}); EXPECT_EQ(graph_->getState(), GraphState::kSuccess); EXPECT_EQ(graph_->getProcessGroupState(), IdentifierHash{"Off"}); @@ -480,7 +481,8 @@ TEST_F(GraphHandleComponentEventTest, failedFirstDuringTransition) // Fail the first job const auto first_job = job_queue_->pop(); graph_->handleComponentEvent( - ActivationFailed{first_job->value().component.get().getIndex(), IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{ + first_job->value().component.get().getIdentifier(), IComponent::ComponentError::kErrorBeforeReady}); const auto second_job = job_queue_->pop(); @@ -498,11 +500,12 @@ TEST_F(GraphHandleComponentEventTest, failureFollowedBySuccessFails) // Fail the first job const auto first_job = job_queue_->pop(); graph_->handleComponentEvent( - ActivationFailed{first_job->value().component.get().getIndex(), IComponent::ComponentError::kErrorBeforeReady}); + ActivationFailed{ + first_job->value().component.get().getIdentifier(), IComponent::ComponentError::kErrorBeforeReady}); const auto second_job = job_queue_->pop(); executeJobSuccessfully(second_job->value()); - graph_->handleComponentEvent(ActivationSuccessful{second_job->value().component.get().getIndex()}); + graph_->handleComponentEvent(ActivationSuccessful{second_job->value().component.get().getIdentifier()}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState); EXPECT_EQ(graph_->getPendingEvent(), ControlClientCode::kFailedUnexpectedTerminationOnEnter); @@ -517,7 +520,7 @@ TEST_F(GraphHandleComponentEventTest, unexpectedTerminationDuringSuccess) completeTransition(IdentifierHash{run_target_name(0)}); - const auto component = graph_->getProcessInfoNode(0); + const auto component = graph_->getProcessInfoNode(IdentifierHash{process_name(0)}); EXPECT_CALL(process_interface_, requestTermination) .WillOnce(DoAll( InvokeWithoutArgs([component] { @@ -525,7 +528,7 @@ TEST_F(GraphHandleComponentEventTest, unexpectedTerminationDuringSuccess) }), Return(osal::OsalReturnType::kSuccess))); - graph_->handleComponentEvent(UnexpectedTermination{0}); + graph_->handleComponentEvent(UnexpectedTermination{component->getIdentifier()}); EXPECT_EQ(graph_->getState(), GraphState::kUndefinedState); } @@ -541,7 +544,7 @@ TEST_F(GraphHandleComponentEventTest, unexpectedTerminationDuringTransition) const auto first_job = job_queue_->pop(); executeJobSuccessfully(first_job->value()); - const auto component_index = first_job.value()->component.get().getIndex(); + const auto component_index = first_job.value()->component.get().getIdentifier(); graph_->handleComponentEvent(ActivationSuccessful{component_index}); const auto component = graph_->getProcessInfoNode(component_index); @@ -557,7 +560,7 @@ TEST_F(GraphHandleComponentEventTest, unexpectedTerminationDuringTransition) const auto second_job = job_queue_->pop(); executeJobSuccessfully(second_job->value()); - graph_->handleComponentEvent(ActivationSuccessful{second_job->value().component.get().getIndex()}); + graph_->handleComponentEvent(ActivationSuccessful{second_job->value().component.get().getIdentifier()}); EXPECT_EQ(graph_->getPendingEvent(), ControlClientCode::kFailedUnexpectedTermination); } @@ -586,7 +589,7 @@ TEST_F(GraphCancelTest, cancelsOngoingTransition) const auto job = job_queue_->pop(); - graph_->handleComponentEvent(JobSkipped{0}); + graph_->handleComponentEvent(JobSkipped{IdentifierHash{process_name(0)}}); EXPECT_TRUE(job->value().stop_token.stop_requested()); EXPECT_EQ(graph_->getPendingEvent(), ControlClientCode::kSetStateCancelled); @@ -602,9 +605,9 @@ TEST_F(GraphUtilitiesTest, getProcessInfoNode) RecordProperty( "Description", "Test that getProcessInfoNode returns process info node pointer or null pointer when expected"); - const auto* pin = graph_->getProcessInfoNode(0); - const auto* oob = graph_->getProcessInfoNode(100); - const auto* rt = graph_->getProcessInfoNode(1); + const auto* pin = graph_->getProcessInfoNode(IdentifierHash{process_name(0)}); + const auto* oob = graph_->getProcessInfoNode(IdentifierHash{"Not real"}); + const auto* rt = graph_->getProcessInfoNode(IdentifierHash{run_target_name(0)}); EXPECT_NE(pin, nullptr); EXPECT_EQ(oob, nullptr); @@ -654,9 +657,9 @@ TEST_F(GraphUtilitiesTest, gettersSetters) RecordProperty("Description", "Test that basic getters return the value the setter sets"); ControlClientID state_manager = {}; - state_manager.process_index_ = 123; + state_manager.process_identifier_ = IdentifierHash{"123"}; graph_->setStateManager(state_manager); - EXPECT_EQ(graph_->getStateManager().process_index_, state_manager.process_index_); + EXPECT_EQ(graph_->getStateManager().process_identifier_, state_manager.process_identifier_); const IdentifierHash pending_state{"Pending"}; const auto previous_pending_state = graph_->getPendingState(); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp index 13172dcdf..633599b2e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/icomponent.hpp @@ -14,6 +14,7 @@ #ifndef SCORE_LCM_ICOMPONENT_HPP_INCLUDED #define SCORE_LCM_ICOMPONENT_HPP_INCLUDED +#include "score/mw/launch_manager/common/identifier_hash.hpp" #include #include @@ -68,7 +69,7 @@ class IComponent [[nodiscard]] virtual RequestResult tryHandleTermination(int32_t status) = 0; /// @returns the index of the component in the graph. - [[nodiscard]] virtual uint32_t getIndex() const = 0; + [[nodiscard]] virtual IdentifierHash getIdentifier() const = 0; /// @returns True if the component is active in the active run target. [[nodiscard]] virtual bool active() const = 0; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp index 7a555531f..3c0413884 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/mock_component.hpp @@ -25,7 +25,7 @@ class MockComponent : public IComponent MOCK_METHOD(RequestResult, activate, (score::cpp::stop_token stop_token), (override)); MOCK_METHOD(RequestResult, deactivate, (score::cpp::stop_token stop_token), (override)); MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t status), (override)); - MOCK_METHOD(uint32_t, getIndex, (), (override, const)); + MOCK_METHOD(IdentifierHash, getIdentifier, (), (override, const)); MOCK_METHOD(bool, active, (), (override, const)); }; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp index 754e9b1bf..8822d9f54 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.cpp @@ -24,17 +24,14 @@ namespace score::mw::lifecycle::internal { -ProcessInfoNode::ProcessInfoNode( - configuration::ComponentConfig&& config, - uint32_t index, - ProcessHandling process_handling) +ProcessInfoNode::ProcessInfoNode(configuration::ComponentConfig&& config, ProcessHandling process_handling) : terminator_(), has_semaphore_(false), - process_index_(index), pid_(0), status_(0), config_(std::move(config)), - process_handling_(std::move(process_handling)) + process_handling_(std::move(process_handling)), + identifier_(IdentifierHash{{config_.name}}) { if (config.component_properties.application_profile.application_type == @@ -93,7 +90,7 @@ IComponent::RequestResult ProcessInfoNode::tryReportSuccess() if (auto time = getTimeForReport()) { - process_handling_.state_publisher_.reportActivation(IdentifierHash{config_.name}, time.value()); + process_handling_.state_publisher_.reportActivation(identifier_, time.value()); } return {RequestState::kSuccess}; @@ -160,8 +157,7 @@ void ProcessInfoNode::unblockSync() IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_status) { - LM_LOG_DEBUG() << "Process" << process_index_ << "pid" << pid_ << "(" << config_.name << ") for node" << this - << "terminated with status" << process_status; + LM_LOG_DEBUG() << "Process" << identifier_ << "( pid" << pid_ << ") terminated with status" << process_status; status_ = process_status; IComponent::RequestResult res = {IComponent::RequestState::kWaiting}; if (has_semaphore_.exchange(false)) @@ -188,8 +184,8 @@ IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_ } else { - LM_LOG_WARN() << "unexpected termination of process" << process_index_ << "pid" << pid_ << "(" - << config_.name << ")" << "( status" << status_ << ")"; + LM_LOG_WARN() << "unexpected termination of process" << identifier_ << "( pid" << pid_ << "status" + << status_ << ")"; res = score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady); } } @@ -205,8 +201,8 @@ IComponent::RequestResult ProcessInfoNode::tryHandleTermination(int32_t process_ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token stop_token) { - LM_LOG_DEBUG() << "Starting process" << process_index_ << "(" << config_.name << ") from executable" - << config_.deployment_config.bin_dir << "/" << config_.component_properties.binary_name; + LM_LOG_DEBUG() << "Starting process (" << identifier_ << ") from executable" << config_.deployment_config.bin_dir + << "/" << config_.component_properties.binary_name; std::optional error; for (std::uint8_t attempts = start_tries_; attempts != 0U; attempts--) @@ -232,7 +228,7 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s if (osal::OsalReturnType::kSuccess == process_handling_.process_interface_->startProcess(pid_, sync_, config_)) { - LM_LOG_DEBUG() << "startProcess pid" << pid_ << "received for process:" << config_.name; + LM_LOG_DEBUG() << "startProcess pid" << pid_ << "received for process:" << identifier_; if (configuration::ApplicationType::StateManager == config_.component_properties.application_profile.application_type) @@ -266,7 +262,7 @@ IComponent::RequestResult ProcessInfoNode::startProcess(score::cpp::stop_token s sync_.reset(); } - LM_LOG_DEBUG() << "startProcess for process" << process_index_ << "(" << config_.name << ") done"; + LM_LOG_DEBUG() << "startProcess for process (" << identifier_ << ") done"; if (error.has_value()) { @@ -304,7 +300,7 @@ score::cpp::expected_blank ProcessInfoNode::handlePr return score::cpp::make_unexpected(ComponentError::kErrorBeforeReady); } - LM_LOG_WARN() << "Got kRunning timeout for process" << process_index_ << "(" << config_.name << ")"; + LM_LOG_WARN() << "Got kRunning timeout for process (" << identifier_ << ")"; terminateProcess(stop_token); return score::cpp::make_unexpected(ComponentError::kActivationTimedOut); } @@ -316,8 +312,7 @@ score::cpp::expected_blank ProcessInfoNode::handlePr { // Error. To get a legal terminated before kRunning the process must be self-terminating, non-reporting // and to have exited with zero status - LM_LOG_WARN() << "Got process termination before kRunning for pid" << pid_ << "(" << config_.name << ") process" - << process_index_; + LM_LOG_WARN() << "Got process termination before kRunning for pid" << pid_ << "(" << identifier_ << ")"; // This will cause the graph to fail unless we have restart attempts left return score::cpp::make_unexpected(ComponentError::kErrorBeforeReady); } @@ -351,40 +346,37 @@ void ProcessInfoNode::handleProcessRunning() { if (configuration::ApplicationType::Native == config_.component_properties.application_profile.application_type) { - LM_LOG_DEBUG() << "Considered kRunning for Non Reporting Process pid" << pid_ << "(" << config_.name - << ") process" << process_index_; + LM_LOG_DEBUG() << "Considered kRunning for Non Reporting Process pid" << pid_ << "(" << identifier_ << ")"; } else { - LM_LOG_DEBUG() << "Got kRunning for pid" << pid_ << "(" << config_.name << ") process" << process_index_; + LM_LOG_DEBUG() << "Got kRunning for pid" << pid_ << "(" << identifier_ << ")"; } } void ProcessInfoNode::terminateProcess(const score::cpp::stop_token& stop_token) { - LM_LOG_DEBUG() << "terminating process" << process_index_ << "(" << config_.name << ")"; + LM_LOG_DEBUG() << "terminating process (" << identifier_ << ")"; if (setState(score::mw::lifecycle::ProcessState::kTerminating)) { handleTerminationProcess(stop_token); } - LM_LOG_DEBUG() << "terminateProcess for process" << process_index_ << "(" << config_.name << ") done"; + LM_LOG_DEBUG() << "terminateProcess for process (" << identifier_ << ") done"; } void ProcessInfoNode::handleTerminationProcess(const score::cpp::stop_token& stop_token) { static_cast(terminator_.init(0U, false)); has_semaphore_.store(true); - LM_LOG_DEBUG() << "Requesting termination of process" << process_index_ << "pid" << pid_ << "(" << config_.name - << ")"; + LM_LOG_DEBUG() << "Requesting termination of process pid" << pid_ << "(" << identifier_ << ")"; // handle request termination if ((process_handling_.process_interface_->requestTermination(pid_) == osal::OsalReturnType::kFail) || (terminator_.timedWait(std::chrono::milliseconds(config_.deployment_config.shutdown_timeout_ms)) == osal::OsalReturnType::kSuccess)) { - LM_LOG_DEBUG() << "Queuing jobs after regular termination of process wait" << process_index_ << "(" - << config_.name << ")"; + LM_LOG_DEBUG() << "Queuing jobs after regular termination of process (" << identifier_ << ")"; } else { @@ -400,13 +392,12 @@ void ProcessInfoNode::handleForcedTermination(const score::cpp::stop_token& stop { static_cast(stop_token); // Not yet supported - LM_LOG_WARN() << "Process" << process_index_ << "(" << config_.name - << ") did not respond to SIGTERM, sending SIGKILL"; + LM_LOG_WARN() << "Process (" << identifier_ << ") did not respond to SIGTERM, sending SIGKILL"; while ((osal::OsalReturnType::kSuccess == process_handling_.process_interface_->forceTermination(pid_)) && (terminator_.timedWait(score::mw::lifecycle::internal::kMaxSigKillDelay) != osal::OsalReturnType::kSuccess)) { - LM_LOG_FATAL() << "Process" << process_index_ << "(" << config_.name << ") did not respond to SIGKILL!!"; + LM_LOG_FATAL() << "Process (" << identifier_ << ") did not respond to SIGKILL!!"; } } @@ -428,7 +419,7 @@ IComponent::RequestResult ProcessInfoNode::deactivate(score::cpp::stop_token sto reached_ready_.store(false); if (auto time = getTimeForReport()) { - process_handling_.state_publisher_.reportDeactivation(IdentifierHash{config_.name}, time.value()); + process_handling_.state_publisher_.reportDeactivation(identifier_, time.value()); } terminateProcess(stop_token); setState(ProcessState::kIdle); @@ -455,9 +446,9 @@ std::chrono::milliseconds ProcessInfoNode::getTerminationTimeout() const return std::chrono::milliseconds{config_.deployment_config.shutdown_timeout_ms}; } -uint32_t ProcessInfoNode::getIndex() const +IdentifierHash ProcessInfoNode::getIdentifier() const { - return process_index_; + return identifier_; } ControlClientChannelP ProcessInfoNode::getControlClientChannel() const diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp index 49fc16671..78c6b25b9 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node.hpp @@ -46,13 +46,12 @@ class ProcessInfoNode final : public IComponent /// @param index The process index within its process group. /// @param ready_condition Whether this process is considered ready when running or when terminated. /// @param process_handling The interfaces used to start, stop and report on the OS process. - ProcessInfoNode(configuration::ComponentConfig&& config, uint32_t index, ProcessHandling process_handling); + ProcessInfoNode(configuration::ComponentConfig&& config, ProcessHandling process_handling); /// @brief Explicit move constructor required due to atomics. PIN must be moveable to exist in the graph ProcessInfoNode(ProcessInfoNode&& other) noexcept : terminator_(), has_semaphore_(other.has_semaphore_.load()), - process_index_(other.process_index_), pid_(other.pid_), status_(other.status_.load()), process_state_(other.process_state_.load()), @@ -60,7 +59,8 @@ class ProcessInfoNode final : public IComponent config_(std::move(other.config_)), control_client_channel_(std::move(other.control_client_channel_)), sync_(std::move(other.sync_)), - process_handling_(std::move(other.process_handling_)) + process_handling_(std::move(other.process_handling_)), + identifier_(other.identifier_) { } @@ -69,7 +69,7 @@ class ProcessInfoNode final : public IComponent ProcessInfoNode& operator=(ProcessInfoNode&& other) = delete; ~ProcessInfoNode() = default; - [[nodiscard]] uint32_t getIndex() const override; + [[nodiscard]] IdentifierHash getIdentifier() const override; RequestResult activate(score::cpp::stop_token stop_token) override; @@ -159,9 +159,6 @@ class ProcessInfoNode final : public IComponent /// @brief True if semaphore is being used std::atomic_bool has_semaphore_{false}; - /// @brief index of this node (process) in the graph (process group) - uint32_t process_index_ = 0; - /// @brief The process id reported by the operating system when the process was started osal::ProcessID pid_ = 0; @@ -192,6 +189,9 @@ class ProcessInfoNode final : public IComponent /// @brief Number ot times to try run the process. std::uint8_t start_tries_{1U}; + + /// @brief Unique hash to identify this node. + IdentifierHash identifier_; }; } // namespace score::mw::lifecycle::internal diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp index 6c176d40a..c8f3c8316 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_info_node_UT.cpp @@ -27,8 +27,9 @@ using namespace testing; using namespace score::mw::lifecycle::internal; using namespace score::mw::lifecycle; -// Default ProcessIndex for testing -constexpr uint32_t kProcessIndex = 111; +// Default process name for testing +constexpr std::string_view kProcessName{"test_process"}; +const IdentifierHash kProcessNameHash{kProcessName}; class MockSafeProcessMapInserter : public SafeProcessMapInserter { @@ -57,8 +58,8 @@ class ProcessInfoNodeFixture : public ::testing::Test configuration::ProcessState ready_state = configuration::ProcessState::Running) { configuration::ComponentConfig config{}; - config.name = "test_process"; - config.component_properties.binary_name = "test_process"; + config.name = kProcessName; + config.component_properties.binary_name = kProcessName; auto& profile = config.component_properties.application_profile; profile.application_type = application_type; @@ -68,7 +69,7 @@ class ProcessInfoNodeFixture : public ::testing::Test config.deployment_config.shutdown_timeout_ms = shutdown_timeout_ms_; return std::make_unique( - std::move(config), kProcessIndex, ProcessHandling{mock_publisher_, &mock_processIf_, process_map_}); + std::move(config), ProcessHandling{mock_publisher_, &mock_processIf_, process_map_}); } /// @brief Helper method to create a ProcessInfoNode that is self-terminating. @@ -139,7 +140,7 @@ TEST_F(ProcessInfoNodeStartupTest, CanConstructIdleProcessInfoNode) auto node = createProcessInfoNode(); - ASSERT_THAT(node->getIndex(), Eq(kProcessIndex)); + ASSERT_THAT(node->getIdentifier(), Eq(kProcessName)); ASSERT_THAT(node->getState(), Eq(score::mw::lifecycle::ProcessState::kIdle)); ASSERT_THAT(node->getPid(), Eq(0)); ASSERT_THAT(node->active(), IsFalse()); @@ -545,7 +546,7 @@ TEST_F(ProcessInfoNodeMoveTest, MoveConstruct_IdleNode_PreservesObservableState) ProcessInfoNode moved{std::move(*source)}; - ASSERT_THAT(moved.getIndex(), Eq(kProcessIndex)); + ASSERT_THAT(moved.getIdentifier(), Eq(kProcessName)); ASSERT_THAT(moved.getState(), Eq(score::mw::lifecycle::ProcessState::kIdle)); ASSERT_THAT(moved.active(), IsFalse()); ASSERT_THAT(moved.getPid(), Eq(0)); @@ -565,7 +566,7 @@ TEST_F(ProcessInfoNodeMoveTest, MoveConstruct_RunningNode_PreservesAtomicState) ProcessInfoNode moved{std::move(*source)}; - ASSERT_THAT(moved.getIndex(), Eq(kProcessIndex)); + ASSERT_THAT(moved.getIdentifier(), Eq(kProcessName)); ASSERT_THAT(moved.getState(), Eq(score::mw::lifecycle::ProcessState::kRunning)); ASSERT_THAT(moved.active(), IsTrue()); } diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp index 653d17422..1b7401b18 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor.cpp @@ -43,16 +43,16 @@ void ProcessMonitor::doWork(ComponentTask&& task) }; auto handle_success = [&]() { - const uint32_t node_index = task.component.get().getIndex(); + const IdentifierHash node_identifier = task.component.get().getIdentifier(); bool push_res = true; switch (task.type) { case ComponentTaskType::kActivate: - push_res = event_queue_.push(ActivationSuccessful{node_index}); + push_res = event_queue_.push(ActivationSuccessful{node_identifier}); break; case ComponentTaskType::kDeactivate: - push_res = event_queue_.push(DeactivationComplete{node_index}); + push_res = event_queue_.push(DeactivationComplete{node_identifier}); break; } @@ -63,12 +63,12 @@ void ProcessMonitor::doWork(ComponentTask&& task) }; auto handle_failure = [&](IComponent::ComponentError& error) { - const uint32_t node_index = task.component.get().getIndex(); + const IdentifierHash node_identifier = task.component.get().getIdentifier(); switch (task.type) { case ComponentTaskType::kActivate: - if (!event_queue_.push(ActivationFailed{node_index, error})) + if (!event_queue_.push(ActivationFailed{node_identifier, error})) { LM_LOG_ERROR() << "Failed to send activation failed event to event queue!"; } @@ -91,7 +91,7 @@ void ProcessMonitor::doWork(ComponentTask&& task) if (task.stop_token.stop_requested()) { - if (!event_queue_.push(JobSkipped{task.component.get().getIndex()})) + if (!event_queue_.push(JobSkipped{task.component.get().getIdentifier()})) { LM_LOG_ERROR() << "Failed to send job skipped event to event queue!"; } @@ -116,11 +116,11 @@ void ProcessMonitor::terminated(IComponent& component, int32_t status) bool push_res = true; if (!res.has_value()) { - push_res = event_queue_.push(UnexpectedTermination{component.getIndex()}); + push_res = event_queue_.push(UnexpectedTermination{component.getIdentifier()}); } else if (res.value() != IComponent::RequestState::kWaiting) { - push_res = event_queue_.push(ActivationSuccessful{component.getIndex()}); + push_res = event_queue_.push(ActivationSuccessful{component.getIdentifier()}); } if (!push_res) diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp index 8bba2f979..b4cde272c 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/process_monitor_UT.cpp @@ -20,6 +20,8 @@ using namespace testing; using namespace score::mw::lifecycle::internal; +const score::mw::lifecycle::IdentifierHash kDefaultIdentifier{"Process"}; + class ProcessMonitorTest : public ::testing::Test { protected: @@ -28,7 +30,7 @@ class ProcessMonitorTest : public ::testing::Test RecordProperty("TestType", "interface-test"); RecordProperty("DerivationTechnique", "explorative-testing"); - ON_CALL(mock_component, getIndex).WillByDefault(Return(1)); + ON_CALL(mock_component, getIdentifier).WillByDefault(Return(kDefaultIdentifier)); } MockComponentEventQueue mock_queue{}; @@ -43,7 +45,9 @@ TEST_F(ProcessMonitorTest, DoWorkNormalActivation) // Given a valid activate task // Then EXPECT_CALL(mock_component, activate).WillOnce(Return(IComponent::RequestState::kSuccess)); - EXPECT_CALL(mock_queue, push(VariantWith(Field(&ActivationSuccessful::node_index, 1)))) + EXPECT_CALL( + mock_queue, + push(VariantWith(Field(&ActivationSuccessful::node_identifier, kDefaultIdentifier)))) .Times(1); // When process_monitor.doWork(ComponentTask{ComponentTaskType::kActivate, mock_component, stop_source.get_token()}); @@ -55,7 +59,9 @@ TEST_F(ProcessMonitorTest, DoWorkNormalDeactivation) // Given a valid activate task // Then EXPECT_CALL(mock_component, deactivate).WillOnce(Return(IComponent::RequestState::kSuccess)); - EXPECT_CALL(mock_queue, push(VariantWith(Field(&DeactivationComplete::node_index, 1)))) + EXPECT_CALL( + mock_queue, + push(VariantWith(Field(&DeactivationComplete::node_identifier, kDefaultIdentifier)))) .Times(1); // When process_monitor.doWork(ComponentTask{ComponentTaskType::kDeactivate, mock_component, stop_source.get_token()}); @@ -70,7 +76,9 @@ TEST_F(ProcessMonitorTest, DoWorkOnTerminationDepProcess) // The component is neither complete nor failed EXPECT_CALL(mock_component, activate).WillOnce(Return(IComponent::RequestState::kWaiting)); EXPECT_CALL(mock_component, tryHandleTermination).WillOnce(Return(IComponent::RequestState::kSuccess)); - EXPECT_CALL(mock_queue, push(VariantWith(Field(&ActivationSuccessful::node_index, 1)))) + EXPECT_CALL( + mock_queue, + push(VariantWith(Field(&ActivationSuccessful::node_identifier, kDefaultIdentifier)))) .Times(1); // When process_monitor.doWork(ComponentTask{ComponentTaskType::kActivate, mock_component, stop_source.get_token()}); @@ -88,7 +96,9 @@ TEST_F(ProcessMonitorTest, TerminatedUnexpectedly) // Then EXPECT_CALL(mock_component, tryHandleTermination) .WillOnce(Return(score::cpp::make_unexpected(IComponent::ComponentError::kErrorAfterReady))); - EXPECT_CALL(mock_queue, push(VariantWith(Field(&UnexpectedTermination::node_index, 1)))) + EXPECT_CALL( + mock_queue, + push(VariantWith(Field(&UnexpectedTermination::node_identifier, kDefaultIdentifier)))) .Times(1); process_monitor.terminated(mock_component, 0); diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp index 71db95d9f..986b5e0c9 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/run_target.hpp @@ -27,11 +27,11 @@ namespace score::mw::lifecycle::internal class RunTarget final : public IComponent { public: - explicit RunTarget(uint32_t index) : index_(index) + explicit RunTarget(const IdentifierHash& index) : identifier_(index) { } - RunTarget(RunTarget&& other) noexcept : index_(other.index_), active_(other.active_.load()) + RunTarget(RunTarget&& other) noexcept : identifier_(other.identifier_), active_(other.active_.load()) { } RunTarget(const RunTarget&) = delete; @@ -56,9 +56,9 @@ class RunTarget final : public IComponent return RequestState::kSuccess; } - uint32_t getIndex() const override + IdentifierHash getIdentifier() const override { - return index_; + return identifier_; } bool active() const override @@ -67,7 +67,9 @@ class RunTarget final : public IComponent } private: - uint32_t index_; + /// @brief Unique identifier of this run target. + IdentifierHash identifier_; + /// @brief True if the run target has been activated and has not yet been deactivated. std::atomic active_{false}; }; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp index a63cec7ce..cace64d1e 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/safeprocessmap_UT.cpp @@ -50,7 +50,7 @@ class MockComponent : public IComponent MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t status), (override)); MOCK_METHOD(bool, active, (), (const override)); MOCK_METHOD(bool, stopped, (), (const override)); - MOCK_METHOD(uint32_t, getIndex, (), (const override)); + MOCK_METHOD(score::mw::lifecycle::IdentifierHash, getIdentifier, (), (const override)); }; class SafeProcessMapTest : public ::testing::Test diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/transition.hpp b/score/launch_manager/src/daemon/src/process_group_manager/details/transition.hpp index ba0b960d1..7aae3f828 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/transition.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/transition.hpp @@ -15,6 +15,7 @@ #include "score/mw/launch_manager/common/concurrency/fixed_size_queue.hpp" #include "score/mw/launch_manager/common/constants.hpp" +#include "score/mw/launch_manager/process_group_manager/details/component_of.hpp" #include "score/mw/launch_manager/process_group_manager/details/dependency_graph.hpp" #include "score/mw/launch_manager/process_group_manager/details/icomponent.hpp" @@ -41,22 +42,25 @@ enum class Action : std::uint8_t }; /// @brief Contains a node that is ready to be activated/deactivated +template struct ReadyNode { - GraphIndex node; + Key node; Action action; }; -inline bool operator==(const ReadyNode& lhs, const ReadyNode& rhs) +template +inline bool operator==(const ReadyNode& lhs, const ReadyNode& rhs) { return lhs.node == rhs.node && lhs.action == rhs.action; } -inline bool operator!=(const ReadyNode& lhs, const ReadyNode& rhs) +template +inline bool operator!=(const ReadyNode& lhs, const ReadyNode& rhs) { return !(lhs == rhs); } -template +template class TransitionBuilder; namespace detail @@ -82,7 +86,7 @@ struct is_component_type()) /// need to be activated (those reachable from the target that are /// not yet active). /// -template +template class Transition { // The transition is split into two phases: @@ -102,7 +106,11 @@ class Transition "Transition requires an ADL-findable componentOf(T&) that returns a reference " "to IComponent&."); - friend class TransitionBuilder; + static_assert( + std::is_trivially_copyable_v, + "This class takes copies of keys so they should be trivially copyable"); + + friend class TransitionBuilder; public: /// @brief Pop the next ready node, or std::nullopt if none is ready right now @@ -110,13 +118,13 @@ class Transition /// is gone from the frontier the moment it's returned. Safe to interleave /// with onNodeFinished() — nodes onNodeFinished() appends are queued behind /// whatever's already pending, never lost, regardless of consumption order. - std::optional nextReady() + std::optional> nextReady() { if (state_.next_nodes.empty()) { return std::nullopt; } - return ReadyNode{state_.next_nodes.tryPop().value(), currentAction()}; + return ReadyNode{state_.next_nodes.tryPop().value(), currentAction()}; } /// @brief Input iterator that drains the transition via nextReady(). @@ -127,8 +135,8 @@ class Transition class Iterator { public: - using value_type = ReadyNode; - using reference = ReadyNode; + using value_type = ReadyNode; + using reference = ReadyNode; using difference_type = std::ptrdiff_t; using iterator_category = std::input_iterator_tag; using pointer = void; @@ -139,7 +147,7 @@ class Transition advance(); } - ReadyNode operator*() const + ReadyNode operator*() const { return *current_; } @@ -164,7 +172,7 @@ class Transition } Transition* owner_ = nullptr; - std::optional current_; + std::optional> current_; }; Iterator begin() @@ -181,7 +189,7 @@ class Transition /// activation: it is part of the target subgraph, and every dependency it /// has is active() /// deactivation: every dependent it has is stopped() - void onNodeFinished(GraphIndex node) + void onNodeFinished(Key node) { SCORE_LANGUAGE_FUTURECPP_ASSERT(isValidNode(node)); @@ -204,11 +212,11 @@ class Transition // phase's direction, filtered by readiness, behind whatever's already // waiting to be dispatched. const auto& successors = state_.phase == Phase::Starting ? graph_.dependents(node) : graph_.dependsOn(node); - for (const GraphIndex s : successors) + for (const Key s : successors) { - if (isReady(s) && !state_.enqueued_set.test(s)) + if (isReady(s) && !state_.node_information.at(s).enqueued_) { - state_.enqueued_set.set(s); + state_.node_information[s].enqueued_ = true; SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( state_.next_nodes.push(s), "Transition queue should never exceed capacity"); } @@ -224,33 +232,38 @@ class Transition private: /// @brief True iff @p node is a valid index into the underlying graph, i.e. in [0, size()). - bool isValidNode(GraphIndex node) const + bool isValidNode(Key node) const { - return node < graph_.size(); + return graph_.find(node) != graph_.end(); } /// @brief Construct a reusable Transition for the given graph. /// @details All the memory needed for a transition is allocated here, so that no further allocations are /// needed while the transition is in flight. The same transition object is then reused for multiple transitions by /// calling @ref setupTransition() with a new target node. - explicit Transition(DependencyGraph& graph) : state_(graph.capacity()), graph_(graph) + explicit Transition(DependencyGraph& graph) : state_(graph.capacity()), graph_(graph) { - state_.in_target_subgraph.assign(graph.capacity(), false); } /// @brief Set up a fresh transition to @p target. /// @details Starts in the Stopping Phase: every node currently running and not needed by @p target is /// deactivated (derived from live component state across the whole graph, so nodes left running by a previous /// aborted transition are captured too). Then moves to the Starting Phase to bring up @p target. - void setupTransition(GraphIndex target) + void setupTransition(Key target) { - std::fill(state_.in_target_subgraph.begin(), state_.in_target_subgraph.end(), false); + // Sets up or resets our stored info for this transition + for (auto [key, value] : graph_) + { + // If the key is not present in the map, this will default construct into it + NodeInfo& info = state_.node_information[key]; + info.enqueued_ = false; + info.in_target_subgraph_ = false; + } state_.target_root = target; clearNextNodes(); state_.pending = 0; state_.phase = Phase::Stopping; - state_.enqueued_set.reset(); setupDeactivation(target); if (state_.pending == 0) { @@ -265,70 +278,76 @@ class Transition Done, ///< every participating node has reached its terminal state }; - struct State + struct NodeInfo { - /// @brief Per-node membership mask - /// @details in_target_subgraph[i] is true iff node - /// i is reachable from target_root (i.e. belongs to the subgraph about to - /// be running). Recomputed at every setup. Serves two purposes: + /// @brief True if the node is in the subgraph of nodes we wish to process + /// @details in_target_subgraph[i] is true iff node is reachable from target_root (i.e. belongs to the subgraph + /// about to be running). Recomputed at every setup. Serves two purposes: /// stopping: the nodes excluded from the whole-graph stop scan, and the /// nodes onNodeFinished must never (re)stop. /// starting: nodes that are directly or indirectly depended on by /// the target_root. - std::vector in_target_subgraph; + bool in_target_subgraph_{false}; + + /// @brief True if the node has been enqueued for the current transition phase + /// @deprecated This is a workaround for the case where two processes are started in parallel and their events + /// processed in sequence. Both onNodeFinished() calls detect that all dependents are ready and try to enqueue + /// successors. Detection of dependency readiness should be reworked to remove this. See #427 + bool enqueued_{false}; + }; + struct State + { /// @brief The destination subgraph's root (the `target` endpoint) - GraphIndex target_root{}; + Key target_root{}; /// @brief The nodes that are ready to be activated/deactivated in the current phase, in the order they were /// discovered. - internal::FixedSizeQueue next_nodes; + internal::FixedSizeQueue next_nodes; std::size_t pending = 0; // nodes still to reach terminal state in this phase Phase phase = Phase::Done; // active vs deactivation vs finished - /// @brief Nodes that have been enqueued for the current transition phase - /// @deprecated This is a workaround for the case where two processes are started in parallel and their events - /// processed in sequence. Both onNodeFinished() calls detect that all dependents are ready and try to enqueue - /// successors. Detection of dependency readiness should be reworked to remove this. - std::bitset(internal::ProcessLimits::kMaxProcesses)> enqueued_set{}; + /// @brief Information we need to maintain about graph nodes for the current transition + std::unordered_map node_information; - State(std::size_t nodes) : next_nodes(nodes) + explicit State(std::size_t nodes) : next_nodes(nodes) { + node_information.reserve(nodes); } }; /// @brief Check if the node is active - bool active(GraphIndex i) + bool active(Key i) { return componentOf(graph_[i]).active(); } /// @brief Check if the node is stopped - bool stopped(GraphIndex i) + bool stopped(Key i) { return !componentOf(graph_[i]).active(); } /// @brief Check if all dependencies of the given node are active - bool allDepsActive(GraphIndex i) + bool allDepsActive(Key i) { const auto& d = graph_.dependsOn(i); - return std::all_of(d.begin(), d.end(), [this](GraphIndex dep) { + return std::all_of(d.begin(), d.end(), [this](Key dep) { return active(dep); }); } /// @brief Check if all dependents of the given node are stopped - bool allDependentsStopped(GraphIndex i) + bool allDependentsStopped(Key i) { const auto& d = graph_.dependents(i); - return std::all_of(d.begin(), d.end(), [this](GraphIndex dep) { + return std::all_of(d.begin(), d.end(), [this](Key dep) { return stopped(dep); }); } State state_; - DependencyGraph& graph_; + DependencyGraph& graph_; /// @brief The action based on whether the transition is in the Stopping or Starting phase Action currentAction() const @@ -337,11 +356,11 @@ class Transition } /// @brief Check if the node is ready to be activated/deactivated in the current phase. - bool isReady(GraphIndex s) + bool isReady(Key s) { return state_.phase == Phase::Starting - ? (state_.in_target_subgraph[s] && !active(s) && allDepsActive(s)) - : (!state_.in_target_subgraph[s] && !stopped(s) && allDependentsStopped(s)); + ? (state_.node_information.at(s).in_target_subgraph_ && !active(s) && allDepsActive(s)) + : (!state_.node_information.at(s).in_target_subgraph_ && !stopped(s) && allDependentsStopped(s)); } void clearNextNodes() @@ -360,7 +379,10 @@ class Transition state_.phase = Phase::Starting; state_.pending = 0; clearNextNodes(); - state_.enqueued_set.reset(); + for (auto& [key, value] : state_.node_information) + { + value.enqueued_ = false; + } setupActivation(state_.target_root); if (state_.pending == 0) @@ -374,10 +396,10 @@ class Transition /// - in_target_subgraph: marks the nodes that are part of the target subgraph /// - next_nodes: the list of nodes that are ready to be activated (those whose dependencies are all active) /// - pending: the count of nodes that are still to be activated - void setupActivation(GraphIndex root) + void setupActivation(Key root) { - graph_.traverse(root, [this](GraphIndex i) -> const std::vector& { - state_.in_target_subgraph[i] = true; + graph_.traverse(root, [this](Key i) -> const std::vector& { + state_.node_information[i].in_target_subgraph_ = true; if (!active(i)) { ++state_.pending; @@ -400,20 +422,21 @@ class Transition /// subgraph. Deriving it from live component state rather than from a source root makes it independent of how the /// previous transition ended, so nodes left running by an aborted transition — even ones outside any assumed /// source subgraph — are still stopped. - void setupDeactivation(GraphIndex target) + void setupDeactivation(Key target) { - graph_.traverse(target, [this](GraphIndex i) -> const std::vector& { - state_.in_target_subgraph[i] = true; + graph_.traverse(target, [this](Key i) -> const std::vector& { + state_.node_information[i].in_target_subgraph_ = true; return graph_.dependsOn(i); }); - for (GraphIndex i = 0; i < graph_.size(); ++i) + + for (const auto& [key, value] : state_.node_information) { - if (!state_.in_target_subgraph[i] && !stopped(i)) + if (!value.in_target_subgraph_ && !stopped(key)) { ++state_.pending; - if (allDependentsStopped(i)) + if (allDependentsStopped(key)) { - state_.next_nodes.push(i); + state_.next_nodes.push(key); } } } @@ -425,11 +448,11 @@ class Transition /// @details The builder only supports a single transition at a time. It is /// expected that whenever a new transition is created, the previous one is no longer in use. /// The reason is that Memory is only allocated during initialization and then reused for each transition. -template +template class TransitionBuilder final { public: - explicit TransitionBuilder(DependencyGraph& graph) : transition_(graph) + explicit TransitionBuilder(DependencyGraph& graph) : transition_(graph) { } @@ -437,7 +460,7 @@ class TransitionBuilder final /// @details First deactivates every node currently running that is not needed by @p target (keeping anything /// shared with @p target active), then activates all nodes reachable from @p target. The stop set is derived from /// live component state, so this recovers correctly even when a previous transition was aborted mid-flight. - Transition& createTransition(GraphIndex target) + Transition& createTransition(Key target) { SCORE_LANGUAGE_FUTURECPP_ASSERT(transition_.isValidNode(target)); transition_.setupTransition(target); @@ -446,7 +469,7 @@ class TransitionBuilder final private: /// @brief The single reusable transition - Transition transition_; + Transition transition_; }; } // namespace score::mw::lifecycle diff --git a/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp b/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp index 11afb6c55..0fb818895 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/details/transition_UT.cpp @@ -10,6 +10,7 @@ * * SPDX-License-Identifier: Apache-2.0 ********************************************************************************/ +#include "score/mw/launch_manager/common/identifier_hash.hpp" #include "score/mw/launch_manager/process_group_manager/details/dependency_graph.hpp" #include "score/mw/launch_manager/process_group_manager/details/icomponent.hpp" #include "score/mw/launch_manager/process_group_manager/details/transition.hpp" @@ -30,13 +31,17 @@ class MockComponent : public IComponent public: MockComponent() { + static std::size_t index{0}; + name_ = IdentifierHash{std::to_string(index++)}; + ON_CALL(*this, active()).WillByDefault(::testing::ReturnPointee(&active_)); + ON_CALL(*this, getIdentifier()).WillByDefault(::testing::ReturnPointee(&name_)); } MOCK_METHOD(RequestResult, activate, (score::cpp::stop_token), (override)); MOCK_METHOD(RequestResult, deactivate, (score::cpp::stop_token), (override)); MOCK_METHOD(RequestResult, tryHandleTermination, (int32_t), (override)); - MOCK_METHOD(uint32_t, getIndex, (), (const, override)); + MOCK_METHOD(IdentifierHash, getIdentifier, (), (const, override)); MOCK_METHOD(bool, active, (), (const, override)); /// @brief Flip both flags together, mirroring a real component reaching a terminal state. @@ -49,14 +54,9 @@ class MockComponent : public IComponent // Default: inactive and fully stopped. bool active_ = false; bool stopped_ = true; -}; -/// @brief Test-only projection for Transition, mirroring component_of.hpp's -/// production overload. Declared in this namespace so Transition finds it via ADL. -IComponent& componentOf(IComponent* node) -{ - return *node; -} + IdentifierHash name_; +}; using ComponentType = internal::IComponent*; @@ -76,32 +76,35 @@ class TransitionTest : public ::testing::Test /// nodes are added. void makeGraph(std::size_t node_count) { - graph_ = std::make_unique>(node_count); + graph_ = std::make_unique>(node_count); components_.clear(); - builder_ = std::make_unique>(*graph_); + builder_ = std::make_unique>(*graph_); } /// @brief Add a fresh mock-backed node and return its index. - GraphIndex addNode() + IdentifierHash addNode() { - components_.push_back(std::make_unique<::testing::NiceMock>()); - return graph_->emplace(components_.back().get()); + auto component = std::make_unique<::testing::NiceMock>(); + const IdentifierHash name = component->name_; + const auto res = components_.emplace(name, std::move(component)); + graph_->try_emplace(name, res.first->second.get()); + return name; } - internal::MockComponent& componentAt(GraphIndex i) + internal::MockComponent& componentAt(IdentifierHash i) { - return *components_[i]; + return *components_.at(i); } /// @brief Mark @p node active and report it finished — mimics an activation completing. - void activate(Transition& t, GraphIndex node) + void activate(Transition& t, IdentifierHash node) { componentAt(node).setActive(true); t.onNodeFinished(node); } /// @brief Mark @p node stopped and report it finished — mimics a deactivation completing. - void deactivate(Transition& t, GraphIndex node) + void deactivate(Transition& t, IdentifierHash node) { componentAt(node).setActive(false); t.onNodeFinished(node); @@ -110,19 +113,19 @@ class TransitionTest : public ::testing::Test /// @brief Drain everything ready right now into a vector so it can be matched. /// @details Iterating CONSUMES the frontier, so call this once per step (after each /// onNodeFinished()), not repeatedly for the same step. - static std::vector collectReady(Transition& t) + static std::vector> collectReady(Transition& t) { - std::vector out; - for (const ReadyNode rn : t) + std::vector> out; + for (const ReadyNode rn : t) { out.push_back(rn); } return out; } - std::unique_ptr> graph_; - std::vector>> components_; - std::unique_ptr> builder_; + std::unique_ptr> graph_; + std::unordered_map>> components_; + std::unique_ptr> builder_; }; // --------------------------------------------------------------------------- @@ -158,7 +161,7 @@ TEST_F(EmptyGraphDeathTest, CreateTransitionAssertsOnOutOfRangeTarget) RecordProperty( "Description", "createTransition() with an out-of-range target index aborts via a futurecpp assert."); - EXPECT_DEATH(builder_->createTransition(0), ""); + EXPECT_DEATH(builder_->createTransition(IdentifierHash{"not real"}), ""); } // --------------------------------------------------------------------------- @@ -178,7 +181,7 @@ class SingleNodeGraphTest : public TransitionTest node_ = addNode(); } - GraphIndex node_{}; + IdentifierHash node_{}; }; TEST_F(SingleNodeGraphTest, TransitionStartsTheNode) @@ -191,7 +194,7 @@ TEST_F(SingleNodeGraphTest, TransitionStartsTheNode) auto& transition = builder_->createTransition(node_); EXPECT_FALSE(transition.isFinished()); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{node_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{node_, Action::Start})); activate(transition, node_); @@ -222,19 +225,19 @@ TEST_F(SingleNodeGraphTest, TransitionToOffStopsTheRunningNodeThenStartsOff) "dependency-less Off node; the transition finishes once both reach their terminal state."); makeGraph(2); - const GraphIndex node = addNode(); - const GraphIndex off = addNode(); + const IdentifierHash node = addNode(); + const IdentifierHash off = addNode(); componentAt(node).setActive(true); // node running; Off node stopped (default) auto& transition = builder_->createTransition(off); // Stopping phase: the running node is stopped first (Off does not depend on it). EXPECT_FALSE(transition.isFinished()); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{node, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{node, Action::Stop})); deactivate(transition, node); // Starting phase: the Off node is now ready to activate. - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{off, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{off, Action::Start})); activate(transition, off); EXPECT_THAT(collectReady(transition), ::testing::IsEmpty()); @@ -249,13 +252,13 @@ TEST_F(SingleNodeGraphTest, TransitionToOffFromAllStoppedStartsTheOffNode) "activates the dependency-less Off node, leaving the stopped application node untouched."); makeGraph(2); - const GraphIndex node = addNode(); // an application node, left stopped - const GraphIndex off = addNode(); + const IdentifierHash node = addNode(); // an application node, left stopped + const IdentifierHash off = addNode(); auto& transition = builder_->createTransition(off); EXPECT_FALSE(transition.isFinished()); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{off, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{off, Action::Start})); activate(transition, off); EXPECT_THAT(collectReady(transition), ::testing::IsEmpty()); @@ -303,12 +306,12 @@ class SharedNodeGraphTest : public TransitionTest graph_->addDependency(rt2_, b_); } - GraphIndex a_{}; - GraphIndex b_{}; - GraphIndex c_{}; - GraphIndex rt1_{}; - GraphIndex rt2_{}; - GraphIndex off_{}; + IdentifierHash a_{}; + IdentifierHash b_{}; + IdentifierHash c_{}; + IdentifierHash rt1_{}; + IdentifierHash rt2_{}; + IdentifierHash off_{}; }; TEST_F(SharedNodeGraphTest, TransitionStartsDependenciesBeforeDependent) @@ -321,13 +324,14 @@ TEST_F(SharedNodeGraphTest, TransitionStartsDependenciesBeforeDependent) auto& transition = builder_->createTransition(rt1_); EXPECT_THAT( collectReady(transition), - ::testing::UnorderedElementsAre(ReadyNode{c_, Action::Start}, ReadyNode{b_, Action::Start})); + ::testing::UnorderedElementsAre( + ReadyNode{c_, Action::Start}, ReadyNode{b_, Action::Start})); activate(transition, c_); EXPECT_THAT(collectReady(transition), ::testing::IsEmpty()); // still blocked on B activate(transition, b_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{rt1_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{rt1_, Action::Start})); activate(transition, rt1_); EXPECT_THAT(collectReady(transition), ::testing::IsEmpty()); @@ -349,7 +353,7 @@ TEST_F(SharedNodeGraphTest, NextReadyInterleavedWithOnNodeFinishedKeepsPendingSi EXPECT_EQ(first->action, Action::Start); activate(transition, first->node); // must not discard the sibling - const GraphIndex sibling = (first->node == c_) ? b_ : c_; + const IdentifierHash sibling = (first->node == c_) ? b_ : c_; const auto second = transition.nextReady(); ASSERT_TRUE(second.has_value()); EXPECT_EQ(second->node, sibling); @@ -378,21 +382,21 @@ TEST_F(SharedNodeGraphTest, TransitionBetweenRunTargetsKeepsSharedNodeActive) componentAt(rt1_).setActive(true); auto& transition = builder_->createTransition(rt2_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{rt1_, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{rt1_, Action::Stop})); deactivate(transition, rt1_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{c_, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{c_, Action::Stop})); // Finishing the last stop-set node auto-advances into the starting phase. deactivate(transition, c_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{a_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{a_, Action::Start})); // B was shared and stayed up the whole time. EXPECT_TRUE(componentAt(b_).active_); EXPECT_FALSE(componentAt(b_).stopped_); activate(transition, a_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{rt2_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{rt2_, Action::Start})); activate(transition, rt2_); EXPECT_THAT(collectReady(transition), ::testing::IsEmpty()); @@ -412,7 +416,7 @@ TEST_F(SharedNodeGraphTest, OnNodeFinishedDuringIterationDrivesWholeTransitionIn auto& transition = builder_->createTransition(rt2_); - std::vector visited; + std::vector> visited; for (const auto rn : transition) { visited.push_back(rn); @@ -424,10 +428,10 @@ TEST_F(SharedNodeGraphTest, OnNodeFinishedDuringIterationDrivesWholeTransitionIn EXPECT_THAT( visited, ::testing::ElementsAre( - ReadyNode{rt1_, Action::Stop}, - ReadyNode{c_, Action::Stop}, - ReadyNode{a_, Action::Start}, - ReadyNode{rt2_, Action::Start})); + ReadyNode{rt1_, Action::Stop}, + ReadyNode{c_, Action::Stop}, + ReadyNode{a_, Action::Start}, + ReadyNode{rt2_, Action::Start})); EXPECT_TRUE(transition.isFinished()); // B was shared and never touched. @@ -447,14 +451,14 @@ TEST_F(SharedNodeGraphTest, FailedTransitionIsRecoveredByAFreshFallbackTransitio componentAt(rt2_).setActive(true); auto& toRt1 = builder_->createTransition(rt1_); - EXPECT_THAT(collectReady(toRt1), ::testing::ElementsAre(ReadyNode{rt2_, Action::Stop})); + EXPECT_THAT(collectReady(toRt1), ::testing::ElementsAre(ReadyNode{rt2_, Action::Stop})); deactivate(toRt1, rt2_); - EXPECT_THAT(collectReady(toRt1), ::testing::ElementsAre(ReadyNode{a_, Action::Stop})); + EXPECT_THAT(collectReady(toRt1), ::testing::ElementsAre(ReadyNode{a_, Action::Stop})); // Finishing the last stop-set node auto-advances into the starting phase. deactivate(toRt1, a_); - EXPECT_THAT(collectReady(toRt1), ::testing::ElementsAre(ReadyNode{c_, Action::Start})); + EXPECT_THAT(collectReady(toRt1), ::testing::ElementsAre(ReadyNode{c_, Action::Start})); // C fails to activate: it never reports finished, so the transition is stuck forever. EXPECT_FALSE(componentAt(c_).active_); @@ -464,10 +468,10 @@ TEST_F(SharedNodeGraphTest, FailedTransitionIsRecoveredByAFreshFallbackTransitio // Fallback RT1 -> RT2: RT1's exclusive nodes are already stopped, so the stopping phase is a // no-op and the transition starts straight in the starting phase. auto& toRt2 = builder_->createTransition(rt2_); - EXPECT_THAT(collectReady(toRt2), ::testing::ElementsAre(ReadyNode{a_, Action::Start})); + EXPECT_THAT(collectReady(toRt2), ::testing::ElementsAre(ReadyNode{a_, Action::Start})); activate(toRt2, a_); - EXPECT_THAT(collectReady(toRt2), ::testing::ElementsAre(ReadyNode{rt2_, Action::Start})); + EXPECT_THAT(collectReady(toRt2), ::testing::ElementsAre(ReadyNode{rt2_, Action::Start})); activate(toRt2, rt2_); EXPECT_THAT(collectReady(toRt2), ::testing::IsEmpty()); @@ -497,7 +501,7 @@ TEST_F(SharedNodeGraphTest, StopsOrphanLeftRunningOutsideTheLastTargetSubgraph) // Drive the whole transition in one loop, reporting each node's terminal state as it comes up. // Everything running is stopped (the orphan A included), then the Off node is activated. - std::vector stopped_nodes; + std::vector> stopped_nodes; bool off_started = false; for (const auto rn : transition) { @@ -527,10 +531,10 @@ TEST_F(SharedNodeGraphTest, StopsOrphanLeftRunningOutsideTheLastTargetSubgraph) EXPECT_THAT( stopped_nodes, ::testing::UnorderedElementsAre( - ReadyNode{a_, Action::Stop}, - ReadyNode{b_, Action::Stop}, - ReadyNode{c_, Action::Stop}, - ReadyNode{rt1_, Action::Stop})); + ReadyNode{a_, Action::Stop}, + ReadyNode{b_, Action::Stop}, + ReadyNode{c_, Action::Stop}, + ReadyNode{rt1_, Action::Stop})); } // --------------------------------------------------------------------------- @@ -571,11 +575,11 @@ class LinearGraphTest : public TransitionTest componentAt(d_).setActive(true); } - GraphIndex a_{}; - GraphIndex b_{}; - GraphIndex c_{}; - GraphIndex d_{}; - GraphIndex off_{}; + IdentifierHash a_{}; + IdentifierHash b_{}; + IdentifierHash c_{}; + IdentifierHash d_{}; + IdentifierHash off_{}; }; TEST_F(LinearGraphTest, TransitionToAStartsChainBottomUp) @@ -587,13 +591,13 @@ TEST_F(LinearGraphTest, TransitionToAStartsChainBottomUp) auto& transition = builder_->createTransition(a_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{d_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{d_, Action::Start})); activate(transition, d_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{c_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{c_, Action::Start})); activate(transition, c_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{b_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{b_, Action::Start})); activate(transition, b_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{a_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{a_, Action::Start})); activate(transition, a_); EXPECT_THAT(collectReady(transition), ::testing::IsEmpty()); @@ -612,17 +616,17 @@ TEST_F(LinearGraphTest, TransitionToOffStopsChainTopDownThenStartsOff) auto& transition = builder_->createTransition(off_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{a_, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{a_, Action::Stop})); deactivate(transition, a_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{b_, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{b_, Action::Stop})); deactivate(transition, b_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{c_, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{c_, Action::Stop})); deactivate(transition, c_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{d_, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{d_, Action::Stop})); deactivate(transition, d_); // Chain fully stopped: the Off node is now ready to activate. - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{off_, Action::Start})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{off_, Action::Start})); activate(transition, off_); EXPECT_THAT(collectReady(transition), ::testing::IsEmpty()); @@ -642,9 +646,9 @@ TEST_F(LinearGraphTest, TransitionToCStopsNodesNotNeededByC) auto& transition = builder_->createTransition(c_); // A has no dependents, so it is ready to stop first; B becomes ready once A is stopped. - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{a_, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{a_, Action::Stop})); deactivate(transition, a_); - EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{b_, Action::Stop})); + EXPECT_THAT(collectReady(transition), ::testing::ElementsAre(ReadyNode{b_, Action::Stop})); deactivate(transition, b_); // C and D are already active, so there is nothing to start: the transition is done. diff --git a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp index ceb4f1ff6..a4adbf861 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.cpp @@ -434,7 +434,7 @@ void ProcessGroupManager::controlClientResponses(Graph& pg) bool ProcessGroupManager::sendResponse(ControlClientMessage msg) { auto pin = getProcessInfoNode( - msg.originating_control_client_.process_group_index_, msg.originating_control_client_.process_index_); + msg.originating_control_client_.process_group_index_, msg.originating_control_client_.process_identifier_); bool ret = true; if (pin) @@ -479,8 +479,7 @@ void ProcessGroupManager::controlClientRequests(Graph& pg) // Fill in some routing details // Single process group at index 0 scc->request().originating_control_client_.process_group_index_ = 0U; - scc->request().originating_control_client_.process_index_ = - static_cast(control_client->getIndex() & 0xFFFFU); + scc->request().originating_control_client_.process_identifier_ = control_client->getIdentifier(); LM_LOG_DEBUG() << "ProcessGroupManager::ControlClientHandler: got request" << scc->toString(scc->request().request_or_response_) << "(" @@ -699,11 +698,11 @@ void ProcessGroupManager::setInitialStateTransitionResult(ControlClientCode resu ControlClientChannel::nudgeControlClientHandler(); } -ProcessInfoNode* ProcessGroupManager::getProcessInfoNode(uint32_t pg_index, uint32_t process_index) +ProcessInfoNode* ProcessGroupManager::getProcessInfoNode(uint32_t pg_index, IdentifierHash process_id) { if (pg_index == 0U && graph_) { - return graph_->getProcessInfoNode(process_index); + return graph_->getProcessInfoNode(process_id); } return nullptr; diff --git a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.hpp b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.hpp index 811cff93a..653d000a2 100644 --- a/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.hpp +++ b/score/launch_manager/src/daemon/src/process_group_manager/process_group_manager.hpp @@ -107,9 +107,9 @@ class ProcessGroupManager final : public ITransitionResultPublisher /// @brief Get a node corresponding to the given process group and process index /// @param pg_index The index of the process group in the list of groups managed by this manager - /// @param process_index The index of the process in the list of processes in the process group + /// @param process_id The identifier of the process /// @return nullptr if the node does not exist, otherwise a pointer to the corresponding node. - ProcessInfoNode* getProcessInfoNode(uint32_t pg_index, uint32_t process_index); + ProcessInfoNode* getProcessInfoNode(uint32_t pg_index, IdentifierHash process_id); /// @brief set the initial machine group state change result, called by graph when the transition completes /// @param result the result to save; it can only be saved once