Skip to content

Add supervision to the new Component class - #779

Open
danth wants to merge 6 commits into
eclipse-score:mainfrom
etas-contrib:pin-split-supervision
Open

Add supervision to the new Component class#779
danth wants to merge 6 commits into
eclipse-score:mainfrom
etas-contrib:pin-split-supervision

Conversation

@danth

@danth danth commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Add supervision setup to the new Component class.

This is a continuation of #705.

@danth
danth requested a deployment to workflow-approval October 12, 2026 15:40 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 12, 2026 15:40 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 12, 2026 15:40 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 12, 2026 15:40 — with GitHub Actions Waiting
@danth danth changed the title Add supervision to new Component class Add supervision to the new Component class Oct 12, 2026
@danth
danth requested a deployment to workflow-approval October 12, 2026 15:45 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 12, 2026 15:45 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 12, 2026 15:45 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 12, 2026 15:45 — with GitHub Actions Waiting
@NicolasFussberger

NicolasFussberger commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

One note regarding the supervision. In the current implementation this is only done via deactivate() call.
There is quite a chain until this point is reached in case of a process crash.
Process Crash -> OSHandler -> .... -> Unexpected Termination Event in EventQueue -> Retrieve event -> deactivate() component -> deactivate supervision.

This time period btw. process crash and actually deactivating the supervision creates some room for false positive supervision failures.

So I am wondering, if the supervision should rather be deactivated directly when getting the process terminated notification in the component to avoid this issue.

@danth
danth force-pushed the pin-split-supervision branch from ed9c66a to a56e02f Compare October 19, 2026 10:11
@github-actions

github-actions Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Documentation preview for this pull request is available at:
pr-779: https://eclipse-score.github.io/lifecycle/pr-779/

@danth
danth force-pushed the pin-split-supervision branch from 8fe6f4c to 2c819e2 Compare October 23, 2026 08:21
@danth
Remove TODO comment
e2870ab 
This does not make sense because the supervision is created once
in the constructor, not when the state changes.
@danth
danth marked this pull request as ready for review October 23, 2026 14:34
NicolasFussberger
NicolasFussberger requested changes Oct 26, 2026
View reviewed changes
Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/component.cpp Outdated
Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/component.cpp
config.component_properties.application_profile.alive_supervision.value());
if (!handle)
{
LM_LOG_ERROR() << "Failed to set up alive supervision for" << config.name;

@NicolasFussberger NicolasFussberger Oct 26, 2026

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.

If this fails, the component must not be started later. Otherwise it would run without supervision

Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/component.cpp

if (!deactivateSupervision())
{
state_ = FaultState{};

@NicolasFussberger NicolasFussberger Oct 26, 2026

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.

What would be the reaction following the fault state?
I wonder if we should still continue shutting down the process here?

NicolasFussberger
NicolasFussberger reviewed Oct 26, 2026
View reviewed changes
Comment thread score/launch_manager/src/daemon/src/process_group_manager/details/component.hpp
/// @param supervision_factory Factory used to construct alive supervision.
/// @param config Configuration of the component.
Component(
const IStartAction& start_action,

@NicolasFussberger NicolasFussberger Oct 26, 2026

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.

What owns these objects if they are passed only as const ref?

@danth
danth requested a deployment to workflow-approval October 26, 2026 11:55 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 26, 2026 11:55 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 26, 2026 11:55 — with GitHub Actions Waiting
@danth
danth requested a deployment to workflow-approval October 26, 2026 11:55 — with GitHub Actions Waiting

This branch is waiting to be deployed

1 waiting deployment
workflow-approval — cb0d0ac7 Waiting Oct 26, 2026 by danth via Build and test arm64-qnx / approval #1778
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NicolasFussberger NicolasFussberger NicolasFussberger requested changes
@pawelrutkaq pawelrutkaq Awaiting requested review from pawelrutkaq pawelrutkaq is a code owner
@ramceb ramceb Awaiting requested review from ramceb ramceb is a code owner
@MaciejKaszynski MaciejKaszynski Awaiting requested review from MaciejKaszynski MaciejKaszynski is a code owner
@FScholPer FScholPer Awaiting requested review from FScholPer FScholPer is a code owner
@antonkri antonkri Awaiting requested review from antonkri antonkri is a code owner
@anmittag anmittag Awaiting requested review from anmittag anmittag is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

LCM - Lifecycle & Health FT
Status: Backlog

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@danth @NicolasFussberger