Skip to content

Graph refactor - #392

Merged
NicolasFussberger merged 26 commits into
eclipse-score:mainfrom
etas-contrib:feature/process-mon-refactor-2
Aug 5, 2026
Merged

Graph refactor#392
NicolasFussberger merged 26 commits into
eclipse-score:mainfrom
etas-contrib:feature/process-mon-refactor-2

Conversation

@WilliamRoebuck

@WilliamRoebuck WilliamRoebuck commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

The description below is not AI generated. It includes some key information needed to begin a review.


Overview

This is the completion of the refactor started in #370. With these changes, the following is achieved:

  • Transitions now activate run targets instead of states and operate on the component level, rather than processes. See IComponent. This means that different types of component can now be implemented, such as containers.
  • Ready conditions implemented in code.
  • Graph now processes events sequentially rather than in parallel. See ComponentEventQueue.
  • ProcessInfoNode is no longer responsible for dependency resolution. This responsibility is moved to Transition.
  • ProcessInfoNode & Graph coupling resolved - ProcessInfoNode no longer has access to Graph.

New components

  • DependencyGraph - Data structure for storing and traversing component dependencies
  • Transition - Resolves run target activation logic. Activates and deactivates components as required.

Behavioral change

Processes are now managed based on component state, rather than process state. This means that once a component is activated, the underlying process shall not be started again until the component becomes inactive (either due to an error or a run target activation). Previously, a self-terminating process would be restarted on every run target activation that included it.

Minor changes

  • kLaunchManager comms type removed. This was legacy and no longer needed.
  • Configuration now generates an "Off" run target if none is configured
  • Several new interfaces have been added to allow for unit testing of existing components

Review strategy

As this PR is quite large, it may be necessary for reviewers each to tackle subsections of the changes. In terms of line numbers, these sections are roughly equal:

  • graph.cpp + graph.hpp (+ unit tests)
  • processinfonode.cpp + processinfonode.hpp (+ unit tests)
  • transition.hpp + transition_UT.hpp
  • dependency_graph.hpp + dependency_graph_UT.cpp + process_group_manager.cpp + process_group_manager.hpp
  • Any remaining unreviewed changes

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.6.0) and connecting to it...
INFO: Invocation ID: 50b2723a-ee99-4c6b-86f1-d4cdbe2d8761
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (66 packages loaded, 10 targets configured)

Analyzing: target //:license-check (90 packages loaded, 10 targets configured)

Analyzing: target //:license-check (141 packages loaded, 595 targets configured)

Analyzing: target //:license-check (156 packages loaded, 3172 targets configured)

Analyzing: target //:license-check (164 packages loaded, 9351 targets configured)

Analyzing: target //:license-check (170 packages loaded, 9397 targets configured)

Analyzing: target //:license-check (171 packages loaded, 9397 targets configured)

Analyzing: target //:license-check (173 packages loaded, 11285 targets configured)

Analyzing: target //:license-check (173 packages loaded, 11285 targets configured)

Analyzing: target //:license-check (174 packages loaded, 11409 targets configured)

INFO: Analyzed target //:license-check (175 packages loaded, 11535 targets configured).
[12 / 16] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox ... (2 actions, 1 running)
[14 / 16] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache, processwrapper-sandbox
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 25.152s, Critical Path: 1.84s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@github-actions

Copy link
Copy Markdown

The created documentation from the pull request is available at: docu-html

@danth danth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Various nitpicks and ramblings. I have seen all the code but did not check the unit tests in much detail yet.

Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp Outdated
Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/graph.cpp Outdated
Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/transition.hpp Outdated
Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/transition.hpp Outdated
@WilliamRoebuck WilliamRoebuck changed the title Process monitor refactor Graph refactor Aug 3, 2026
@paulquiring

Copy link
Copy Markdown
Contributor

@WilliamRoebuck @NicolasFussberger @danth @MaciejKaszynski
Most of the remaining comments are just improvements which we can collect in issues to continue to work on. This PR in good shape, lets merge today.

MaciejKaszynski
MaciejKaszynski previously approved these changes Aug 5, 2026
Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/graph.hpp Outdated
@NicolasFussberger
NicolasFussberger merged commit f3dcf9b into eclipse-score:main Aug 5, 2026
20 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in LCM - Lifecycle & Health FT Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants