fault_manager: a planned-stop switch, so a weekend is not a wave of faults by bburda · Pull Request #664 · selfpatch/ros2_medkit

bburda · 2026-09-08T08:15:26Z

Pull Request

Summary

A plant stops on purpose: a weekend, a changeover, a maintenance slot. Every controller the fault manager watches goes quiet at once, and there was no way to tell it that this was expected. With a debounce set and healing on, every one of those faults confirms and heals, every transition is an audit row, and the first Monday starts with a fault list nobody can read.

A planned stop is a switch on the fault manager, and the operator's interface owns the calendar. ~/set_planned_stop {active, reason, declared_by} and ~/get_planned_stop are ROS services, so they are already SOVD operations on the fault manager entity: POST /apps/fault_manager/operations/set_planned_stop/executions, behind the operations routes' existing role. No new route, no new DTO, no change to any released message or service type, no gateway source file touched; the served OpenAPI document is byte-identical before and after.

Marked, not suppressed. While the switch is on, a fault whose cycle starts (a new code, a reactivation from CLEARED, or a re-fail from HEALED) is recorded, debounced, captured and audited exactly as any other fault, and is muted the way a correlated symptom is muted: out of the default fault list, counted in muted_count, visible with include_muted, its confirmation and updates withheld from the event topic (a clear or heal is published as it is for any muted fault). The bridges report one FAILED per transition, so dropping such a fault would lose it for as long as the alarm stands on the controller; marking keeps it.

Ownership is a persisted fact. The stop owns the cycles that started inside it, as a flag on the fault row in the store; the correlation engine derives its mute map from that ownership plus any rule that overlays it, and when the rule's mute ends the stop's mute is back. A restart inside a stop keeps every owned fault muted. Switching off writes the declaration, announces one confirmation per CONFIRMED owned fault, then clears the flags, and a start-up that finds flags behind a withdrawn declaration finishes that release before it serves anything. Every switch transition is an audit row with the reason and the declarer (with audit_log.enabled), and get_planned_stop serves the last declaration after the withdrawal.

Issue

closes Declare a planned stop, so a weekend is not a wave of faults #656

Type

Bug fix
New feature or tests
Breaking change
Documentation only

One observable change on a fault manager with no correlation rules: the engine and its cleanup timer now always exist, so muted_count and cluster_count are emitted as 0 and the timer costs about 190 ns per tick at the 5 s default, measured.

Testing

Gateway end to end (real gateway, fault manager and a reporting node over the real graph): the switch through the operations route, faults raised during the stop out of the default list and in include_muted, nothing on the stream for them, a fault confirmed before the stop untouched by it, one confirmation per released fault after switch-off with a settle window proving no second frame.
Fault manager over its services with SQLite: the exact event set seen during a stop, a fault that clears during the stop, a re-fail from HEALED, a rule-muted symptom and a cluster-muted code composed with the stop, the scoped acknowledge, the auto-confirm timer under the stop, restart inside a stop, an interrupted release finished at the next start, a new stop declared right after such a start, a store that refuses the write, and 200 faults released at once.
Unit: the engine's two mute sources and their hand-over, the storage flag and its migration on a database from before the column existed, the audit markers, the release ordering with a store that records call order.
Mutations run and recorded: unmute-at-switch-off removed, announcement removed, persistence removed, cycle-start gate forced, PASSED gate removed, re-mute on rule expiry removed, flags cleared before the announcement; each reddens the test that pins it.
Suites: fault manager unit and integration, msgs unit, the fault, operations, health, OpenAPI contract and error-coverage feature tests, lint; clang-tidy zero findings in changed hunks; Sphinx with no new warnings.

Checklist

Breaking changes are clearly described (and announced in docs / changelog if needed)
Tests were added or updated if needed
Docs were updated if behavior or public API changed

SetPlannedStop declares or withdraws a planned stop and answers with the state the switch was in before the call; GetPlannedStop reads back the declaration, its reason, its declarer and when it started. Refs #656

…opping them The correlation engine gains a second mute source. While a planned stop is declared, a fault whose cycle starts is reported, debounced, confirmed, captured and audited exactly as it would be otherwise, and is registered as muted with rule_id 'planned_stop' and the pseudo root cause 'PLANNED_STOP': absent from the default fault list, counted in muted_count, and never announced on the event stream. A rule that mutes the same code takes the mute over, and withdrawing the stop then leaves it alone. Withdrawing releases every fault the stop alone was holding down and publishes one EVENT_CONFIRMED for each of those that is CONFIRMED, because that confirmation was never announced. Both transitions are recorded in the audit log with their reason and declarer, whatever audit_log.transitions says; a request for the state the switch is already in changes nothing and records nothing. The declaration is stored, so a stop declared before a restart is still in force after it. The correlation engine is now always constructed, with no rules when none are configured, because the switch needs no configuration. Refs #656

Engine unit cases pin the two mute sources composing: a rule takes a mute over from the switch and keeps it at switch-off, and a fault cleared inside the stop leaves both maps. Storage cases pin the declaration surviving a reopen on SQLite and living for the process on the memory backend, and the audit case pins a transition with no fault code chaining and staying tamper-evident. Three launch tests drive a live manager: the main one covers marking, counting, capture, the audit rows, idempotent requests, an empty and a kilobyte reason, 200 faults released by one switch-off, a fault mid-debounce when the stop ends, and a rule-muted symptom the switch must not release; one kills the manager and lets launch bring it back; one drives a confirmation through the auto-confirm timer with a control fault proving the timer fires at all. The gateway feature test is the acceptance: it turns the stop on and off through POST /apps/fault_manager/operations/set_planned_stop/executions, watches GET /faults and the SSE fault stream, and counts confirmations per fault code either side of the switch-off. Refs #656

Covers what marking means and what it does not change, how the switch and a correlation rule compose, which two configuration choices decide whether the declaration survives a restart and whether transitions are audited, and how an operator reaches the switch over SOVD as an operation on the fault manager entity - including that hybrid and manifest discovery need the entity declared before that address exists. Refs #656

Reporters are level-triggered: FaultReporter and the diagnostic bridge send FAILED for as long as the condition holds, not once per transition. Muting on any report therefore took a fault that was confirmed and announced BEFORE the stop off the fault list within a second of the declaration, and announced it a second time at the switch-off. The mark is now gated on the cycle boundary the report path already computes: a new fault, or one raised again after a clear. A repeat report of a fault the stop already marked changes nothing. One mute now has one owner. The hand-over to a rule happens where the rule registers its mute, not where any path reports should_mute: the auto-cluster path sets that flag without ever writing the mute map, so the old hand-over left the stop's entry owned by nobody - muted for good, released by nothing. In the other direction the stop no longer overwrites an entry a rule holds. An acknowledgement that skips the correlation cascade, which is every scoped per-entity clear, now also releases the stop's mute. The mark survives a restart: with a declaration still in force the manager re-registers it for every stored fault that is not CLEARED and whose cycle started at or after the declaration, so the switch-off after a reboot still releases and announces them. A store that refuses the write ends the request with success=false instead of taking the process down, and the declaration is written before anything is muted, unmuted, announced or audited. GetPlannedStop keeps serving the declaration after the withdrawal, stamped with ended_at, so the reason stays readable once the plant is back up. The audit rows carry the same __audit__ fault code the log's own lifecycle markers use. Refs #656

…anged The silence assertion now reads the exact set of event types published per fault code, and the scenario re-reports a marked fault so that an update leaking past the mute is a case the test can actually see - counting confirmations alone could not. The exactly-once assertions wait out a settle window after the counts are right, because a duplicate published a moment later satisfies a poll and is never seen. New cases: a fault already up when the stop begins stays visible and is announced once, over the services and over the gateway; a fault raised again after a clear is marked; a scoped acknowledgement releases the mute; a cluster rule does not orphan it; the declaration is readable after the withdrawal; a store that refuses the write is reported rather than fatal; the audit rows carry the installation sentinel. Docs on every surface that described the muting: what a cycle is, that publication equals a rule-muted symptom exactly (CLEARED included), that the audit rows exist only when the audit log is on, that the declaration outlives the stop, and that the correlation cleanup timer now runs on every manager. Refs #656

It repeated the claim that a marked fault is never published as a FaultEvent, and listed a GetPlannedStop response that no longer matches the service. Refs #656

The stop marked faults by writing a mute entry and remembering the codes in a set. That set was the only record, so it could not survive a restart intact, a cluster rule could strand an entry it never wrote, and a rule releasing a fault mid-stop dropped it out of the stop for good - its confirmation announced by nobody, which is the failure marking exists to prevent. Ownership is now a flag on the fault row: the stop owns a CYCLE that starts while it is declared, for the whole cycle. The mute is derived from that. A rule's mute overlays an owned fault instead of taking it, and when the overlay ends - root cause acknowledged, window closed, cluster expired - the engine re-asserts the stop's mute. Ownership ends when the fault is acknowledged or the stop is withdrawn, and a restart reads the flags back rather than comparing timestamps, which a clock step would break and which cannot tell a rule's mute from the stop's. A cycle now also starts when a healed fault fails again: the manager publishes EVENT_CLEARED at the heal, so the confirmation that follows is fresh news. occurrence_count keeps its own, narrower definition. Muting gates the PASSED path too. should_mute was computed for FAILED reports only, so a PASSED that left a muted fault CONFIRMED announced an update for a fault the list was hiding; and it was computed from what the report matched rather than from the mute map, so a repeat report of a fault whose rule had stopped matching did the same. The withdrawal writes the declaration, announces, and clears the flags last, so a process that dies in between leaves faults owned by an ended declaration - which the next startup recognises, announces once a consumer is listening, and clears. Clearing first would have turned a crash into permanent silence. The storage-injection seam is a protected constructor rather than a public runtime swap: the backend is in place before snapshot and rosbag capture borrow a raw pointer to it. Refs #656

The ordering that decides what a crash mid-release costs was only reasoned about: the interrupted-release test starts from the state such a crash leaves and cannot see how the state came about. A storage double now records the switch-off's own calls, so clearing the ownership flags before the announcement fails a test instead of passing one. The cleanup timer's cost is measured rather than asserted: ~190 ns per call on a rules-free engine, ~2.3 us of CPU per minute at the 5 s default. Docs on every surface follow the ownership model - what a cycle is, that a rule's mute overlays ownership rather than taking it and hands the fault back when it ends, that the flags and the declaration survive a restart while a rule's mute does not, and that an interrupted switch-off is finished at startup. Two stale sentences went with them: the audit header still described an empty fault code, and SetPlannedStop still said the reason is served only while the stop is on. Refs #656

…tructor Deferring it to a wall timer left a window: the services exist by then, so an operator could declare a new stop and new cycles could become owned by it, and the deferred work would announce a fault the new stop was holding and clear every ownership flag - the new stop's included. The recovery now runs synchronously in the constructor, before this node has a service or a timer that could run, and drops exactly the flags it captured; the storage gains a scoped release for that. The price is an announcement that may reach no subscriber, because nothing has matched a publisher that is milliseconds old and the events topic is volatile. That is the same accepted loss as any other publication made at startup, and the released faults are in the default fault list either way - which is what the launch test asserts, alongside the manager's own log line. Refs #656

mfaferek93 · 2026-09-11T07:46:11Z

+              "%zu fault(s) are still owned by a planned stop that was already withdrawn - a previous run did not "
+              "finish releasing them. Releasing them now.",
+              owned.size());
+  const size_t announced = announce_released(owned);


The announce half of the interrupted release reaches nobody. The publisher is created at 258 and publishes milliseconds later, before any subscriber has matched. Checked in Docker with ros2 topic echo --qos-reliability reliable /fault_manager/events up 6 s before the node and a DB with planned_stop.active=0, DURING_STOP.planned_stop_owned=1: the node logged "released 1 fault(s), announced 1", the subscriber got nothing for DURING_STOP, and did get fault_confirmed for a control fault reported 8 s later. test_planned_stop_interrupted_release.test.py:348-353 asserts only the log line. Keeping the flag clear here and deferring the publish to a one-shot timer that waits (bounded) for get_subscription_count() > 0, re-checking ownership per code at fire time, would make it real.

mfaferek93 · 2026-09-11T07:46:11Z

