From 14824420138c1ada2c10762464cfc728b2d0419d Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 24 Apr 2026 13:28:14 -0700 Subject: [PATCH 1/2] [fm] minimum viable sitrep equality check This branch implements the sitrep equality check that I left unimplemented in #10258. Now, when a new sitrep is generated, we check if it represents the same situation as the parent sitrep, and if it does not, we will insert it into the database. This is done using a normal PartialEq implementation. I punted on implementing proper diffing support (see #10259), since it's not actually necessary to test if two sitreps are equivalent. This is an alternate design from the one I implemented in #10322. Rather than factoring out the sitrep's state into a new struct, which was a bit awkward, we just add a wrapper type for doing state equivalence comparisons. Closes #10322. --- nexus/src/app/background/tasks/fm_analysis.rs | 29 ++++++------ nexus/types/src/fm.rs | 47 +++++++++++++++++++ 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/nexus/src/app/background/tasks/fm_analysis.rs b/nexus/src/app/background/tasks/fm_analysis.rs index 3e0894734d4..78491c4d97d 100644 --- a/nexus/src/app/background/tasks/fm_analysis.rs +++ b/nexus/src/app/background/tasks/fm_analysis.rs @@ -263,21 +263,20 @@ impl FmAnalysis { }; } - // TODO(eliza): diff the sitrep against the parent, and return - // `Unchanged` if it's the same. - let unchanged = true; - if unchanged { - slog::info!( - &opctx.log, - "fault management analysis produced no changes from the \ - current sitrep" - ); - return status::AnalysisStatus { - start_time, - end_time, - report, - outcome: status::AnalysisOutcome::Unchanged, - }; + if let Some(parent) = inputs.parent_sitrep() { + if parent.compare_state() == sitrep { + slog::info!( + &opctx.log, + "fault management analysis produced no changes from the \ + current sitrep" + ); + return status::AnalysisStatus { + start_time, + end_time, + report, + outcome: status::AnalysisOutcome::Unchanged, + }; + } } let sitrep_id = sitrep.id(); diff --git a/nexus/types/src/fm.rs b/nexus/types/src/fm.rs index fe1c258e45b..861af7d2eba 100644 --- a/nexus/types/src/fm.rs +++ b/nexus/types/src/fm.rs @@ -94,6 +94,20 @@ impl Sitrep { case.support_bundles_requested.iter().map(move |req| (case, req)) }) } + + /// Returns a value which implements `PartialEq` and can be used to + /// test whether another sitrep represents the same state of the system as + /// this sitrep. + /// + /// This is distinct than comparing two `Sitrep`s directly using their + /// `PartialEq` implementation, as that comparison will test whether *all + /// fields of the sitreps* are exactly the same, while this comparison type + /// will compare equal to *any other sitrep which represents an identical + /// system state*. Two sitreps with different metadata (ID, creator ID, + /// timestamp, etc) can still represent identical states. + pub fn compare_state(&self) -> SitrepStateComparison<'_> { + SitrepStateComparison { sitrep: self } + } } /// Metadata describing a sitrep. @@ -146,6 +160,39 @@ pub struct SitrepMetadata { pub time_created: DateTime, } +pub struct SitrepStateComparison<'sitrep> { + sitrep: &'sitrep Sitrep, +} + +impl PartialEq for SitrepStateComparison<'_> { + fn eq(&self, other: &Sitrep) -> bool { + let Sitrep { + // Ignore the metadata, because two sitreps with different IDs, + // creator IDs, and timestamps can represent the same state. + metadata: _, + // Ignore the map of ereports by ID, as it's just an additional + // index of ereports that are already contained in the `cases` map. + // The state we want to compare is just whether or not the same + // ereports are assigned to the same cases, and it is unnecessary to + // also test that the by-ID indices have the same contents, since + // they always will if the two sitreps' cases contain the same + // ereports. + ereports_by_id: _, + cases, + } = self.sitrep; + + cases == &other.cases + // TO FUTURE GENERATIONS: if other top-level sitrep objects which + // represent the state of the system are added, compare them here! + } +} + +impl PartialEq for SitrepStateComparison<'_> { + fn eq(&self, other: &Self) -> bool { + self == other.sitrep + } +} + /// An entry in the sitrep version history. #[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)] pub struct SitrepVersion { From 37518ea5600441147fb98012bc8fd3cb33003b87 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Sat, 25 Apr 2026 10:13:17 -0700 Subject: [PATCH 2/2] rustfmt --- nexus/types/src/fm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/types/src/fm.rs b/nexus/types/src/fm.rs index 861af7d2eba..cb78bc05f6a 100644 --- a/nexus/types/src/fm.rs +++ b/nexus/types/src/fm.rs @@ -98,7 +98,7 @@ impl Sitrep { /// Returns a value which implements `PartialEq` and can be used to /// test whether another sitrep represents the same state of the system as /// this sitrep. - /// + /// /// This is distinct than comparing two `Sitrep`s directly using their /// `PartialEq` implementation, as that comparison will test whether *all /// fields of the sitreps* are exactly the same, while this comparison type