recovery: audit fixes — live at-risk excludes NonTried, latency-gated wedge, drop dead block counters

- Live located drilldown (sweep + patch progress snapshots) now excludes
  NonTried, matching the one-shot path. Including the unread remainder made
  main_at_risk_ms show ~full-movie at sweep start and melt to 0 as it
  progressed — unread is unknown, not damage.
- Wedge abort now requires the failure to be FAST (< WEDGE_FASTFAIL_MS,
  500ms) as well as wedge-family sense: a real uncorrectable sector on
  Hardware-error media spends ECC-recovery time before failing, so it no
  longer false-trips the fast-fail wedge abort. New regression test.
- Removed the always-zero blocks_attempted/read_ok/read_failed/
  unreadable_count from PatchOutcome + PatchLoopState (dead residue from
  the old grind loop; the HandlerScoreboard supersedes them) so the
  patch.done log no longer emits misleading zeros.
This commit is contained in:
Matthew Jackson
2026-07-01 12:07:08 -07:00
parent 9519628954
commit cf13838f12
4 changed files with 91 additions and 35 deletions
-6
View File
@@ -2930,9 +2930,6 @@ impl Disc {
tracing::info!( tracing::info!(
target: "freemkv::disc", target: "freemkv::disc",
phase = "patch_done", phase = "patch_done",
blocks_attempted = pr.blocks_attempted,
blocks_read_ok = pr.blocks_read_ok,
blocks_read_failed = pr.blocks_read_failed,
bytes_recovered = pr.bytes_recovered_this_pass, bytes_recovered = pr.bytes_recovered_this_pass,
halted = pr.halted, halted = pr.halted,
wedged_exit = pr.wedged_exit, wedged_exit = pr.wedged_exit,
@@ -3870,9 +3867,6 @@ pub struct PatchOutcome {
pub bytes_pending: u64, pub bytes_pending: u64,
pub bytes_recovered_this_pass: u64, pub bytes_recovered_this_pass: u64,
pub halted: bool, pub halted: bool,
pub blocks_attempted: u64,
pub blocks_read_ok: u64,
pub blocks_read_failed: u64,
pub wedged_exit: bool, pub wedged_exit: bool,
pub wedged_threshold: u64, pub wedged_threshold: u64,
} }
+5 -20
View File
@@ -148,10 +148,11 @@ pub(super) enum PatchItem {
/// Mapfile snapshot the sink republishes after every record so the /// Mapfile snapshot the sink republishes after every record so the
/// producer can drive its stall / progress logic without holding the /// producer can drive its stall / progress logic without holding the
/// mapfile lock for long. `bad_ranges` mirrors what /// mapfile lock for long. `bad_ranges` is the DAMAGE set
/// `Mapfile::ranges_with(&[NonTrimmed, Unreadable, NonScraped, NonTried])` /// (`NonTrimmed + Unreadable + NonScraped`) — NOT NonTried, which is the unread
/// would return — same set the pre-split patch loop computed inline /// remainder, not damage. Including NonTried inflated the live located drilldown
/// for the progress callback. /// (at-risk movie time + range count) with unread sectors; excluding it matches
/// the one-shot progress path.
pub(super) struct SharedPatchState { pub(super) struct SharedPatchState {
pub stats: MapStats, pub stats: MapStats,
pub bad_ranges: Vec<(u64, u64)>, pub bad_ranges: Vec<(u64, u64)>,
@@ -169,7 +170,6 @@ impl SharedPatchState {
SectorStatus::NonTrimmed, SectorStatus::NonTrimmed,
SectorStatus::Unreadable, SectorStatus::Unreadable,
SectorStatus::NonScraped, SectorStatus::NonScraped,
SectorStatus::NonTried,
]); ]);
bad_ranges.truncate(Self::MAX_BAD_RANGES); bad_ranges.truncate(Self::MAX_BAD_RANGES);
Self { Self {
@@ -614,10 +614,6 @@ pub(super) fn build_outcome(
tracing::info!( tracing::info!(
target: "freemkv::disc", target: "freemkv::disc",
phase = "patch.done", phase = "patch.done",
blocks_attempted = state.blocks_attempted,
blocks_read_ok = state.blocks_read_ok,
blocks_read_failed = state.blocks_read_failed,
unreadable_count = state.unreadable_count,
wedged_exit = state.wedged_exit, wedged_exit = state.wedged_exit,
halted = state.halted, halted = state.halted,
bytes_recovered = stats.bytes_good.saturating_sub(state.bytes_good_before), bytes_recovered = stats.bytes_good.saturating_sub(state.bytes_good_before),
@@ -635,9 +631,6 @@ pub(super) fn build_outcome(
bytes_pending: stats.bytes_pending, bytes_pending: stats.bytes_pending,
bytes_recovered_this_pass: stats.bytes_good.saturating_sub(state.bytes_good_before), bytes_recovered_this_pass: stats.bytes_good.saturating_sub(state.bytes_good_before),
halted: state.halted, halted: state.halted,
blocks_attempted: state.blocks_attempted,
blocks_read_ok: state.blocks_read_ok,
blocks_read_failed: state.blocks_read_failed,
wedged_exit: state.wedged_exit, wedged_exit: state.wedged_exit,
wedged_threshold, wedged_threshold,
} }
@@ -651,10 +644,6 @@ pub(super) struct PatchLoopState {
// Counters // Counters
pub halted: bool, pub halted: bool,
pub wedged_exit: bool, pub wedged_exit: bool,
pub blocks_attempted: u64,
pub blocks_read_ok: u64,
pub blocks_read_failed: u64,
pub unreadable_count: u64,
// Clock seam: the handler chain reads wall time through this rather than // Clock seam: the handler chain reads wall time through this rather than
// calling `Instant::now()` inline, so the per-handler deadline is driven by // calling `Instant::now()` inline, so the per-handler deadline is driven by
// an injectable clock and deterministic tests can wind it forward. // an injectable clock and deterministic tests can wind it forward.
@@ -697,10 +686,6 @@ impl PatchLoopState {
Self { Self {
halted: false, halted: false,
wedged_exit: false, wedged_exit: false,
blocks_attempted: 0,
blocks_read_ok: 0,
blocks_read_failed: 0,
unreadable_count: 0,
now, now,
bytes_good_before, bytes_good_before,
total_bytes, total_bytes,
+79 -8
View File
@@ -77,6 +77,15 @@ const UNPRODUCTIVE_YIELD: u32 = 4;
/// by `wedge_streak_persists_across_sections_for_tier1`. /// by `wedge_streak_persists_across_sections_for_tier1`.
const WEDGE_ABORT_STREAK: u32 = 16; const WEDGE_ABORT_STREAK: u32 = 16;
/// A wedge-family failure only counts toward [`WEDGE_ABORT_STREAK`] if it came
/// back faster than this — the fast-fail wedge rejects a CDB in <100ms with no
/// recovery attempt, whereas a genuine uncorrectable sector on Hardware-error
/// media spends real time on ECC recovery before failing. Gating on latency stops
/// slow, real damage that happens to report a Hardware sense from false-tripping
/// the wedge abort. Generous (500ms) so a slow bus adds margin without admitting
/// a true fast-fail.
const WEDGE_FASTFAIL_MS: u64 = 500;
/// Where a handler left the section after its bounded attempt. /// Where a handler left the section after its bounded attempt.
#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(super) enum HandlerOutcome { pub(super) enum HandlerOutcome {
@@ -199,6 +208,7 @@ fn read_span(
count >= 1 && pos % SECTOR == 0, count >= 1 && pos % SECTOR == 0,
"read_span requires a sector-aligned, >=1-sector span (pos={pos}, count={count})" "read_span requires a sector-aligned, >=1-sector span (pos={pos}, count={count})"
); );
let read_started = (ctx.now)();
let hit = match recovery_read(ctx.reader, ctx.decrypt_is_aacs, lba, count, buf, recovery) { let hit = match recovery_read(ctx.reader, ctx.decrypt_is_aacs, lba, count, buf, recovery) {
Ok(_) => { Ok(_) => {
ctx.sink.recovered(pos, &buf[..bytes]); ctx.sink.recovered(pos, &buf[..bytes]);
@@ -206,17 +216,24 @@ fn read_span(
} }
Err(e) if e.is_scsi_transport_failure() => ReadHit::Transport, Err(e) if e.is_scsi_transport_failure() => ReadHit::Transport,
Err(e) => { Err(e) => {
// Wedge watch: a Hardware / IllegalRequest sense is the drive's // Wedge watch: the drive's fast-fail wedge REJECTS every CDB in <100ms
// fast-fail signature. Count consecutive ones; a sustained run means // without attempting recovery, with a Hardware / IllegalRequest sense.
// the firmware wedged (rejecting every CDB), so escalate to Transport // Both signals are required to count toward the streak:
// and let the pass abort + spin-cycle rather than grind on. A medium // (1) wedge-family sense (Hardware / IllegalRequest), AND
// error or any success below resets the streak — real bad sectors // (2) the failure came back FAST (< WEDGE_FASTFAIL_MS).
// never trip it. // The latency gate is what keeps a genuine uncorrectable sector on
let wedge = e // Hardware-error media from false-tripping the wedge abort: a real
// ECC-recovery attempt takes far longer than a fast-fail rejection, so
// a SLOW Hardware-error is real damage (resets the streak, retried
// next pass), while only the fast rejections — the actual wedge —
// accumulate. A medium error or any success below also resets it.
let sense_is_wedge = e
.scsi_sense() .scsi_sense()
.map(|s| SenseFamily::from_sense_key(s.sense_key).is_wedge_family()) .map(|s| SenseFamily::from_sense_key(s.sense_key).is_wedge_family())
.unwrap_or(false); .unwrap_or(false);
if wedge { let elapsed = (ctx.now)().duration_since(read_started);
let fast_fail = elapsed.as_millis() < WEDGE_FASTFAIL_MS as u128;
if sense_is_wedge && fast_fail {
ctx.wedge_streak = ctx.wedge_streak.saturating_add(1); ctx.wedge_streak = ctx.wedge_streak.saturating_add(1);
if ctx.wedge_streak >= WEDGE_ABORT_STREAK { if ctx.wedge_streak >= WEDGE_ABORT_STREAK {
ReadHit::Transport ReadHit::Transport
@@ -1141,6 +1158,60 @@ mod tests {
); );
} }
#[test]
fn slow_hardware_error_media_does_not_false_trip_wedge_abort() {
// A genuine uncorrectable sector on Hardware-error media reports a
// wedge-FAMILY sense (IllegalRequest here) but comes back SLOW — the drive
// spent real time on ECC recovery before failing. That must NOT count
// toward the wedge abort (which targets the drive's <100ms fast-fail
// rejection). Each read here costs 600ms (> WEDGE_FASTFAIL_MS), so even a
// wholly-"wedge-sense" section never escalates to TransportFault — it just
// leaves the residue bad for the next pass, exactly like ordinary damage.
let (h, disc) = Harness::build(&[], None, Duration::from_millis(600));
let mut disc = disc;
disc.wedge = (0..1000u32).collect();
let mut sink = RecordSink::default();
let now = h.now_fn();
let mut ctx = HandlerCtx {
reader: &mut disc,
sink: &mut sink,
now: &now,
halt: None,
decrypt_is_aacs: false,
tick: None,
unproductive: 0,
wedge_streak: 0,
};
let mut bad = SubRanges::from_section(0, 1000 * SECTOR);
let mut handlers: Vec<Box<dyn SectionHandler>> = vec![
Box::new(Bisect),
Box::new(Jump),
Box::new(Linear {
reverse: true,
fast: true,
}),
Box::new(Linear {
reverse: false,
fast: true,
}),
];
let mut scoreboard = HandlerScoreboard::default();
// Long per-handler deadline so the deadline (not the wedge) is never the
// reason a handler stops — we're isolating the wedge-escalation decision.
let out = run_handlers(&mut ctx, &mut handlers, &mut bad, &mut scoreboard, |_| {
(h.now_fn())() + Duration::from_secs(3600)
});
assert_ne!(
out,
HandlerOutcome::TransportFault,
"slow (ECC-recovery) Hardware-error reads must NOT trip the fast-fail wedge abort"
);
assert_eq!(
ctx.wedge_streak, 0,
"slow wedge-family reads must not accumulate the streak"
);
}
#[test] #[test]
fn wedge_streak_persists_across_sections_for_tier1() { fn wedge_streak_persists_across_sections_for_tier1() {
// Tier 1 is only TWO handlers, so one wedged section builds at most // Tier 1 is only TWO handlers, so one wedged section builds at most
+7 -1
View File
@@ -190,11 +190,17 @@ impl Sink<WorkItem> for SweepSink {
} }
WorkItem::StatsRequest => { WorkItem::StatsRequest => {
let stats = self.map.stats(); let stats = self.map.stats();
// DAMAGE only — NOT NonTried. NonTried is the unread remainder
// ahead of the sweep head, not damage; including it made the live
// located drilldown (at-risk movie time + range count) treat the
// whole unread disc as confirmed damage, so at sweep start it
// showed ~full-movie at-risk and melted to 0 as the sweep
// progressed. Matches the one-shot progress path, which already
// excludes NonTried.
let bad_ranges = self.map.ranges_with(&[ let bad_ranges = self.map.ranges_with(&[
SectorStatus::NonTrimmed, SectorStatus::NonTrimmed,
SectorStatus::Unreadable, SectorStatus::Unreadable,
SectorStatus::NonScraped, SectorStatus::NonScraped,
SectorStatus::NonTried,
]); ]);
// Best-effort: drop on backpressure; producer's cache // Best-effort: drop on backpressure; producer's cache
// stays current enough. // stays current enough.