Skip to content

Introduce watchdog into ProcessGroupManager [3/N]: Move ownership to ProcessGroupManager#355

Open
cameron-craig-etas wants to merge 7 commits into
eclipse-score:mainfrom
etas-contrib:feature/move-watchdog-to-launch-manager-part-3
Open

Introduce watchdog into ProcessGroupManager [3/N]: Move ownership to ProcessGroupManager#355
cameron-craig-etas wants to merge 7 commits into
eclipse-score:mainfrom
etas-contrib:feature/move-watchdog-to-launch-manager-part-3

Conversation

@cameron-craig-etas

@cameron-craig-etas cameron-craig-etas commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

#330

This is part 3 of a number of PRs, to move the watchdog ownership to ProcessGroupManager. Now that we have prepared the watchdog sources in previous PRs, we are now doing the actual owner change here.

I have performed the following steps:

  • We now create a watchdog object using the createWatchdog() factory.
  • The watchdog has been removed from the alive monitor and PhmDaemon.
  • We now call the watchdog from the main loop in process_group_manager.cpp
  • The watchdog is only enabled for the new config.

Note that unit tests are to follow in a separate PR.

@github-actions

github-actions Bot commented Jul 23, 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: 4229620d-e202-44c1-ad35-d95bf74903c7
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
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 (40 packages loaded, 10 targets configured)

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

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

Analyzing: target //:license-check (144 packages loaded, 2769 targets configured)

Analyzing: target //:license-check (152 packages loaded, 3575 targets configured)

Analyzing: target //:license-check (154 packages loaded, 8165 targets configured)

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

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

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

Analyzing: target //:license-check (167 packages loaded, 9716 targets configured)

Analyzing: target //:license-check (168 packages loaded, 10283 targets configured)

INFO: Analyzed target //:license-check (169 packages loaded, 10409 targets configured).
[12 / 16] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox ... (2 actions running)
[14 / 16] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache, processwrapper-sandbox
[15 / 16] Building license.check.license_check.jar (); 0s disk-cache, multiplex-worker
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.428s, Critical Path: 2.49s
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>]

@cameron-craig-etas
cameron-craig-etas force-pushed the feature/move-watchdog-to-launch-manager-part-3 branch from c2daf96 to d7352ab Compare July 23, 2026 12:52
@github-actions

Copy link
Copy Markdown

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

{
LM_LOG_ERROR() << "Watchdog: Invalid watchdog device configuration. Watchdog initialization failed.";
isSuccess = false;
LM_LOG_ERROR() << "Watchdog: Invalid watchdog timeout value " << watchdog_config.max_timeout_ms

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
LM_LOG_ERROR() << "Watchdog: Invalid watchdog timeout value " << watchdog_config.max_timeout_ms
LM_LOG_ERROR() << "Watchdog: Invalid watchdog timeout value" << watchdog_config.max_timeout_ms

I think the "<<" operator in logging implementation adds an additional whitespace automatically. If we add an extra whitespace, we'll have 2 whitespace in the resulting log line

@NicolasFussberger

Copy link
Copy Markdown
Contributor

I think score/launch_manager/src/daemon/src/watchdog/IDeviceConfigFactory.hpp can now be moved to details folder, as the public interface does not require this anymore.

hdrs = ["DeviceIf.hpp"],
include_prefix = "score/mw/launch_manager/watchdog/details",
strip_include_prefix = "/score/launch_manager/src/daemon/src/watchdog/details",
visibility = ["//score:__subpackages__"],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One idea: We could set the visibility for the library targets in this file to "//score/launch_manager/src/watchdog:__subpackages__" so that bazel also enforced that these classes are internal to the watchdog component.

@NicolasFussberger

Copy link
Copy Markdown
Contributor

If I see it correctly, I think score/launch_manager/src/daemon/src/alive_monitor/details/factory/MachineConfigFactory.cpp and score/launch_manager/src/daemon/src/alive_monitor/details/factory/MachineConfigFactory_new.cpp can be removed / trimmed down as the watchdog is no longer loaded from config there.

I think the only value still required is the periodicity in which supervisions are evaluated.
For the other values we anyway only use the defaults as this is no longer configurable in the json.

@NicolasFussberger

Copy link
Copy Markdown
Contributor

If I see it correctly, I think score/launch_manager/src/daemon/src/alive_monitor/details/factory/MachineConfigFactory.cpp and score/launch_manager/src/daemon/src/alive_monitor/details/factory/MachineConfigFactory_new.cpp can be removed / trimmed down as the watchdog is no longer loaded from config there.

I think the only value still required is the periodicity in which supervisions are evaluated. For the other values we anyway only use the defaults as this is no longer configurable in the json.

Though this could also be done in a separate PR if you like to keep the changes here limited.

visibility = ["//score:__subpackages__"],
deps = [":i_device_config_factory"],
deps = [
":i_device_config_factory",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
":i_device_config_factory",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants