From 34f2eb205c314376fa80fb4954b2f92e0a3e28c2 Mon Sep 17 00:00:00 2001 From: Alan Hanson Date: Wed, 18 Mar 2026 15:14:38 -0700 Subject: [PATCH 1/8] verify extent after LR --- downstairs/src/lib.rs | 2 +- downstairs/src/region.rs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/downstairs/src/lib.rs b/downstairs/src/lib.rs index 683e925a7..4374bb7df 100644 --- a/downstairs/src/lib.rs +++ b/downstairs/src/lib.rs @@ -1851,7 +1851,7 @@ impl ActiveConnection { dependencies, extent, } => { - let result = region.reopen_extent(*extent); + let result = region.reopen_extent_post_repair(*extent); debug!( self.log, "LiveReopen:{} extent {} deps:{:?} res:{}", diff --git a/downstairs/src/region.rs b/downstairs/src/region.rs index 01dce10d1..131427b56 100644 --- a/downstairs/src/region.rs +++ b/downstairs/src/region.rs @@ -465,6 +465,23 @@ impl Region { Ok(()) } + /** + * Re open an extent after live repair, then verify that all block + * data matches its stored hash. + */ + pub fn reopen_extent_post_repair( + &mut self, + eid: ExtentId, + ) -> Result<(), CrucibleError> { + self.reopen_extent(eid)?; + + info!(self.log, "Validating extent {} after live repair", eid); + let ExtentState::Opened(extent) = &self.extents[eid.0 as usize] else { + panic!("extent {eid} not open after reopen"); + }; + extent.validate() + } + pub fn close_extent( &mut self, eid: ExtentId, From 4161bc5d92e063f5781a743c174bc5d5f9ca3559 Mon Sep 17 00:00:00 2001 From: Alan Hanson Date: Fri, 20 Mar 2026 17:32:19 +0000 Subject: [PATCH 2/8] unwrap when validating extents after LR --- downstairs/src/region.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/downstairs/src/region.rs b/downstairs/src/region.rs index 131427b56..9707a851e 100644 --- a/downstairs/src/region.rs +++ b/downstairs/src/region.rs @@ -479,7 +479,7 @@ impl Region { let ExtentState::Opened(extent) = &self.extents[eid.0 as usize] else { panic!("extent {eid} not open after reopen"); }; - extent.validate() + extent.validate().unwrap() } pub fn close_extent( From df76b7950b592cfc29464cfb3bde7ab8130890d7 Mon Sep 17 00:00:00 2001 From: Alan Hanson Date: Fri, 20 Mar 2026 18:00:00 +0000 Subject: [PATCH 3/8] make panic message contain info I want --- downstairs/src/region.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/downstairs/src/region.rs b/downstairs/src/region.rs index 9707a851e..e037fd0b8 100644 --- a/downstairs/src/region.rs +++ b/downstairs/src/region.rs @@ -479,7 +479,10 @@ impl Region { let ExtentState::Opened(extent) = &self.extents[eid.0 as usize] else { panic!("extent {eid} not open after reopen"); }; - extent.validate().unwrap() + extent.validate().unwrap_or_else(|e| { + panic!("Extent {eid} failed validation after live repair: {e}") + }); + Ok(()) } pub fn close_extent( From b061cfe842392ce5e7c19f77cf3295600a6659b2 Mon Sep 17 00:00:00 2001 From: Alan Hanson Date: Fri, 27 Mar 2026 19:42:21 +0000 Subject: [PATCH 4/8] panic if write fails --- downstairs/src/extent_inner_raw.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/downstairs/src/extent_inner_raw.rs b/downstairs/src/extent_inner_raw.rs index fa3c04fbe..a835e6d79 100644 --- a/downstairs/src/extent_inner_raw.rs +++ b/downstairs/src/extent_inner_raw.rs @@ -397,6 +397,10 @@ impl ExtentInner for RawInner { let r = self.write_inner(write, &writes_to_skip); if r.is_err() { + panic!( + "This is what James warned us about, job:{} extent:{} blocks:{}", + job_id.0, self.extent_number.0, num_blocks + ); for i in 0..write.block_contexts.len() { if !writes_to_skip.contains(&i) { // Try to recompute the context slot from the file. If this From da3c75bcb4624bb51dadd4df7de9b8af56219feb Mon Sep 17 00:00:00 2001 From: Alan Hanson Date: Tue, 31 Mar 2026 17:00:24 +0000 Subject: [PATCH 5/8] Return error if writes fail in downstairs --- downstairs/src/extent_inner_raw.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/downstairs/src/extent_inner_raw.rs b/downstairs/src/extent_inner_raw.rs index a835e6d79..99becc7f8 100644 --- a/downstairs/src/extent_inner_raw.rs +++ b/downstairs/src/extent_inner_raw.rs @@ -397,10 +397,11 @@ impl ExtentInner for RawInner { let r = self.write_inner(write, &writes_to_skip); if r.is_err() { - panic!( - "This is what James warned us about, job:{} extent:{} blocks:{}", - job_id.0, self.extent_number.0, num_blocks - ); + // How can I log this? + // panic!( + // "This is what James warned us about, job:{} extent:{} blocks:{}", + // job_id.0, self.extent_number.0, num_blocks + // ); for i in 0..write.block_contexts.len() { if !writes_to_skip.contains(&i) { // Try to recompute the context slot from the file. If this @@ -410,6 +411,11 @@ impl ExtentInner for RawInner { self.recompute_slot_from_file(block).unwrap(); } } + // Maybe a different probe? That seems wrong. + cdt::extent__write__file__done!(|| { + (job_id.0, self.extent_number.0, num_blocks) + }); + r } else { // Now that writes have gone through, update active context slots for i in 0..write.block_contexts.len() { @@ -418,13 +424,12 @@ impl ExtentInner for RawInner { self.active_context.swap(write.offset.0 + i as u64); } } - } - - cdt::extent__write__file__done!(|| { - (job_id.0, self.extent_number.0, num_blocks) - }); + cdt::extent__write__file__done!(|| { + (job_id.0, self.extent_number.0, num_blocks) + }); - Ok(()) + Ok(()) + } } fn read( From b88445042a6a95b7dfead9670d20fce7607f78db Mon Sep 17 00:00:00 2001 From: Alan Hanson Date: Fri, 10 Apr 2026 08:59:32 -0700 Subject: [PATCH 6/8] Add full extent verification on startup. DO NOT MERGE THIS TO MAIN --- downstairs/src/region.rs | 48 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/downstairs/src/region.rs b/downstairs/src/region.rs index e037fd0b8..31b1a8d97 100644 --- a/downstairs/src/region.rs +++ b/downstairs/src/region.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Result, bail}; use futures::TryStreamExt; +use rayon::prelude::*; use tracing::instrument; @@ -321,6 +322,7 @@ impl Region { }; region.open_extents()?; + region.validate_extents()?; Ok(region) } @@ -375,6 +377,52 @@ impl Region { Ok(()) } + /// Validate every extent by hashing all blocks and checking + /// against stored on-disk hashes. Runs up to 20 extents in + /// parallel. + fn validate_extents(&self) -> Result<()> { + let pool = rayon::ThreadPoolBuilder::new() + .num_threads(20) + .build() + .expect("Failed to build validation thread pool"); + + let errors: Vec<_> = pool.install(|| { + self.extents + .par_iter() + .filter_map(|e| { + let extent = match e { + ExtentState::Opened(extent) => extent, + ExtentState::Closed => { + panic!("validate on closed extent!") + } + }; + + if let Err(err) = extent.validate() { + Some((extent.number, err)) + } else { + None + } + }) + .collect() + }); + + if !errors.is_empty() { + for (number, err) in &errors { + error!( + self.log, + "validation failed for extent {number}: {err}", + ); + } + bail!("Region failed to validate ({} extents bad)", errors.len()); + } + info!( + self.log, + "validated {} extents on startup", + self.extents.len(), + ); + Ok(()) + } + /// Creates `self.extent_count` extent files and opens them fn create_extents(&mut self, backend: Backend) -> Result<()> { let next_eid = self.extents.len() as u32; From ad5296d5fb951e3f3bc34b37444bd1d68a2ec866 Mon Sep 17 00:00:00 2001 From: Alan Hanson Date: Fri, 10 Apr 2026 14:19:51 -0700 Subject: [PATCH 7/8] skip sqlite extents so test will pass --- downstairs/src/extent.rs | 4 ++++ downstairs/src/region.rs | 11 +++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/downstairs/src/extent.rs b/downstairs/src/extent.rs index 1fab7b2cf..0a4c8e57d 100644 --- a/downstairs/src/extent.rs +++ b/downstairs/src/extent.rs @@ -505,6 +505,10 @@ impl Extent { self.inner.dirty().unwrap() } + pub fn read_only(&self) -> bool { + self.read_only + } + /// Close an extent, returning a tuple of `(gen, flush, dirty)` pub fn close(self) -> Result<(u64, u64, bool), CrucibleError> { let generation = self.inner.gen_number().unwrap(); diff --git a/downstairs/src/region.rs b/downstairs/src/region.rs index 31b1a8d97..aa85a9d33 100644 --- a/downstairs/src/region.rs +++ b/downstairs/src/region.rs @@ -377,9 +377,10 @@ impl Region { Ok(()) } - /// Validate every extent by hashing all blocks and checking + /// Validate every raw extent by hashing all blocks and checking /// against stored on-disk hashes. Runs up to 20 extents in - /// parallel. + /// parallel. SQLite-backed extents (read-only snapshots) are + /// skipped. fn validate_extents(&self) -> Result<()> { let pool = rayon::ThreadPoolBuilder::new() .num_threads(20) @@ -397,6 +398,12 @@ impl Region { } }; + // SQLite extents don't support validate and + // are only used for read-only snapshots. + if extent.read_only() { + return None; + } + if let Err(err) = extent.validate() { Some((extent.number, err)) } else { From 4447695ee834b474ed6363602aeea0a9bc2447c2 Mon Sep 17 00:00:00 2001 From: Alan Hanson Date: Mon, 13 Apr 2026 15:56:24 +0000 Subject: [PATCH 8/8] verify after repair removed --- downstairs/src/extent_inner_raw.rs | 5 ----- downstairs/src/lib.rs | 2 +- downstairs/src/region.rs | 20 -------------------- 3 files changed, 1 insertion(+), 26 deletions(-) diff --git a/downstairs/src/extent_inner_raw.rs b/downstairs/src/extent_inner_raw.rs index a32b0a215..1a9a052cd 100644 --- a/downstairs/src/extent_inner_raw.rs +++ b/downstairs/src/extent_inner_raw.rs @@ -416,11 +416,6 @@ impl ExtentInner for RawInner { let r = self.write_inner(write, &writes_to_skip); if r.is_err() { - // How can I log this? - // panic!( - // "This is what James warned us about, job:{} extent:{} blocks:{}", - // job_id.0, self.extent_number.0, num_blocks - // ); for i in 0..write.block_contexts.len() { if !writes_to_skip.contains(&i) { // Try to recompute the context slot from the file. If this diff --git a/downstairs/src/lib.rs b/downstairs/src/lib.rs index 4374bb7df..683e925a7 100644 --- a/downstairs/src/lib.rs +++ b/downstairs/src/lib.rs @@ -1851,7 +1851,7 @@ impl ActiveConnection { dependencies, extent, } => { - let result = region.reopen_extent_post_repair(*extent); + let result = region.reopen_extent(*extent); debug!( self.log, "LiveReopen:{} extent {} deps:{:?} res:{}", diff --git a/downstairs/src/region.rs b/downstairs/src/region.rs index aa85a9d33..c4ead0864 100644 --- a/downstairs/src/region.rs +++ b/downstairs/src/region.rs @@ -520,26 +520,6 @@ impl Region { Ok(()) } - /** - * Re open an extent after live repair, then verify that all block - * data matches its stored hash. - */ - pub fn reopen_extent_post_repair( - &mut self, - eid: ExtentId, - ) -> Result<(), CrucibleError> { - self.reopen_extent(eid)?; - - info!(self.log, "Validating extent {} after live repair", eid); - let ExtentState::Opened(extent) = &self.extents[eid.0 as usize] else { - panic!("extent {eid} not open after reopen"); - }; - extent.validate().unwrap_or_else(|e| { - panic!("Extent {eid} failed validation after live repair: {e}") - }); - Ok(()) - } - pub fn close_extent( &mut self, eid: ExtentId,