Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SILKIT-1477 SystemController Utility: Use SignalHandler, better report state #33

Merged
merged 15 commits into from
May 7, 2024
Merged

SILKIT-1477 SystemController Utility: Use SignalHandler, better report state #33

merged 15 commits into from
May 7, 2024

Conversation

AndreasRentschler
Copy link
Contributor

Subject

Improvements of SystemController utility

Description

SILKIT-1477
System controller blocks new participants after required participants have terminated

  • SystemController did not consistently report about the system's shutdown/error state.
  • Reconnecting a participant after the system went into Shutdown state made the SystemController appear to hang.
  • Changed to SignalHandler approach to allow for implicit exit after shutdown (std::cin.ignore() cannot be canceled).
    SIgnalHandler had to be turned into a reusable module, previously owned by Registry utility.
  • Bugfix: Warning on valid transition from Aborting to Shutdown
  • RequestReply was ignored when barrier has not yet been passed. Now, a warning is emitted and the new RequestReply is executed.

Instructions for review / testing

  • SignalHandler, refactored for public use
  • Changes in SystemStateTracker.cpp and LifecycleStates.cpp

Developer checklist (address before review)

  • Changelog.md updated
  • Prepared update for depending repositories
  • Documentation updated (public API changes only)
  • API docstrings updated (public API changes only)
  • Rebase → commit history clean
  • Squash and merge → proper PR title

@AndreasRentschler
SILKIT-1477 SystemController Utility: Improve messages for certain sc…
46e6ed3 
…enarios
@AndreasRentschler
SILKIT-1477 SystemController Utility: Improve messages for certain sc…
f727e1b 
…enarios

* Leaner messages: "SIL Kit simulation" => "simulation"
@AndreasRentschler
SILKIT-1477 SystemController Utility: Replace interactive mode with S…
aa0b2c0 
…igInt handler
@AndreasRentschler
SILKIT-1477 SystemController Utility: Use common SignalHandler
a9905fd
@AndreasRentschler
SILKIT-1477 SystemController Utility: Revert experimental changes to …
edbe5b7 
…VAsioConnection.cpp
@AndreasRentschler
SILKIT-1477 SystemController Utility: Add --non-interactive mode back
3684914
@AndreasRentschler
SILKIT-1477 Lifecycle state transition from Aborting to Shutdown is v…
b52f8d3 
…alid
@AndreasRentschler
SILKIT-1477 SystemController Utility: Remove unused signal hander code
2a1c74a
@AndreasRentschler
SILKIT-1477 Fix clang build
ab9b372
@AndreasRentschler
SILKIT-1477 Just warn if reply barrier is entered while still active
03acf49
@AndreasRentschler
SILKIT-1477 Shut down participant even in case of Error
c72bdd1
@AndreasRentschler
SILKIT-1477 SystemController Utility: Cleanup messages
ad8bb9a
@AndreasRentschler
SILKIT-1477 Update CHANGELOG.rst
48da4ee
@AndreasRentschler
SILKIT-1477 SystemController Utility: Revert documentation changes
3bde84a
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 25, 2024 17:48 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 25, 2024 17:48 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 25, 2024 17:48 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 25, 2024 17:48 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 25, 2024 17:48 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 25, 2024 17:48 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 25, 2024 17:48 — with GitHub Actions Inactive
MariusBgm
MariusBgm requested changes Apr 26, 2024
View reviewed changes
Copy link
Collaborator

@MariusBgm MariusBgm left a comment

Choose a reason for hiding this comment

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

Only looked at the SignalHandler.cpp: we should initialize the stack variables

SilKit/source/util/SignalHandler.cpp Outdated Show resolved Hide resolved
SilKit/source/util/SignalHandler.cpp Outdated Show resolved Hide resolved
SilKit/source/util/SignalHandler.cpp Show resolved Hide resolved
KonradBkd
KonradBkd requested changes Apr 26, 2024
View reviewed changes
Copy link
Contributor

@KonradBkd KonradBkd left a comment

Choose a reason for hiding this comment

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

The two critical changes should be reverted, otherwise ok👍

SilKit/source/core/requests/procs/ParticipantReplies.cpp Show resolved Hide resolved
SilKit/source/services/orchestration/LifecycleStates.cpp Outdated Show resolved Hide resolved
@AndreasRentschler
SILKIT-1477 SystemController Utility: Changes based on code reviews
9bc59c4 
* Reverted change which made error state a final state, so that the system can still be debugged
* Reverted change that made previous call-replied action be ignored, so that new call-replied action will be ignored, as previously. Issue must be handled correctly, e.g. by supporting multiple barriers.
 * Fixed several issues in SignalHandler.cpp
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 26, 2024 18:09 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 26, 2024 18:09 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 26, 2024 18:09 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 26, 2024 18:09 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 26, 2024 18:09 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 26, 2024 18:09 — with GitHub Actions Inactive
@AndreasRentschler AndreasRentschler temporarily deployed to public-github-runners April 26, 2024 18:09 — with GitHub Actions Inactive
@AndreasRentschler
Copy link
Contributor Author

Thanks for your reviews, all suggestions were implemented.

KonradBkd
KonradBkd approved these changes Apr 29, 2024
View reviewed changes
Copy link
Contributor

@KonradBkd KonradBkd left a comment

Choose a reason for hiding this comment

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

LGTM

@AndreasRentschler AndreasRentschler merged commit b20981f into main May 7, 2024
8 checks passed
@AndreasRentschler AndreasRentschler deleted the silkit-1477_reconnecting_to_systemcontroller branch May 7, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KonradBkd KonradBkd KonradBkd approved these changes

@MariusBgm MariusBgm MariusBgm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AndreasRentschler @MariusBgm @KonradBkd