+      // Ownership is persisted, not inferred: the switch-off, and a restart, both
+      // read it back from the store rather than comparing timestamps.
+      if (planned_stop_.active && cycle_started) {
+        storage_->set_planned_stop_owned(request->fault_code, true);


This write lands after report_fault already committed (sqlite_fault_storage.cpp:754). A crash between the two leaves a confirmed fault with planned_stop_owned=0: restart recovery misses it, the stop's end does not release it, and its later reports announce updates during the stop. Persisting ownership inside the report transaction (pass planned_stop_.active && cycle_started into the storage call) removes the window.

mfaferek93 · 2026-09-11T07:46:11Z

+    auto it = muted_faults_.find(fault_code);
+    // A rule's overlay is not the stop's to lift: that fault stays muted, and
+    // stays unannounced, for as long as the rule holds it.
+    if (it != muted_faults_.end() && it->second.by_planned_stop) {


Cluster muting only sets result.should_mute (644) and never touches the stop's entry, so a show_as_single non-representative fault that is also stop-owned still has by_planned_stop == true here, gets released, and announce_released publishes EVENT_CONFIRMED for it with no cluster check (fault_manager_node.cpp:1583-1593). Two confirmed symptoms of one cluster during a stop both get announced when the stop ends, which is the wave this PR exists to prevent. Skipping codes an active cluster still hides (or re-asking process_fault's cluster verdict at announce time) keeps the rule in charge.

mfaferek93 · 2026-09-11T07:46:11Z

+Ownership is a flag on the fault row, so it survives a restart and is what the
+switch-off releases. The mute is derived from it: a correlation rule muting an owned
+fault overlays the stop rather than taking the fault from it, the withdrawal leaves
+a rule-held fault alone, and when the rule lets go - root cause acknowledged, window


"when the rule lets go - window closed, cluster expired - the fault is muted by the stop again" (also README:105) is not something the engine does: muted_faults_ entries are erased only in process_clear (151, 259, 264) and on the planned-stop paths (291, 313); cleanup_expired() erases none, so reassert_planned_stop_mutes() at 432 never finds work. A rule mute ends on acknowledge and nothing else; the docs should say that.

…a cluster keep hiding through a switch-off Ownership of a fault cycle is recorded by the report that starts it, inside the same storage transaction as the fault row: the store is told whether a stop is in force and decides the cycle boundary itself, so no state exists in which a cycle that started inside a stop is confirmed and unowned. Both backends carry it; on SQLite a test fails the ownership statement through a trigger and shows the fault row rolled back with it. The separate ownership setter had no production caller and is gone. At the switch-off, a fault an active show_as_single cluster still hides has its stop entry erased and is not announced, and nothing is written in its place: the cluster hides through the verdict it gives every report, so a member that lived through a stop is indistinguishable in the muted list, the counts and the audit log from one that did not. A representative acknowledged after its window closed is promoted from the active cluster's members. A fault that joins a cluster below min_count is not folded into it. An active cluster hides its members until it dissolves; min_count gates formation only. Cluster membership is not persisted, so a stop that spans a restart releases and announces every fault the store owns, and the docs say so. The docs, the service definition and the message reference describe the withdrawal as what is unmuted and what is announced, and name the one trigger that lets a rule mute go.

…ing apart from announcing A cluster folds in the members it formed with for as long as its window_ms covers their reports; a report after the window starts a new burst, so that fault is announced at the switch-off like any owned fault. The same late report publishes an update with no stop in force, so nothing about the stop causes it. The switch-off releases every fault the store says the stop owns and announces the CONFIRMED ones among them, and the restart integration test now arranges a formed cluster and a PREFAILED fault before the restart to pin both halves. end_planned_stop() returns what it unmuted and what is to be announced as two lists, and the node logs both counts. The design doc scopes the single-thread argument to the paths that touch the ownership sets and names all three forms of process_clear that end an overlay; the message package README names the cluster listing in the parity set.

bburda added 10 commits September 7, 2026 16:16

msgs: add the planned-stop switch service pair

09b6022

SetPlannedStop declares or withdraws a planned stop and answers with the state the switch was in before the call; GetPlannedStop reads back the declaration, its reason, its declarer and when it started. Refs #656

docs: correct the message package's planned-stop description

fbcf0dc

It repeated the claim that a marked fault is never published as a FaultEvent, and listed a GetPlannedStop response that no longer matches the service. Refs #656

bburda self-assigned this Sep 8, 2026

bburda marked this pull request as ready for review September 10, 2026 20:09

mfaferek93 reviewed Sep 11, 2026

View reviewed changes

bburda added 2 commits September 11, 2026 13:56

Navigation Menu

Uh oh!

fault_manager: a planned-stop switch, so a weekend is not a wave of faults - #664

fault_manager: a planned-stop switch, so a weekend is not a wave of faults#664
bburda wants to merge 12 commits into
mainfrom
feat/planned-stop-switch

bburda commented Sep 8, 2026

Uh oh!

mfaferek93 Sep 11, 2026

Uh oh!

mfaferek93 Sep 11, 2026

Uh oh!

mfaferek93 Sep 11, 2026

Uh oh!

mfaferek93 Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bburda commented Sep 8, 2026

Pull Request

Summary

Issue

Type

Testing

Checklist

Uh oh!

mfaferek93 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

mfaferek93 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

mfaferek93 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

mfaferek93 Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants