From 559070c7f3eb9aa01919126729c5522706f73a53 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 20 Sep 2026 11:51:05 +0800 Subject: [PATCH 1/7] refactor: collapse duplicated harness and engine implementations Benchmarks: consolidate the per-binary copies of env parsing, RSS sampling, file cleanup guards, write retry, latency sampling, and splitmix64 into shared benchmarks::config and the new benchmarks::harness. The copies had drifted; the unified versions keep knob names and report formats, and align a few error messages with the existing config style. Library: give the little-endian field codecs and the zeroed-field CRC32C pattern one home in the new codec module instead of four reimplementations (the shared reader also guards offset overflow, which two copies lacked). Collapse the identical POSIX and io_uring IoEngine impl blocks into trait default methods behind inner() and runtime_io_stats() accessors, share retry_interrupted with the uring engine, keep one is_read_pressure classifier, delegate the three global_slot computations to IndexPartitionRange, and move the three TestFile copies into fixtures so file-based tests clean up on failure. --- benchmarks/cache/main.rs | 177 ++----------- benchmarks/cache_soak/main.rs | 239 ++--------------- benchmarks/mixed_workloads/main.rs | 166 +++--------- benchmarks/recovery_scale/main.rs | 201 ++------------- benchmarks/region_index_turnover/main.rs | 16 +- benchmarks/src/config.rs | 91 ++++++- benchmarks/src/harness.rs | 240 ++++++++++++++++++ benchmarks/src/lib.rs | 1 + benchmarks/src/report.rs | 21 ++ cache2/src/codec.rs | 102 ++++++++ cache2/src/fixtures.rs | 70 ++++- cache2/src/io/backend.rs | 63 ++--- cache2/src/io/engine/mod.rs | 99 ++++++-- cache2/src/io/engine/posix.rs | 99 +------- cache2/src/io/engine/tests.rs | 89 ++----- cache2/src/io/engine/uring.rs | 114 +-------- cache2/src/lib.rs | 1 + cache2/src/region/index/storage/mod.rs | 207 ++++++--------- .../src/region/index/storage/page_format.rs | 24 +- cache2/src/region/mod.rs | 9 +- cache2/src/region/record/mod.rs | 65 +---- cache2/src/region/recovery/metadata.rs | 45 +--- cache2/src/region/recovery/mod.rs | 64 +---- cache2/src/region/runtime/mod.rs | 33 +-- cache2/src/region/runtime/shutdown_tests.rs | 79 +----- 25 files changed, 891 insertions(+), 1424 deletions(-) create mode 100644 benchmarks/src/harness.rs create mode 100644 cache2/src/codec.rs diff --git a/benchmarks/cache/main.rs b/benchmarks/cache/main.rs index bd4cf9f..34b90fb 100644 --- a/benchmarks/cache/main.rs +++ b/benchmarks/cache/main.rs @@ -14,25 +14,31 @@ use std::env; use std::fmt; -use std::fs; use std::hint::black_box; use std::io; use std::ops::Range; -use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; use std::time::Instant; -use std::time::SystemTime; -use std::time::UNIX_EPOCH; use asyncband::barrier::Barrier; +use benchmarks::config::env_bool; +use benchmarks::config::env_optional_f64; +use benchmarks::config::env_u32; +use benchmarks::config::env_usize; +use benchmarks::config::invalid; use benchmarks::config::io_engine_from_env; +use benchmarks::config::parse_io_mode; +use benchmarks::config::parse_l1_eviction_policy; use benchmarks::config::read_max_in_flight; +use benchmarks::harness::BenchFiles; +use benchmarks::harness::put_eventually; use benchmarks::report::JobReport; use benchmarks::report::LatencyHistogram; use benchmarks::report::RunReporter; use benchmarks::report::emit_cache_report; +use benchmarks::report::should_sample; use cache2::Cache; use cache2::CacheConfig; use cache2::CacheTier; @@ -56,7 +62,6 @@ const MAX_VALUE_BYTES: usize = REGION_BYTES - 64; const READ_RETRY_TIMEOUT: Duration = Duration::from_secs(1); const WRITE_RETRY_TIMEOUT: Duration = Duration::from_secs(10); const RETRY_DELAY: Duration = Duration::from_micros(50); -const WRITE_YIELD_RETRIES: usize = 8; struct BenchConfig { entries: usize, @@ -105,24 +110,8 @@ impl BenchConfig { env_usize("CACHE_BENCH_READ_LATENCY_SAMPLE_INTERVAL", 16)?; let clients = env_usize("CACHE_BENCH_CLIENTS", 8)?; let write_clients = env_usize("CACHE_BENCH_WRITE_CLIENTS", 4)?; - let io_mode = match env::var("CACHE_BENCH_IO_MODE") - .unwrap_or_else(|_| "buffered".to_owned()) - .as_str() - { - "buffered" => IoMode::Buffered, - "direct" => IoMode::Direct, - value => return Err(invalid(format!("unsupported I/O mode: {value}"))), - }; - let l1_eviction_policy = match env::var("CACHE_BENCH_L1_EVICTION") - .unwrap_or_else(|_| "clock".to_owned()) - .as_str() - { - "clock" => L1EvictionPolicy::Clock, - "s3-fifo" => L1EvictionPolicy::S3Fifo, - value => { - return Err(invalid(format!("unsupported L1 eviction policy: {value}"))); - } - }; + let io_mode = parse_io_mode("CACHE_BENCH_IO_MODE")?; + let l1_eviction_policy = parse_l1_eviction_policy("CACHE_BENCH_L1_EVICTION")?; let latency = |name| -> io::Result { Ok(match env_u32(name, 0)? { 0 => cache2::LatencyMode::Off, @@ -284,38 +273,6 @@ impl BenchConfig { } } -struct BenchFiles { - data: PathBuf, -} - -impl BenchFiles { - fn new(directory: &Path) -> Self { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos(); - Self { - data: directory.join(format!( - "cache2-bench-{}-{timestamp}.cache", - std::process::id() - )), - } - } -} - -impl Drop for BenchFiles { - fn drop(&mut self) { - for path in [ - self.data.clone(), - sidecar(&self.data, ".state"), - sidecar(&self.data, ".image"), - sidecar(&self.data, ".image.next"), - ] { - let _ = fs::remove_file(path); - } - } -} - struct Measurement { elapsed: Duration, operations: usize, @@ -375,7 +332,7 @@ fn run_benchmark() -> io::Result<()> { } async fn run(config: BenchConfig) -> io::Result<()> { - let files = BenchFiles::new(&config.directory); + let files = BenchFiles::new(&config.directory, "bench"); let l1_entry_eligible = benchmark_entry_is_l1_eligible(config.value_bytes); let cache_config = CacheConfig::new(config.storage_options().build()?, config.runtime_options())?; @@ -415,11 +372,11 @@ async fn run(config: BenchConfig) -> io::Result<()> { config.io_mode, config.stats.activity_counters, ); - println!("file={}", files.data.display()); + println!("file={}", files.data().display()); let cache = Arc::new( Cache::open( - &files.data, + files.data(), if initial_l1_bytes == config.l1_capacity_bytes { cache_config.clone() } else { @@ -477,7 +434,7 @@ async fn run(config: BenchConfig) -> io::Result<()> { let warm_close = started.elapsed(); report_latency("warm_close", "warm close", warm_close); - let cache = Arc::new(Cache::open(&files.data, cache_config.clone()).await?); + let cache = Arc::new(Cache::open(files.data(), cache_config.clone()).await?); if cache.startup_mode() != StartupMode::Warm { return Err(io::Error::other( "benchmark did not reopen from a clean image", @@ -659,7 +616,7 @@ async fn run(config: BenchConfig) -> io::Result<()> { drop(cache); let resident = if l1_entry_eligible { - let cache = Arc::new(Cache::open(&files.data, cache_config.clone()).await?); + let cache = Arc::new(Cache::open(files.data(), cache_config.clone()).await?); if cache.startup_mode() != StartupMode::Cold { return Err(io::Error::other( "fast-closed benchmark did not reopen empty", @@ -741,8 +698,12 @@ fn concurrent_writes( for ordinal in (client..entries).step_by(clients) { let key = benchmark_key(ordinal); value[..8].copy_from_slice(&(ordinal as u64).to_le_bytes()); - let (receipt, operation_attempts) = - put_eventually(&cache, black_box(&key), black_box(&value))?; + let (receipt, operation_attempts) = put_eventually( + &cache, + black_box(&key), + black_box(&value), + WRITE_RETRY_TIMEOUT, + )?; attempts = attempts.saturating_add(operation_attempts); throttled_writes = throttled_writes.saturating_add(usize::from(operation_attempts > 1)); @@ -804,17 +765,14 @@ async fn concurrent_reads( let mut latency = LatencyHistogram::default(); for ordinal in (client..operations).step_by(clients) { let key_ordinal = first_key + ordinal % key_count; - let started = (latency_sample_interval != 0 - && ordinal.is_multiple_of(latency_sample_interval)) - .then(Instant::now); + let started = + should_sample(ordinal as u64, latency_sample_interval).then(Instant::now); let value = if expected_tier == CacheTier::L2 { read_l2_once(&cache, key_ordinal).await? } else { Some(read_l1_eventually(&cache, key_ordinal, client).await?) }; - if let Some(started) = started { - latency.record(started.elapsed()); - } + latency.record_sampled(started); primary.operations += 1; if let Some(value) = value { primary.bytes += value.len() as u128; @@ -953,31 +911,6 @@ async fn read_l1_eventually(cache: &Cache, key_ordinal: usize, client: usize) -> } } -fn put_eventually(cache: &Cache, key: &[u8], value: &[u8]) -> io::Result<(u64, usize)> { - let deadline = Instant::now() + WRITE_RETRY_TIMEOUT; - let mut attempts = 0_usize; - loop { - attempts = attempts.saturating_add(1); - match cache.put(key, value) { - Ok(receipt) => return Ok((receipt, attempts)), - Err(error) if error.kind() == cache2::ErrorKind::Overloaded => { - if Instant::now() >= deadline { - return Err(io::Error::new( - io::ErrorKind::TimedOut, - "benchmark write did not enter bounded staging", - )); - } - if attempts <= WRITE_YIELD_RETRIES { - std::thread::yield_now(); - } else { - std::thread::sleep(RETRY_DELAY); - } - } - Err(error) => return Err(error.into()), - } - } -} - fn benchmark_key(ordinal: usize) -> [u8; 16] { let mut key = [0_u8; 16]; key[..8].copy_from_slice(b"cache2::"); @@ -1138,61 +1071,3 @@ fn require_minimum_rate(name: &str, measurement: &Measurement) -> io::Result<()> } Ok(()) } - -fn env_optional_f64(name: &str) -> io::Result> { - match env::var(name) { - Ok(value) => { - let parsed = value - .parse::() - .map_err(|_| invalid(format!("{name} must be a finite non-negative number")))?; - if !parsed.is_finite() || parsed < 0.0 { - return Err(invalid(format!( - "{name} must be a finite non-negative number" - ))); - } - Ok(Some(parsed)) - } - Err(env::VarError::NotPresent) => Ok(None), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn env_usize(name: &str, default: usize) -> io::Result { - match env::var(name) { - Ok(value) => value - .parse() - .map_err(|_| invalid(format!("{name} must be an unsigned integer"))), - Err(env::VarError::NotPresent) => Ok(default), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn env_u32(name: &str, default: u32) -> io::Result { - match env::var(name) { - Ok(value) => value - .parse() - .map_err(|_| invalid(format!("{name} must be an unsigned integer"))), - Err(env::VarError::NotPresent) => Ok(default), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn env_bool(name: &str, default: bool) -> io::Result { - match env::var(name) { - Ok(value) if value == "true" || value == "1" => Ok(true), - Ok(value) if value == "false" || value == "0" => Ok(false), - Ok(_) => Err(invalid(format!("{name} must be true, false, 1, or 0"))), - Err(env::VarError::NotPresent) => Ok(default), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn sidecar(path: &Path, suffix: &str) -> PathBuf { - let mut value = path.as_os_str().to_owned(); - value.push(suffix); - PathBuf::from(value) -} - -fn invalid(message: impl Into) -> io::Error { - io::Error::new(io::ErrorKind::InvalidInput, message.into()) -} diff --git a/benchmarks/cache_soak/main.rs b/benchmarks/cache_soak/main.rs index 387e98d..0d6db57 100644 --- a/benchmarks/cache_soak/main.rs +++ b/benchmarks/cache_soak/main.rs @@ -15,26 +15,34 @@ use std::cmp::min; use std::env; use std::fmt; -use std::fs; use std::io; -use std::mem::MaybeUninit; -use std::path::Path; use std::path::PathBuf; use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use std::time::Duration; use std::time::Instant; -use std::time::SystemTime; -use std::time::UNIX_EPOCH; +use benchmarks::config::env_bool; +use benchmarks::config::env_u32; +use benchmarks::config::env_u64; +use benchmarks::config::env_usize; +use benchmarks::config::env_usize_list; +use benchmarks::config::invalid; use benchmarks::config::io_engine_from_env; +use benchmarks::config::parse_io_mode; +use benchmarks::config::parse_l1_eviction_policy; use benchmarks::config::reclaim_max_in_flight; +use benchmarks::harness::BenchFiles; +use benchmarks::harness::current_rss_bytes; +use benchmarks::harness::peak_rss_bytes; +use benchmarks::harness::splitmix64; use benchmarks::report::AtomicLatencyHistogram; use benchmarks::report::JobReport; use benchmarks::report::LatencyHistogram; use benchmarks::report::RunReporter; use benchmarks::report::emit_cache_report; +use benchmarks::report::should_sample; use cache2::Cache; use cache2::CacheConfig; use cache2::CacheHealth; @@ -211,68 +219,6 @@ impl SoakConfig { } } -struct SoakFiles { - data: PathBuf, - cleanup_on_drop: AtomicBool, -} - -impl SoakFiles { - fn new(directory: &Path) -> Self { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos(); - Self { - data: directory.join(format!( - "cache2-soak-{}-{timestamp}.cache", - std::process::id() - )), - cleanup_on_drop: AtomicBool::new(false), - } - } - - fn mark_success(&self) { - self.cleanup_on_drop.store(true, Ordering::Release); - } - - fn logical_bytes(&self) -> io::Result { - [ - self.data.clone(), - sidecar(&self.data, ".state"), - sidecar(&self.data, ".image"), - sidecar(&self.data, ".image.next"), - ] - .into_iter() - .try_fold(0_u64, |total, path| match fs::metadata(path) { - Ok(metadata) => total - .checked_add(metadata.len()) - .ok_or_else(|| invalid("logical disk byte count overflow")), - Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(total), - Err(error) => Err(error), - }) - } -} - -impl Drop for SoakFiles { - fn drop(&mut self) { - if !self.cleanup_on_drop.load(Ordering::Acquire) { - eprintln!( - "soak artifacts preserved after failure: data={}", - self.data.display() - ); - return; - } - for path in [ - self.data.clone(), - sidecar(&self.data, ".state"), - sidecar(&self.data, ".image"), - sidecar(&self.data, ".image.next"), - ] { - let _ = fs::remove_file(path); - } - } -} - #[derive(Default)] struct SoakCounters { writes: AtomicU64, @@ -352,7 +298,7 @@ fn run_benchmark() -> io::Result<()> { .thread_name("cache2-soak") .enable_time() .build()?; - let files = SoakFiles::new(&config.directory); + let mut files = BenchFiles::preserved_on_failure(&config.directory, "soak"); let storage = config.storage_options().build()?; let peak_disk_bytes = storage.peak_disk_bytes(); let cache_config = CacheConfig::new(storage, config.runtime_options())?; @@ -420,7 +366,7 @@ fn run_benchmark() -> io::Result<()> { peak_disk_bytes, config.rss_slack_bytes, config.rss_reopen_allowance_bytes, - files.data.display(), + files.data().display(), ); std::thread::scope(|scope| -> io::Result<()> { @@ -621,10 +567,10 @@ fn init_logforth() -> io::Result<()> { fn open_cache( runtime: &tokio::runtime::Runtime, - files: &SoakFiles, + files: &BenchFiles, config: &CacheConfig, ) -> io::Result { - Ok(runtime.block_on(Cache::open(&files.data, config.clone()))?) + Ok(runtime.block_on(Cache::open(files.data(), config.clone()))?) } fn populate_for_warm_reopen( @@ -703,11 +649,11 @@ fn run_writer( let put_started = should_sample(ordinal, config.latency_sample_interval).then(Instant::now); match cache.put(key, &value[..value_bytes]) { Ok(_) => { - record_latency(&counters.put_latency, put_started); + counters.put_latency.record_sampled(put_started); counters.writes.fetch_add(1, Ordering::Relaxed); } Err(error) if error.kind() == cache2::ErrorKind::Overloaded => { - record_latency(&counters.put_latency, put_started); + counters.put_latency.record_sampled(put_started); counters.write_rejections.fetch_add(1, Ordering::Relaxed); std::thread::sleep(OVERLOAD_DELAY); continue; @@ -721,11 +667,11 @@ fn run_writer( should_sample(delete_ordinal, config.latency_sample_interval).then(Instant::now); match cache.delete(key) { Ok(_) => { - record_latency(&counters.delete_latency, delete_started); + counters.delete_latency.record_sampled(delete_started); counters.deletes.fetch_add(1, Ordering::Relaxed); } Err(error) if error.kind() == cache2::ErrorKind::Overloaded => { - record_latency(&counters.delete_latency, delete_started); + counters.delete_latency.record_sampled(delete_started); counters.delete_rejections.fetch_add(1, Ordering::Relaxed); std::thread::sleep(OVERLOAD_DELAY); } @@ -762,7 +708,7 @@ fn run_reader( let get_started = should_sample(ordinal, config.latency_sample_interval).then(Instant::now); match runtime.block_on(cache.get(&key))? { Some(observed) => { - record_latency(&counters.get_latency, get_started); + counters.get_latency.record_sampled(get_started); let latest = expected[sampled].load(Ordering::SeqCst); let stale = validate_observed( sampled, @@ -777,7 +723,7 @@ fn run_reader( } } None => { - record_latency(&counters.get_latency, get_started); + counters.get_latency.record_sampled(get_started); counters.misses.fetch_add(1, Ordering::Relaxed); } } @@ -863,16 +809,13 @@ fn validate_observed( } fn mixed_value_index(sequence: u64, value_size_count: u64) -> io::Result { - let mut mixed = sequence.wrapping_add(0x9e37_79b9_7f4a_7c15); - mixed = (mixed ^ (mixed >> 30)).wrapping_mul(0xbf58_476d_1ce4_e5b9); - mixed = (mixed ^ (mixed >> 27)).wrapping_mul(0x94d0_49bb_1331_11eb); - mixed ^= mixed >> 31; + let mixed = splitmix64(sequence.wrapping_add(0x9e37_79b9_7f4a_7c15)); usize::try_from(mixed % value_size_count).map_err(|_| invalid("value-size index exceeds usize")) } fn resource_sample( cache: &Cache, - files: &SoakFiles, + files: &BenchFiles, peak_disk_bytes: u64, rss_slack_bytes: usize, rss_reopen_allowance_bytes: usize, @@ -1096,140 +1039,8 @@ fn report_sample( ); } -fn should_sample(ordinal: u64, interval: usize) -> bool { - interval != 0 && ordinal.is_multiple_of(interval as u64) -} - -fn record_latency(histogram: &AtomicLatencyHistogram, started: Option) { - if let Some(started) = started { - histogram.record(started.elapsed()); - } -} - fn pace(interval: Duration) { if !interval.is_zero() { std::thread::sleep(interval); } } - -#[cfg(unix)] -fn peak_rss_bytes() -> u64 { - let mut usage = MaybeUninit::::zeroed(); - // SAFETY: `usage` points to writable storage for one `rusage` value. - if unsafe { libc::getrusage(libc::RUSAGE_SELF, usage.as_mut_ptr()) } != 0 { - return 0; - } - // SAFETY: a successful getrusage initialized the complete value. - let usage = unsafe { usage.assume_init() }; - #[cfg(any(target_os = "macos", target_os = "ios"))] - { - u64::try_from(usage.ru_maxrss).unwrap_or(0) - } - #[cfg(not(any(target_os = "macos", target_os = "ios")))] - { - u64::try_from(usage.ru_maxrss) - .unwrap_or(0) - .saturating_mul(1024) - } -} - -#[cfg(target_os = "linux")] -fn current_rss_bytes() -> io::Result { - let status = fs::read_to_string("/proc/self/status")?; - let kib = status - .lines() - .find_map(|line| line.strip_prefix("VmRSS:")) - .and_then(|value| value.split_ascii_whitespace().next()) - .and_then(|value| value.parse::().ok()) - .ok_or_else(|| io::Error::other("cannot read current RSS from /proc/self/status"))?; - kib.checked_mul(1024) - .ok_or_else(|| io::Error::other("current RSS byte count overflow")) -} - -#[cfg(not(target_os = "linux"))] -fn current_rss_bytes() -> io::Result { - Ok(0) -} - -#[cfg(not(unix))] -fn peak_rss_bytes() -> u64 { - 0 -} - -fn env_u64(name: &str, default: u64) -> io::Result { - match env::var(name) { - Ok(value) => value - .parse() - .map_err(|_| invalid(format!("{name} must be an unsigned integer"))), - Err(env::VarError::NotPresent) => Ok(default), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn env_usize(name: &str, default: usize) -> io::Result { - env_u64(name, default as u64).and_then(|value| { - usize::try_from(value).map_err(|_| invalid(format!("{name} does not fit usize"))) - }) -} - -fn env_usize_list(name: &str, default: &[usize]) -> io::Result> { - match env::var(name) { - Ok(value) => value - .split(',') - .map(|item| { - item.parse::() - .map_err(|_| invalid(format!("{name} must be comma-separated integers"))) - }) - .collect::>>() - .map(Vec::into_boxed_slice), - Err(env::VarError::NotPresent) => Ok(default.to_vec().into_boxed_slice()), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn env_u32(name: &str, default: u32) -> io::Result { - env_u64(name, u64::from(default)) - .and_then(|value| u32::try_from(value).map_err(|_| invalid(format!("{name} exceeds u32")))) -} - -fn env_bool(name: &str, default: bool) -> io::Result { - match env::var(name) { - Ok(value) if value == "true" || value == "1" => Ok(true), - Ok(value) if value == "false" || value == "0" => Ok(false), - Ok(_) => Err(invalid(format!("{name} must be true, false, 1, or 0"))), - Err(env::VarError::NotPresent) => Ok(default), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn parse_io_mode(name: &str) -> io::Result { - match env::var(name) - .unwrap_or_else(|_| "buffered".to_owned()) - .as_str() - { - "buffered" => Ok(IoMode::Buffered), - "direct" => Ok(IoMode::Direct), - value => Err(invalid(format!("unsupported I/O mode: {value}"))), - } -} - -fn parse_l1_eviction_policy(name: &str) -> io::Result { - match env::var(name) - .unwrap_or_else(|_| "clock".to_owned()) - .as_str() - { - "clock" => Ok(L1EvictionPolicy::Clock), - "s3-fifo" => Ok(L1EvictionPolicy::S3Fifo), - value => Err(invalid(format!("unsupported L1 eviction policy: {value}"))), - } -} - -fn sidecar(path: &Path, suffix: &str) -> PathBuf { - let mut value = path.as_os_str().to_owned(); - value.push(suffix); - PathBuf::from(value) -} - -fn invalid(message: impl Into) -> io::Error { - io::Error::new(io::ErrorKind::InvalidInput, message.into()) -} diff --git a/benchmarks/mixed_workloads/main.rs b/benchmarks/mixed_workloads/main.rs index f99ce8f..ffd6e43 100644 --- a/benchmarks/mixed_workloads/main.rs +++ b/benchmarks/mixed_workloads/main.rs @@ -15,27 +15,33 @@ use std::env; use std::f64::consts::TAU; use std::fmt; -use std::fs; use std::hint::black_box; use std::io; -use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use std::time::Duration; use std::time::Instant; -use std::time::SystemTime; -use std::time::UNIX_EPOCH; use asyncband::barrier::Barrier; +use benchmarks::config::env_optional_usize; +use benchmarks::config::env_u32; +use benchmarks::config::env_u64; +use benchmarks::config::env_usize; +use benchmarks::config::invalid; use benchmarks::config::io_engine_from_env; +use benchmarks::config::parse_io_mode; +use benchmarks::config::parse_l1_eviction_policy; use benchmarks::config::read_max_in_flight; use benchmarks::config::reclaim_max_in_flight; +use benchmarks::harness::BenchFiles; +use benchmarks::harness::splitmix64; use benchmarks::report::JobReport; use benchmarks::report::LatencyHistogram; use benchmarks::report::RunReporter; use benchmarks::report::emit_cache_report; +use benchmarks::report::should_sample; use cache2::Cache; use cache2::CacheConfig; use cache2::CacheHealth; @@ -157,8 +163,8 @@ impl Scenario { } fn key_size(self, seed: u64, key_index: usize) -> usize { - let first = mixed(seed ^ key_index as u64 ^ 0x1319_8a2e_0370_7344); - let second = mixed(first); + let first = splitmix64(seed ^ key_index as u64 ^ 0x1319_8a2e_0370_7344); + let second = splitmix64(first); let sampled = match self { Self::Mixed | Self::NegativeLookup => { sample_piecewise(&MIXED_KEY_BOUNDS, &MIXED_KEY_WEIGHTS, first, second) @@ -174,8 +180,8 @@ impl Scenario { } fn value_size(self, seed: u64, key_index: usize) -> usize { - let first = mixed(seed ^ key_index as u64 ^ 0xbe54_66cf_34e9_0c6c); - let second = mixed(first); + let first = splitmix64(seed ^ key_index as u64 ^ 0xbe54_66cf_34e9_0c6c); + let second = splitmix64(first); let sampled = match self { Self::Mixed | Self::NegativeLookup => { sample_piecewise(&MIXED_VALUE_BOUNDS, &MIXED_VALUE_WEIGHTS, first, second) @@ -231,22 +237,8 @@ impl HarnessOptions { let io_engine = io_engine_from_env("CACHE_WORKLOAD")?; let latency_sample_interval = env_usize("CACHE_WORKLOAD_LATENCY_SAMPLE_INTERVAL", 16)?; let seed = env_u64("CACHE_WORKLOAD_SEED", DEFAULT_SEED)?; - let io_mode = match env::var("CACHE_WORKLOAD_IO_MODE") - .unwrap_or_else(|_| "buffered".to_owned()) - .as_str() - { - "buffered" => IoMode::Buffered, - "direct" => IoMode::Direct, - value => return Err(invalid(format!("unsupported I/O mode: {value}"))), - }; - let l1_eviction_policy = match env::var("CACHE_WORKLOAD_L1_EVICTION") - .unwrap_or_else(|_| "clock".to_owned()) - .as_str() - { - "clock" => L1EvictionPolicy::Clock, - "s3-fifo" => L1EvictionPolicy::S3Fifo, - value => return Err(invalid(format!("unsupported L1 eviction policy: {value}"))), - }; + let io_mode = parse_io_mode("CACHE_WORKLOAD_IO_MODE")?; + let l1_eviction_policy = parse_l1_eviction_policy("CACHE_WORKLOAD_L1_EVICTION")?; let directory = env::var_os("CACHE_WORKLOAD_DIR") .map(PathBuf::from) .unwrap_or_else(env::temp_dir); @@ -401,39 +393,6 @@ impl ScenarioConfig { } } -struct BenchFiles { - data: PathBuf, -} - -impl BenchFiles { - fn new(directory: &Path, scenario: Scenario) -> Self { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos(); - Self { - data: directory.join(format!( - "cache2-mixed-workload-{}-{}-{timestamp}.cache", - scenario.slug(), - std::process::id() - )), - } - } -} - -impl Drop for BenchFiles { - fn drop(&mut self) { - for file in [ - self.data.clone(), - sidecar(&self.data, ".state"), - sidecar(&self.data, ".image"), - sidecar(&self.data, ".image.next"), - ] { - let _ = fs::remove_file(file); - } - } -} - #[derive(Default)] struct WorkloadResult { gets: u64, @@ -517,7 +476,7 @@ impl DeterministicRng { fn next_u64(&mut self) -> u64 { self.state = self.state.wrapping_add(RNG_GAMMA); - mixed(self.state) + splitmix64(self.state) } fn bounded(&mut self, upper: u64) -> u64 { @@ -590,10 +549,13 @@ async fn run_scenario_inner(config: ScenarioConfig) -> io::Result<()> { config.io_mode, ); - let files = BenchFiles::new(&config.directory, scenario); + let files = BenchFiles::new( + &config.directory, + &format!("mixed-workload-{}", scenario.slug()), + ); let storage = config.storage_options().build()?; let cache_config = CacheConfig::new(storage, config.runtime_options())?; - let cache = Arc::new(Cache::open(&files.data, cache_config).await?); + let cache = Arc::new(Cache::open(files.data(), cache_config).await?); let expected: Arc<[AtomicU64]> = (0..config.key_count) .map(|_| AtomicU64::new(0)) .collect::>() @@ -666,7 +628,7 @@ async fn run_worker( expected: &[AtomicU64], config: &ScenarioConfig, ) -> io::Result { - let worker_seed = mixed( + let worker_seed = splitmix64( config.seed ^ config.scenario.seed_salt() ^ (worker_id as u64).wrapping_mul(RNG_GAMMA), ); let mut rng = DeterministicRng::new(worker_seed); @@ -687,14 +649,19 @@ async fn run_worker( .and_then(|base| base.checked_add(operation_index)) .ok_or_else(|| invalid("worker operation ordinal overflow"))?; let operation = config.scenario.operation(rng.bounded(100)); - let sample_latency = should_sample(operation, &result, config.latency_sample_interval); + let completed = match operation { + Operation::Get => result.gets, + Operation::Set => result.sets, + Operation::Delete => result.deletes, + }; + let sample_latency = should_sample(completed, config.latency_sample_interval); if config.scenario == Scenario::NegativeLookup { write_negative_lookup_key(&mut miss_key, config.seed, global_index as u64); result.gets = result.gets.saturating_add(1); let started = sample_latency.then(Instant::now); let outcome = cache.get(miss_key).await; - record_sample(&mut result.get_latency, started); + result.get_latency.record_sampled(started); match outcome { Ok(Some(_)) => { return Err(io::Error::new( @@ -721,7 +688,7 @@ async fn run_worker( result.gets = result.gets.saturating_add(1); let started = sample_latency.then(Instant::now); let outcome = cache.get(key).await; - record_sample(&mut result.get_latency, started); + result.get_latency.record_sampled(started); match outcome { Ok(Some(observed)) => { let latest = expected[key_index].load(Ordering::SeqCst); @@ -733,7 +700,7 @@ async fn run_worker( result.served_value_bytes = result .served_value_bytes .saturating_add(observed.len() as u64); - result.checksum ^= black_box(mixed( + result.checksum ^= black_box(splitmix64( key_index as u64 ^ observed_version.rotate_left(17) ^ observed.len() as u64, @@ -759,7 +726,7 @@ async fn run_worker( .saturating_add(value_size as u64); let started = sample_latency.then(Instant::now); let outcome = cache.put(key, &value[..value_size]); - record_sample(&mut result.set_latency, started); + result.set_latency.record_sampled(started); match outcome { Ok(_) => { result.set_accepted = result.set_accepted.saturating_add(1); @@ -777,7 +744,7 @@ async fn run_worker( result.deletes = result.deletes.saturating_add(1); let started = sample_latency.then(Instant::now); let outcome = cache.delete(key); - record_sample(&mut result.delete_latency, started); + result.delete_latency.record_sampled(started); match outcome { Ok(_) => { result.delete_accepted = result.delete_accepted.saturating_add(1); @@ -902,22 +869,7 @@ fn validate_value(key_index: usize, value: &[u8], latest: u64) -> io::Result u8 { - mixed(key_index ^ version.rotate_left(29) ^ length as u64) as u8 -} - -fn record_sample(histogram: &mut LatencyHistogram, started: Option) { - if let Some(started) = started { - histogram.record(started.elapsed()); - } -} - -fn should_sample(operation: Operation, result: &WorkloadResult, interval: usize) -> bool { - interval != 0 - && match operation { - Operation::Get => result.gets.is_multiple_of(interval as u64), - Operation::Set => result.sets.is_multiple_of(interval as u64), - Operation::Delete => result.deletes.is_multiple_of(interval as u64), - } + splitmix64(key_index ^ version.rotate_left(29) ^ length as u64) as u8 } fn validate_snapshot(result: &WorkloadResult, snapshot: &CacheSnapshot) -> io::Result<()> { @@ -1184,44 +1136,6 @@ fn default_managed_memory_limit( .ok_or_else(|| invalid("managed-memory estimate rounding overflow")) } -fn mixed(mut value: u64) -> u64 { - value = (value ^ (value >> 30)).wrapping_mul(0xbf58_476d_1ce4_e5b9); - value = (value ^ (value >> 27)).wrapping_mul(0x94d0_49bb_1331_11eb); - value ^ (value >> 31) -} - -fn env_optional_usize(name: &str) -> io::Result> { - match env::var(name) { - Ok(value) => value - .parse::() - .map(Some) - .map_err(|_| invalid(format!("{name} must be an unsigned integer"))), - Err(env::VarError::NotPresent) => Ok(None), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn env_u64(name: &str, default: u64) -> io::Result { - match env::var(name) { - Ok(value) => value - .parse() - .map_err(|_| invalid(format!("{name} must be an unsigned integer"))), - Err(env::VarError::NotPresent) => Ok(default), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn env_usize(name: &str, default: usize) -> io::Result { - env_u64(name, default as u64).and_then(|value| { - usize::try_from(value).map_err(|_| invalid(format!("{name} does not fit usize"))) - }) -} - -fn env_u32(name: &str, default: u32) -> io::Result { - env_u64(name, u64::from(default)) - .and_then(|value| u32::try_from(value).map_err(|_| invalid(format!("{name} exceeds u32")))) -} - fn mib_to_usize(name: &str, value: usize) -> io::Result { value .checked_mul(MIB) @@ -1235,16 +1149,6 @@ fn mib_to_u64(name: &str, value: usize) -> io::Result { .ok_or_else(|| invalid(format!("{name} is too large"))) } -fn sidecar(path: &Path, suffix: &str) -> PathBuf { - let mut value = path.as_os_str().to_owned(); - value.push(suffix); - PathBuf::from(value) -} - -fn invalid(message: impl Into) -> io::Error { - io::Error::new(io::ErrorKind::InvalidInput, message.into()) -} - fn invalid_data(message: impl Into) -> io::Error { io::Error::new(io::ErrorKind::InvalidData, message.into()) } diff --git a/benchmarks/recovery_scale/main.rs b/benchmarks/recovery_scale/main.rs index 3b50d64..8d33746 100644 --- a/benchmarks/recovery_scale/main.rs +++ b/benchmarks/recovery_scale/main.rs @@ -14,16 +14,19 @@ use std::env; use std::fmt; -use std::fs; use std::io; -use std::mem::MaybeUninit; -use std::path::Path; use std::path::PathBuf; use std::time::Duration; use std::time::Instant; -use std::time::SystemTime; -use std::time::UNIX_EPOCH; +use benchmarks::config::env_u64; +use benchmarks::config::env_usize; +use benchmarks::config::invalid; +use benchmarks::config::reject_renamed_env; +use benchmarks::harness::BenchFiles; +use benchmarks::harness::current_rss_bytes; +use benchmarks::harness::peak_rss_bytes; +use benchmarks::harness::put_eventually; use benchmarks::report::JobReport; use benchmarks::report::RunReporter; use cache2::Cache; @@ -50,7 +53,7 @@ struct ScaleConfig { impl ScaleConfig { fn from_env() -> io::Result { - benchmarks::config::reject_renamed_env("CACHE_RECOVERY")?; + reject_renamed_env("CACHE_RECOVERY")?; let expected_entries = env_usize("CACHE_RECOVERY_EXPECTED_ENTRIES", 1_000_000)?; let capacity_bytes = env_u64("CACHE_RECOVERY_CAPACITY_MIB", 256)? .checked_mul(MIB as u64) @@ -106,87 +109,6 @@ impl ScaleConfig { } } -struct ScaleFiles { - data: PathBuf, - cleanup_on_drop: bool, -} - -impl ScaleFiles { - fn new(directory: &Path) -> Self { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_nanos(); - Self { - data: directory.join(format!( - "cache2-recovery-scale-{}-{timestamp}.cache", - std::process::id() - )), - cleanup_on_drop: false, - } - } - - fn mark_success(&mut self) { - self.cleanup_on_drop = true; - } - - fn logical_bytes(&self) -> io::Result { - self.paths() - .into_iter() - .try_fold(0_u64, |total, path| match fs::metadata(path) { - Ok(metadata) => total - .checked_add(metadata.len()) - .ok_or_else(|| invalid("logical file size overflow")), - Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(total), - Err(error) => Err(error), - }) - } - - #[cfg(unix)] - fn allocated_bytes(&self) -> io::Result { - use std::os::unix::fs::MetadataExt; - - self.paths() - .into_iter() - .try_fold(0_u64, |total, path| match fs::metadata(path) { - Ok(metadata) => total - .checked_add(metadata.blocks().saturating_mul(512)) - .ok_or_else(|| invalid("allocated file size overflow")), - Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(total), - Err(error) => Err(error), - }) - } - - #[cfg(not(unix))] - fn allocated_bytes(&self) -> io::Result { - self.logical_bytes() - } - - fn paths(&self) -> [PathBuf; 4] { - [ - self.data.clone(), - sidecar(&self.data, ".state"), - sidecar(&self.data, ".image"), - sidecar(&self.data, ".image.next"), - ] - } -} - -impl Drop for ScaleFiles { - fn drop(&mut self) { - if !self.cleanup_on_drop { - eprintln!( - "recovery-scale artifacts preserved after failure: data={}", - self.data.display() - ); - return; - } - for path in self.paths() { - let _ = fs::remove_file(path); - } - } -} - fn main() -> io::Result<()> { let reporter = RunReporter::start("recovery_scale", None); let result = run_benchmark(); @@ -208,7 +130,7 @@ fn run_benchmark() -> io::Result<()> { } async fn run(config: ScaleConfig) -> io::Result<()> { - let mut files = ScaleFiles::new(&config.directory); + let mut files = BenchFiles::preserved_on_failure(&config.directory, "recovery-scale"); let storage = config.storage_options().build()?; let cache_config = CacheConfig::new(storage.clone(), config.runtime_options())?; let peak_disk_bytes = storage.peak_disk_bytes(); @@ -226,7 +148,7 @@ async fn run(config: ScaleConfig) -> io::Result<()> { ); let opened = Instant::now(); - let cache = Cache::open(&files.data, cache_config.clone()).await?; + let cache = Cache::open(files.data(), cache_config.clone()).await?; emit("fresh_open", "control", opened.elapsed(), 1, 0); require_startup(cache.startup_mode(), StartupMode::Cold)?; let resources = cache.snapshot()?; @@ -242,7 +164,7 @@ async fn run(config: ScaleConfig) -> io::Result<()> { let populated = Instant::now(); for (ordinal, key) in keys.iter().enumerate() { value[..8].copy_from_slice(&(ordinal as u64).to_le_bytes()); - put_eventually(&cache, key, &value)?; + put_eventually(&cache, key, &value, WRITE_RETRY_TIMEOUT)?; } cache.drain().await?; emit( @@ -259,7 +181,7 @@ async fn run(config: ScaleConfig) -> io::Result<()> { emit_sizes(&files, peak_disk_bytes)?; let reopened = Instant::now(); - let cache = Cache::open(&files.data, cache_config.clone()).await?; + let cache = Cache::open(files.data(), cache_config.clone()).await?; emit("warm_open", "control", reopened.elapsed(), 1, 0); require_startup(cache.startup_mode(), StartupMode::Warm)?; verify_sentinels(&cache, &keys, config.value_bytes).await?; @@ -270,7 +192,7 @@ async fn run(config: ScaleConfig) -> io::Result<()> { emit_sizes(&files, peak_disk_bytes)?; let reopened = Instant::now(); - let cache = Cache::open(&files.data, cache_config.clone()).await?; + let cache = Cache::open(files.data(), cache_config.clone()).await?; emit("second_warm_open", "control", reopened.elapsed(), 1, 0); require_startup(cache.startup_mode(), StartupMode::Warm)?; verify_sentinels(&cache, &keys, config.value_bytes).await?; @@ -307,25 +229,6 @@ async fn verify_sentinels(cache: &Cache, keys: &[[u8; 16]], value_bytes: usize) Ok(()) } -fn put_eventually(cache: &Cache, key: &[u8], value: &[u8]) -> io::Result<()> { - let deadline = Instant::now() + WRITE_RETRY_TIMEOUT; - loop { - match cache.put(key, value) { - Ok(_) => return Ok(()), - Err(error) if error.kind() == cache2::ErrorKind::Overloaded => { - if Instant::now() >= deadline { - return Err(io::Error::new( - io::ErrorKind::TimedOut, - "recovery benchmark write did not enter bounded staging", - )); - } - std::thread::sleep(Duration::from_micros(50)); - } - Err(error) => return Err(error.into()), - } - } -} - fn require_startup(observed: StartupMode, expected: StartupMode) -> io::Result<()> { if observed != expected { return Err(io::Error::other(format!( @@ -350,58 +253,12 @@ fn emit(phase: &str, operation: &str, elapsed: Duration, operations: u64, bytes: "result phase={phase} elapsed_ns={} elapsed_seconds={:.6} current_rss_bytes={} peak_rss_bytes={}", elapsed.as_nanos(), elapsed.as_secs_f64(), - current_rss_bytes(), + current_rss_bytes().unwrap_or(0), peak_rss_bytes(), ); } -#[cfg(unix)] -fn peak_rss_bytes() -> u64 { - let mut usage = MaybeUninit::::zeroed(); - // SAFETY: `usage` points to writable storage for one `rusage` value. - if unsafe { libc::getrusage(libc::RUSAGE_SELF, usage.as_mut_ptr()) } != 0 { - return 0; - } - // SAFETY: a successful getrusage initialized the complete value. - let usage = unsafe { usage.assume_init() }; - #[cfg(any(target_os = "macos", target_os = "ios"))] - { - u64::try_from(usage.ru_maxrss).unwrap_or(0) - } - #[cfg(not(any(target_os = "macos", target_os = "ios")))] - { - u64::try_from(usage.ru_maxrss) - .unwrap_or(0) - .saturating_mul(1024) - } -} - -#[cfg(not(unix))] -fn peak_rss_bytes() -> u64 { - 0 -} - -#[cfg(target_os = "linux")] -fn current_rss_bytes() -> u64 { - fs::read_to_string("/proc/self/status") - .ok() - .and_then(|status| { - status.lines().find_map(|line| { - line.strip_prefix("VmRSS:") - .and_then(|value| value.split_ascii_whitespace().next()) - .and_then(|value| value.parse::().ok()) - }) - }) - .unwrap_or(0) - .saturating_mul(1024) -} - -#[cfg(not(target_os = "linux"))] -fn current_rss_bytes() -> u64 { - 0 -} - -fn emit_sizes(files: &ScaleFiles, peak_disk_bytes: u64) -> io::Result<()> { +fn emit_sizes(files: &BenchFiles, peak_disk_bytes: u64) -> io::Result<()> { let logical_bytes = files.logical_bytes()?; let allocated_bytes = files.allocated_bytes()?; if logical_bytes > peak_disk_bytes { @@ -420,29 +277,3 @@ fn sentinel_key(ordinal: usize) -> [u8; 16] { key[8..].copy_from_slice(&(ordinal as u64).to_le_bytes()); key } - -fn env_u64(name: &str, default: u64) -> io::Result { - match env::var(name) { - Ok(value) => value - .parse() - .map_err(|_| invalid(format!("{name} must be an unsigned integer"))), - Err(env::VarError::NotPresent) => Ok(default), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn env_usize(name: &str, default: usize) -> io::Result { - env_u64(name, default as u64).and_then(|value| { - usize::try_from(value).map_err(|_| invalid(format!("{name} does not fit usize"))) - }) -} - -fn sidecar(path: &Path, suffix: &str) -> PathBuf { - let mut value = path.as_os_str().to_owned(); - value.push(suffix); - PathBuf::from(value) -} - -fn invalid(message: impl Into) -> io::Error { - io::Error::new(io::ErrorKind::InvalidInput, message.into()) -} diff --git a/benchmarks/region_index_turnover/main.rs b/benchmarks/region_index_turnover/main.rs index 7bc8a45..3a40bc8 100644 --- a/benchmarks/region_index_turnover/main.rs +++ b/benchmarks/region_index_turnover/main.rs @@ -12,10 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::env; use std::fmt; use std::io; +use benchmarks::config::env_usize; use benchmarks::report::JobReport; use benchmarks::report::RunReporter; use cache2::benchmarking::RegionIndexTurnoverOptions; @@ -129,17 +129,3 @@ fn report_phase(turn: usize, phase: &str, measurement: RegionIndexTurnoverPhase) measurement.checksum, ); } - -fn env_usize(name: &str, default: usize) -> io::Result { - match env::var(name) { - Ok(value) => value - .parse() - .map_err(|_| invalid(format!("{name} must be an unsigned integer"))), - Err(env::VarError::NotPresent) => Ok(default), - Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), - } -} - -fn invalid(message: impl Into) -> io::Error { - io::Error::new(io::ErrorKind::InvalidInput, message.into()) -} diff --git a/benchmarks/src/config.rs b/benchmarks/src/config.rs index a05440a..1891af3 100644 --- a/benchmarks/src/config.rs +++ b/benchmarks/src/config.rs @@ -19,9 +19,11 @@ use std::io; use std::str::FromStr; use cache2::IoEngineOptions; +use cache2::IoMode; use cache2::IoUringOptions; use cache2::IoUringPoolOptions; use cache2::IoUringSqPollOptions; +use cache2::L1EvictionPolicy; use cache2::PosixIoOptions; /// Reads backend-specific pool settings under a benchmark's environment prefix. @@ -95,6 +97,89 @@ pub fn reject_renamed_env(prefix: &str) -> io::Result<()> { Ok(()) } +/// Reads an unsigned 64-bit setting, falling back to `default` when unset. +pub fn env_u64(name: &str, default: u64) -> io::Result { + Ok(setting(name)?.unwrap_or(default)) +} + +/// Reads a `usize` setting, falling back to `default` when unset. +pub fn env_usize(name: &str, default: usize) -> io::Result { + Ok(setting(name)?.unwrap_or(default)) +} + +/// Reads an unsigned 32-bit setting, falling back to `default` when unset. +pub fn env_u32(name: &str, default: u32) -> io::Result { + Ok(setting(name)?.unwrap_or(default)) +} + +/// Reads a boolean setting (`true`/`1` or `false`/`0`), falling back to `default` when unset. +pub fn env_bool(name: &str, default: bool) -> io::Result { + match setting::(name)?.as_deref() { + None => Ok(default), + Some("true" | "1") => Ok(true), + Some("false" | "0") => Ok(false), + Some(_) => Err(invalid(format!("{name} must be true, false, 1, or 0"))), + } +} + +/// Reads an optional `usize` setting. +pub fn env_optional_usize(name: &str) -> io::Result> { + setting(name) +} + +/// Reads an optional finite, non-negative threshold setting. +pub fn env_optional_f64(name: &str) -> io::Result> { + let Some(value) = setting::(name)? else { + return Ok(None); + }; + if !value.is_finite() || value < 0.0 { + return Err(invalid(format!( + "{name} must be a finite non-negative number" + ))); + } + Ok(Some(value)) +} + +/// Reads a comma-separated list of `usize` values, falling back to `default` when unset. +pub fn env_usize_list(name: &str, default: &[usize]) -> io::Result> { + match env::var(name) { + Ok(value) => value + .split(',') + .map(|item| { + item.parse::() + .map_err(|_| invalid(format!("{name} must be comma-separated integers"))) + }) + .collect::>>() + .map(Vec::into_boxed_slice), + Err(env::VarError::NotPresent) => Ok(default.to_vec().into_boxed_slice()), + Err(error) => Err(invalid(format!("cannot read {name}: {error}"))), + } +} + +/// Reads an I/O mode setting (`buffered` or `direct`, default `buffered`). +pub fn parse_io_mode(name: &str) -> io::Result { + match env::var(name) + .unwrap_or_else(|_| "buffered".to_owned()) + .as_str() + { + "buffered" => Ok(IoMode::Buffered), + "direct" => Ok(IoMode::Direct), + value => Err(invalid(format!("unsupported I/O mode: {value}"))), + } +} + +/// Reads an L1 eviction policy setting (`clock` or `s3-fifo`, default `clock`). +pub fn parse_l1_eviction_policy(name: &str) -> io::Result { + match env::var(name) + .unwrap_or_else(|_| "clock".to_owned()) + .as_str() + { + "clock" => Ok(L1EvictionPolicy::Clock), + "s3-fifo" => Ok(L1EvictionPolicy::S3Fifo), + value => Err(invalid(format!("unsupported L1 eviction policy: {value}"))), + } +} + fn io_uring_pool( prefix: &str, role: &str, @@ -128,7 +213,8 @@ fn io_uring_pool( Ok(options) } -fn setting(name: &str) -> io::Result> { +/// Reads an optional typed setting, reporting the raw value on parse failure. +pub fn setting(name: &str) -> io::Result> { match env::var(name) { Ok(value) => value .parse() @@ -139,6 +225,7 @@ fn setting(name: &str) -> io::Result> { } } -fn invalid(message: impl Into) -> io::Error { +/// Builds an `InvalidInput` error for a rejected benchmark setting. +pub fn invalid(message: impl Into) -> io::Error { io::Error::new(io::ErrorKind::InvalidInput, message.into()) } diff --git a/benchmarks/src/harness.rs b/benchmarks/src/harness.rs new file mode 100644 index 0000000..66682b0 --- /dev/null +++ b/benchmarks/src/harness.rs @@ -0,0 +1,240 @@ +// Copyright 2026 ScopeDB, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Runtime support shared by the benchmark harnesses: benchmark file +//! lifecycle, process memory sampling, write-admission retry, and +//! deterministic mixing. + +use std::fs; +use std::io; +#[cfg(unix)] +use std::mem::MaybeUninit; +use std::path::Path; +use std::path::PathBuf; +use std::time::Duration; +use std::time::Instant; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; + +use cache2::Cache; + +use crate::config::invalid; + +const RETRY_DELAY: Duration = Duration::from_micros(50); +const YIELD_RETRIES: usize = 8; + +/// A benchmark's data file and its sidecars, removed together on drop. +/// +/// The set covers the data file plus its `.state`, `.image`, and +/// `.image.next` siblings. +pub struct BenchFiles { + data: PathBuf, + stem: String, + cleanup_on_drop: bool, +} + +impl BenchFiles { + /// Creates a timestamped file set that drop always removes. + pub fn new(directory: &Path, stem: &str) -> Self { + Self::with_cleanup(directory, stem, true) + } + + /// Creates a timestamped file set that drop preserves until + /// [`mark_success`](Self::mark_success), so failed runs keep their + /// artifacts for inspection. + pub fn preserved_on_failure(directory: &Path, stem: &str) -> Self { + Self::with_cleanup(directory, stem, false) + } + + fn with_cleanup(directory: &Path, stem: &str, cleanup_on_drop: bool) -> Self { + let timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + Self { + data: directory.join(format!( + "cache2-{stem}-{}-{timestamp}.cache", + std::process::id() + )), + stem: stem.to_owned(), + cleanup_on_drop, + } + } + + /// Arms drop to remove the file set. + pub fn mark_success(&mut self) { + self.cleanup_on_drop = true; + } + + /// Returns the path of the benchmark's data file. + pub fn data(&self) -> &Path { + &self.data + } + + /// Returns the total logical bytes across the existing files in the set. + pub fn logical_bytes(&self) -> io::Result { + self.paths() + .into_iter() + .try_fold(0_u64, |total, path| match fs::metadata(path) { + Ok(metadata) => total + .checked_add(metadata.len()) + .ok_or_else(|| invalid("logical disk byte count overflow")), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(total), + Err(error) => Err(error), + }) + } + + /// Returns the total allocated on-disk bytes across the existing files in the set. + #[cfg(unix)] + pub fn allocated_bytes(&self) -> io::Result { + use std::os::unix::fs::MetadataExt; + + self.paths() + .into_iter() + .try_fold(0_u64, |total, path| match fs::metadata(path) { + Ok(metadata) => total + .checked_add(metadata.blocks().saturating_mul(512)) + .ok_or_else(|| invalid("allocated file size overflow")), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(total), + Err(error) => Err(error), + }) + } + + /// Returns the total allocated bytes, approximated by logical size without block accounting. + #[cfg(not(unix))] + pub fn allocated_bytes(&self) -> io::Result { + self.logical_bytes() + } + + fn paths(&self) -> [PathBuf; 4] { + [ + self.data.clone(), + sidecar(&self.data, ".state"), + sidecar(&self.data, ".image"), + sidecar(&self.data, ".image.next"), + ] + } +} + +impl Drop for BenchFiles { + fn drop(&mut self) { + if !self.cleanup_on_drop { + eprintln!( + "{} artifacts preserved after failure: data={}", + self.stem, + self.data.display() + ); + return; + } + for path in self.paths() { + let _ = fs::remove_file(path); + } + } +} + +/// Writes one value, retrying overload rejections until `timeout` elapses. +/// +/// Returns the write receipt and the number of attempts. The first few +/// retries yield the thread so bounded staging can drain; later retries back +/// off with a short sleep. +pub fn put_eventually( + cache: &Cache, + key: &[u8], + value: &[u8], + timeout: Duration, +) -> io::Result<(u64, usize)> { + let deadline = Instant::now() + timeout; + let mut attempts = 0_usize; + loop { + attempts = attempts.saturating_add(1); + match cache.put(key, value) { + Ok(receipt) => return Ok((receipt, attempts)), + Err(error) if error.kind() == cache2::ErrorKind::Overloaded => { + if Instant::now() >= deadline { + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "benchmark write did not enter bounded staging", + )); + } + if attempts <= YIELD_RETRIES { + std::thread::yield_now(); + } else { + std::thread::sleep(RETRY_DELAY); + } + } + Err(error) => return Err(error.into()), + } + } +} + +/// Returns the peak resident set size of this process in bytes, or 0 when unavailable. +#[cfg(unix)] +pub fn peak_rss_bytes() -> u64 { + let mut usage = MaybeUninit::::zeroed(); + // SAFETY: `usage` points to writable storage for one `rusage` value. + if unsafe { libc::getrusage(libc::RUSAGE_SELF, usage.as_mut_ptr()) } != 0 { + return 0; + } + // SAFETY: a successful getrusage initialized the complete value. + let usage = unsafe { usage.assume_init() }; + #[cfg(any(target_os = "macos", target_os = "ios"))] + { + u64::try_from(usage.ru_maxrss).unwrap_or(0) + } + #[cfg(not(any(target_os = "macos", target_os = "ios")))] + { + u64::try_from(usage.ru_maxrss) + .unwrap_or(0) + .saturating_mul(1024) + } +} + +/// Returns the peak resident set size of this process in bytes, or 0 when unavailable. +#[cfg(not(unix))] +pub fn peak_rss_bytes() -> u64 { + 0 +} + +/// Returns the current resident set size in bytes, read from `/proc/self/status`. +#[cfg(target_os = "linux")] +pub fn current_rss_bytes() -> io::Result { + let status = fs::read_to_string("/proc/self/status")?; + let kib = status + .lines() + .find_map(|line| line.strip_prefix("VmRSS:")) + .and_then(|value| value.split_ascii_whitespace().next()) + .and_then(|value| value.parse::().ok()) + .ok_or_else(|| io::Error::other("cannot read current RSS from /proc/self/status"))?; + kib.checked_mul(1024) + .ok_or_else(|| io::Error::other("current RSS byte count overflow")) +} + +/// Returns the current resident set size in bytes, or 0 on platforms without `/proc`. +#[cfg(not(target_os = "linux"))] +pub fn current_rss_bytes() -> io::Result { + Ok(0) +} + +/// Deterministic 64-bit bit mixer (the splitmix64 finalizer). +pub fn splitmix64(mut value: u64) -> u64 { + value = (value ^ (value >> 30)).wrapping_mul(0xbf58_476d_1ce4_e5b9); + value = (value ^ (value >> 27)).wrapping_mul(0x94d0_49bb_1331_11eb); + value ^ (value >> 31) +} + +fn sidecar(path: &Path, suffix: &str) -> PathBuf { + let mut value = path.as_os_str().to_owned(); + value.push(suffix); + PathBuf::from(value) +} diff --git a/benchmarks/src/lib.rs b/benchmarks/src/lib.rs index c23b318..04a39b5 100644 --- a/benchmarks/src/lib.rs +++ b/benchmarks/src/lib.rs @@ -15,4 +15,5 @@ //! Shared support for the standalone C² benchmark targets. pub mod config; +pub mod harness; pub mod report; diff --git a/benchmarks/src/report.rs b/benchmarks/src/report.rs index 0b160a5..e96ac2c 100644 --- a/benchmarks/src/report.rs +++ b/benchmarks/src/report.rs @@ -29,6 +29,13 @@ use cache2::DetailedCacheSnapshot; const LATENCY_BUCKETS: usize = 65; +/// Returns true when an operation ordinal falls on the sampling interval. +/// +/// Interval 0 disables sampling. +pub fn should_sample(ordinal: u64, interval: usize) -> bool { + interval != 0 && ordinal.is_multiple_of(interval as u64) +} + /// A fixed-size log2 latency histogram. /// /// Percentiles are reported as bucket upper bounds. Mean and standard @@ -63,6 +70,13 @@ impl LatencyHistogram { self.maximum_ns = self.maximum_ns.max(nanos); } + /// Records one sample when `started` carries a sampling instant. + pub fn record_sampled(&mut self, started: Option) { + if let Some(started) = started { + self.record(started.elapsed()); + } + } + /// Merges another bounded histogram. pub fn merge(&mut self, other: Self) { for (target, value) in self.buckets.iter_mut().zip(other.buckets) { @@ -162,6 +176,13 @@ impl AtomicLatencyHistogram { self.maximum_ns.fetch_max(nanos, Ordering::Relaxed); } + /// Records one sample when `started` carries a sampling instant. + pub fn record_sampled(&self, started: Option) { + if let Some(started) = started { + self.record(started.elapsed()); + } + } + /// Takes a non-transactional snapshot suitable for periodic reporting. pub fn snapshot(&self) -> LatencyHistogram { let buckets = array::from_fn(|index| self.buckets[index].load(Ordering::Relaxed)); diff --git a/cache2/src/codec.rs b/cache2/src/codec.rs new file mode 100644 index 0000000..00f7ce8 --- /dev/null +++ b/cache2/src/codec.rs @@ -0,0 +1,102 @@ +// Copyright 2026 ScopeDB, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Little-endian field codecs shared by the persistent formats. +//! +//! The on-disk structures are encoded field-by-field; their Rust layout is not +//! part of the disk format. Reads are checked so decoders can reject truncated +//! input instead of panicking, while writes panic because encoders size their +//! buffers from the same constants as their field offsets. + +use crate::checksum::Crc32c; + +/// Reads the little-endian `u16` at `offset`, or returns `None` when the field +/// range falls outside `input`. +pub fn get_u16(input: &[u8], offset: usize) -> Option { + let bytes: [u8; size_of::()] = input + .get(offset..offset.checked_add(size_of::())?)? + .try_into() + .ok()?; + Some(u16::from_le_bytes(bytes)) +} + +/// Reads the little-endian `u32` at `offset`, or returns `None` when the field +/// range falls outside `input`. +pub fn get_u32(input: &[u8], offset: usize) -> Option { + let bytes: [u8; size_of::()] = input + .get(offset..offset.checked_add(size_of::())?)? + .try_into() + .ok()?; + Some(u32::from_le_bytes(bytes)) +} + +/// Reads the little-endian `u64` at `offset`, or returns `None` when the field +/// range falls outside `input`. +pub fn get_u64(input: &[u8], offset: usize) -> Option { + let bytes: [u8; size_of::()] = input + .get(offset..offset.checked_add(size_of::())?)? + .try_into() + .ok()?; + Some(u64::from_le_bytes(bytes)) +} + +/// Writes `value` as little-endian bytes at `offset`. +/// +/// Panics when the field range falls outside `output`; encoders size their +/// buffers from the same constants as their field offsets. +pub fn put_u16(output: &mut [u8], offset: usize, value: u16) { + output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); +} + +/// Writes `value` as little-endian bytes at `offset`. +/// +/// Panics when the field range falls outside `output`; encoders size their +/// buffers from the same constants as their field offsets. +pub fn put_u32(output: &mut [u8], offset: usize, value: u32) { + output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); +} + +/// Writes `value` as little-endian bytes at `offset`. +/// +/// Panics when the field range falls outside `output`; encoders size their +/// buffers from the same constants as their field offsets. +pub fn put_u64(output: &mut [u8], offset: usize, value: u64) { + output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); +} + +/// Returns the CRC32C of `input` with the little-endian `u32` checksum field at +/// `checksum_offset` treated as zero, or `None` when the field range falls +/// outside `input`. +/// +/// Persistent headers and pages checksum their image with the checksum field +/// itself zeroed, so writers and readers cover the same bytes without copying. +pub fn crc32c_with_zeroed_u32(input: &[u8], checksum_offset: usize) -> Option { + let field_end = checksum_offset.checked_add(size_of::())?; + let before = input.get(..checksum_offset)?; + let after = input.get(field_end..)?; + let mut checksum = Crc32c::new(); + checksum.update(before); + checksum.update(&[0; size_of::()]); + checksum.update(after); + Some(checksum.finish()) +} + +/// Returns whether the stored checksum field at `checksum_offset` matches +/// [`crc32c_with_zeroed_u32`]. +pub fn crc32c_with_zeroed_u32_matches(input: &[u8], checksum_offset: usize) -> bool { + let Some(expected) = get_u32(input, checksum_offset) else { + return false; + }; + crc32c_with_zeroed_u32(input, checksum_offset) == Some(expected) +} diff --git a/cache2/src/fixtures.rs b/cache2/src/fixtures.rs index 3839388..702c4de 100644 --- a/cache2/src/fixtures.rs +++ b/cache2/src/fixtures.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Shared byte assertions for module-local persistent-format fixtures. +//! Shared assertions and guards for module-local tests. //! //! Golden fixtures pin versioned on-disk bytes. Changes require an explicit format-version //! decision; tests never regenerate them. Each fixture lives beside the module that owns its @@ -20,6 +20,21 @@ //! //! The sparse representation starts with the complete byte length. Each following line contains a //! hexadecimal offset and hexadecimal bytes; unspecified bytes are zero. +//! +//! [`TestFile`] gives file-based tests a unique temporary path and removes it on drop, including +//! when an assertion fails midway. + +use std::env; +use std::fs::File; +use std::fs::OpenOptions; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; +use std::sync::atomic::AtomicU64; +use std::sync::atomic::Ordering; + +use crate::io::backend::FileBackend; +use crate::io::backend::IoBackend; /// Checks every byte, including zero padding, and returns the committed bytes /// for decoder compatibility checks. @@ -67,3 +82,56 @@ fn sparse_golden(input: &str) -> Vec { } output.expect("golden fixture must declare its length") } + +/// Unique temporary file removed when the guard drops, even when the test fails midway. +pub struct TestFile { + path: PathBuf, +} + +impl TestFile { + /// Returns a guard for a unique `cache2-{label}-*` path in the system temp directory. + pub fn new(label: &str) -> Self { + static NEXT_ID: AtomicU64 = AtomicU64::new(0); + let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); + Self { + path: env::temp_dir().join(format!("cache2-{label}-{}-{id}.tmp", std::process::id())), + } + } + + /// Returns the temporary file path. + pub fn path(&self) -> &Path { + &self.path + } + + /// Opens the file for reading and writing, creating it when missing. + pub fn open(&self) -> File { + OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&self.path) + .unwrap() + } + + /// Creates the file exclusively for reading and writing. + pub fn create_new(&self) -> File { + OpenOptions::new() + .read(true) + .write(true) + .create_new(true) + .open(&self.path) + .unwrap() + } + + /// Opens the file as a buffered backend. + pub fn backend(&self) -> Arc { + Arc::new(FileBackend::open(&self.path).unwrap()) + } +} + +impl Drop for TestFile { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.path); + } +} diff --git a/cache2/src/io/backend.rs b/cache2/src/io/backend.rs index 5de8aa5..835c0c5 100644 --- a/cache2/src/io/backend.rs +++ b/cache2/src/io/backend.rs @@ -922,7 +922,7 @@ pub fn write_all_at_with_progress( (Ok(()), transferred) } -fn retry_interrupted(mut operation: impl FnMut() -> io::Result) -> io::Result { +pub fn retry_interrupted(mut operation: impl FnMut() -> io::Result) -> io::Result { let mut retries = 0_usize; loop { match operation() { @@ -949,43 +949,11 @@ unsafe extern "C" { #[cfg(test)] mod tests { - use std::env; - use std::path::PathBuf; - use std::sync::atomic::AtomicU64; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use super::*; - - static NEXT_PATH: AtomicU64 = AtomicU64::new(0); - - struct TestFile(PathBuf); - - impl TestFile { - fn new(label: &str) -> Self { - let nonce = NEXT_PATH.fetch_add(1, Ordering::Relaxed); - Self(env::temp_dir().join(format!( - "cache2-{label}-{}-{nonce}.cache", - std::process::id() - ))) - } - - fn open(&self) -> File { - OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(false) - .open(&self.0) - .unwrap() - } - } - - impl Drop for TestFile { - fn drop(&mut self) { - let _ = std::fs::remove_file(&self.0); - } - } + use crate::fixtures::TestFile; #[repr(align(4096))] struct AlignedBytes([u8; 2 * DIRECT_IO_ALIGNMENT]); @@ -1227,7 +1195,7 @@ mod tests { #[test] fn preallocate_sets_the_exact_file_extent() { let file = TestFile::new("preallocate"); - let backend = FileBackend::open(&file.0).unwrap(); + let backend = FileBackend::open(file.path()).unwrap(); let len = 2 * DIRECT_IO_ALIGNMENT as u64; backend.preallocate(len).unwrap(); assert_eq!(backend.len().unwrap(), len); @@ -1242,7 +1210,7 @@ mod tests { #[test] fn unsupported_physical_preallocation_fails_closed() { let file = TestFile::new("preallocate-unsupported"); - let backend = FileBackend::open(&file.0).unwrap(); + let backend = FileBackend::open(file.path()).unwrap(); let error = backend.preallocate(DIRECT_IO_ALIGNMENT as u64).unwrap_err(); assert_eq!(error.kind(), io::ErrorKind::Unsupported); assert_eq!(backend.len().unwrap(), 0); @@ -1254,11 +1222,11 @@ mod tests { let alias = TestFile::new("control-alias"); let other = TestFile::new("control-other"); drop(primary.open()); - std::fs::hard_link(&primary.0, &alias.0).unwrap(); + std::fs::hard_link(primary.path(), alias.path()).unwrap(); - let primary = FileBackend::open(&primary.0).unwrap(); - let alias = FileBackend::open(&alias.0).unwrap(); - let other = FileBackend::open(&other.0).unwrap(); + let primary = FileBackend::open(primary.path()).unwrap(); + let alias = FileBackend::open(alias.path()).unwrap(); + let other = FileBackend::open(other.path()).unwrap(); let cloned = ControlIoBackend::try_clone_control_file(&primary).unwrap(); primary @@ -1274,12 +1242,12 @@ mod tests { #[test] fn recovery_temp_creation_never_reopens_an_existing_target() { let image = TestFile::new("recovery-create-new"); - let backend = FileBackend::create_new_buffered(&image.0).unwrap(); + let backend = FileBackend::create_new_buffered(image.path()).unwrap(); backend .write_at(WritePoint::RecoveryImageMetadata, b"metadata", 0) .unwrap(); - let error = FileBackend::create_new_buffered(&image.0) + let error = FileBackend::create_new_buffered(image.path()) .err() .expect("create_new must reject an existing recovery target"); assert_eq!(error.kind(), io::ErrorKind::AlreadyExists); @@ -1296,9 +1264,10 @@ mod tests { let image = TestFile::new("shared-fault-image"); let temp = TestFile::new("shared-fault-temp"); let faults = FaultHandle::default(); - let state = FaultBackend::open_with_handle(&state.0, faults.clone()).unwrap(); - let image = FaultBackend::open_with_handle(&image.0, faults.clone()).unwrap(); - let temp = FaultBackend::create_new_buffered_with_handle(&temp.0, faults.clone()).unwrap(); + let state = FaultBackend::open_with_handle(state.path(), faults.clone()).unwrap(); + let image = FaultBackend::open_with_handle(image.path(), faults.clone()).unwrap(); + let temp = + FaultBackend::create_new_buffered_with_handle(temp.path(), faults.clone()).unwrap(); faults.arm( FaultEvent::Write(WritePoint::State), @@ -1338,9 +1307,9 @@ mod tests { let target = TestFile::new("symlink-target"); let link = TestFile::new("symlink-link"); drop(target.open()); - symlink(&target.0, &link.0).unwrap(); + symlink(target.path(), link.path()).unwrap(); - assert!(FileBackend::open(&link.0).is_err()); + assert!(FileBackend::open(link.path()).is_err()); } } diff --git a/cache2/src/io/engine/mod.rs b/cache2/src/io/engine/mod.rs index 3c2db49..9453609 100644 --- a/cache2/src/io/engine/mod.rs +++ b/cache2/src/io/engine/mod.rs @@ -952,41 +952,104 @@ fn submit_cache_io_until( } pub trait IoEngine: Send + Sync { + /// Shared submission, completion, and bookkeeping state behind the + /// engine-specific driver. + fn inner(&self) -> &Arc; + /// Backend path and direct-I/O counters reported with the engine stats. + fn runtime_io_stats(&self) -> RuntimeIoStats; + /// Installed once during construction, before any requests are admitted. - fn set_latency_recorder(&self, recorder: crate::stats::recording::IoTiming); - fn try_reserve_read(&self) -> io::Result; - fn read_slot_waiter(&self) -> ReadSlotWaiter; + fn set_latency_recorder(&self, recorder: crate::stats::recording::IoTiming) { + assert!( + self.inner().shared.latency.set(recorder).is_ok(), + "I/O recorder installed twice" + ); + } + + fn try_reserve_read(&self) -> io::Result { + self.inner().try_reserve_read() + } + + fn read_slot_waiter(&self) -> ReadSlotWaiter { + self.inner().read_slot_waiter() + } + fn submit_reserved_read( &self, slot: ReadSlot, operation: IoOperation, - ) -> Result; + ) -> Result { + self.inner().submit_reserved_read(slot, operation) + } + #[cfg(test)] - fn submit(&self, operation: IoOperation) -> Result; + fn submit(&self, operation: IoOperation) -> Result { + self.inner().submit(operation) + } + #[cfg(test)] - fn submit_wait(&self, operation: IoOperation) -> Result; + fn submit_wait(&self, operation: IoOperation) -> Result { + self.inner().submit_wait(operation) + } + fn submit_wait_controlled( &self, operation: IoOperation, cancelled: &AtomicBool, deadline: Option, - ) -> Result; - fn wake_slot_waiters(&self); - fn cancel(&self, request_id: RequestId, state: &CompletionState) -> io::Result; - fn shutdown(&self) -> io::Result<()>; - fn in_flight(&self) -> usize; + ) -> Result { + self.inner() + .submit_wait_controlled(operation, cancelled, deadline) + } + + fn wake_slot_waiters(&self) { + self.inner().shared.wake_slot_waiters(); + } + + fn cancel(&self, request_id: RequestId, state: &CompletionState) -> io::Result { + self.inner().cancel(request_id, state) + } + + fn shutdown(&self) -> io::Result<()> { + self.inner().shutdown() + } + + fn in_flight(&self) -> usize { + self.inner().shared.total_in_flight() + } + #[cfg(test)] - fn direct_active(&self) -> bool; + fn direct_active(&self) -> bool { + self.runtime_io_stats().direct_active + } + /// Permanently stop accepting requests after a target operation missed both its /// deadline and cancellation grace period. - fn stop_accepting_requests(&self); - fn writes_in_flight(&self) -> usize; + fn stop_accepting_requests(&self) { + self.inner().stop_accepting_requests(); + } + + fn writes_in_flight(&self) -> usize { + self.inner().shared.writes_in_flight() + } + /// True means a failed driver could not fence an issued write. /// The cache must retain its exclusive file lock for process lifetime. - fn has_unfenced_writes(&self) -> bool; + fn has_unfenced_writes(&self) -> bool { + self.inner().shared.has_unfenced_writes() + } + #[cfg(test)] - fn mark_unfenced_writes_for_test(&self); - fn stats(&self) -> EngineIoSnapshot; + fn mark_unfenced_writes_for_test(&self) { + self.inner().shared.mark_unfenced_writes(); + } + + fn stats(&self) -> EngineIoSnapshot { + EngineIoSnapshot { + requests: self.inner().shared.snapshot(), + runtime: self.runtime_io_stats(), + } + } #[cfg(test)] fn read_exact_at(&self, buffer: IoBuffer, offset: u64) -> Result { @@ -1589,7 +1652,7 @@ struct ShutdownState { stopped: Condvar, } -struct RuntimeInner { +pub(crate) struct RuntimeInner { shared: Arc, commands: SyncSender, submit_state: Arc>, diff --git a/cache2/src/io/engine/posix.rs b/cache2/src/io/engine/posix.rs index 6a20768..670841c 100644 --- a/cache2/src/io/engine/posix.rs +++ b/cache2/src/io/engine/posix.rs @@ -19,40 +19,31 @@ use std::sync::Arc; use std::sync::Condvar; use std::sync::Mutex; use std::sync::RwLock; -use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use std::sync::mpsc; use std::sync::mpsc::Receiver; -use std::time::Instant; use crate::io::backend::IoBackend; #[cfg(unix)] use crate::io::backend::RuntimeFileBackend; #[cfg(unix)] use crate::io::backend::RuntimeFileSet; +use crate::io::backend::RuntimeIoStats; use crate::io::backend::read_exact_at_uninit_with_progress; use crate::io::backend::write_all_at_with_progress; use crate::io::engine::BackendIoEngine; -use crate::io::engine::CompletionState; use crate::io::engine::CompletionStatus; use crate::io::engine::DriverCommand; -use crate::io::engine::EngineIoSnapshot; use crate::io::engine::IoEngine; use crate::io::engine::IoOperation; -use crate::io::engine::IoRequest; -use crate::io::engine::ReadSlot; -use crate::io::engine::ReadSlotWaiter; -use crate::io::engine::RequestId; use crate::io::engine::RuntimeInner; use crate::io::engine::RuntimeShared; use crate::io::engine::ShutdownPhase; use crate::io::engine::ShutdownState; -use crate::io::engine::SubmitError; use crate::io::engine::SubmitState; use crate::io::engine::lock_unpoisoned; use crate::managed_memory::CACHE_THREAD_STACK_BYTES; -use crate::stats::recording::IoTiming; impl BackendIoEngine { #[cfg(unix)] @@ -172,92 +163,12 @@ impl BackendIoEngine { } impl IoEngine for BackendIoEngine { - fn set_latency_recorder(&self, recorder: IoTiming) { - assert!( - self.inner.shared.latency.set(recorder).is_ok(), - "I/O recorder installed twice" - ); + fn inner(&self) -> &Arc { + &self.inner } - fn try_reserve_read(&self) -> io::Result { - self.inner.try_reserve_read() - } - - fn read_slot_waiter(&self) -> ReadSlotWaiter { - self.inner.read_slot_waiter() - } - - fn submit_reserved_read( - &self, - slot: ReadSlot, - operation: IoOperation, - ) -> Result { - self.inner.submit_reserved_read(slot, operation) - } - - #[cfg(test)] - fn submit(&self, operation: IoOperation) -> Result { - self.inner.submit(operation) - } - - #[cfg(test)] - fn submit_wait(&self, operation: IoOperation) -> Result { - self.inner.submit_wait(operation) - } - - fn submit_wait_controlled( - &self, - operation: IoOperation, - cancelled: &AtomicBool, - deadline: Option, - ) -> Result { - self.inner - .submit_wait_controlled(operation, cancelled, deadline) - } - - fn wake_slot_waiters(&self) { - self.inner.shared.wake_slot_waiters(); - } - - fn cancel(&self, request_id: RequestId, state: &CompletionState) -> io::Result { - self.inner.cancel(request_id, state) - } - - fn shutdown(&self) -> io::Result<()> { - self.inner.shutdown() - } - - fn in_flight(&self) -> usize { - self.inner.shared.total_in_flight() - } - - #[cfg(test)] - fn direct_active(&self) -> bool { - self.backend.runtime_io_stats().direct_active - } - - fn stop_accepting_requests(&self) { - self.inner.stop_accepting_requests(); - } - - fn writes_in_flight(&self) -> usize { - self.inner.shared.writes_in_flight() - } - - fn has_unfenced_writes(&self) -> bool { - self.inner.shared.has_unfenced_writes() - } - - #[cfg(test)] - fn mark_unfenced_writes_for_test(&self) { - self.inner.shared.mark_unfenced_writes(); - } - - fn stats(&self) -> EngineIoSnapshot { - EngineIoSnapshot { - requests: self.inner.shared.snapshot(), - runtime: self.backend.runtime_io_stats(), - } + fn runtime_io_stats(&self) -> RuntimeIoStats { + self.backend.runtime_io_stats() } } diff --git a/cache2/src/io/engine/tests.rs b/cache2/src/io/engine/tests.rs index fa230cb..5f48788 100644 --- a/cache2/src/io/engine/tests.rs +++ b/cache2/src/io/engine/tests.rs @@ -12,12 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::env; -use std::fs; -use std::fs::File; -use std::fs::OpenOptions; -use std::path::PathBuf; -use std::sync::atomic::AtomicU64; use std::sync::mpsc; use std::time::Duration; @@ -25,7 +19,7 @@ use super::*; use crate::IoOutcome; use crate::IoRole; use crate::StatsOptions; -use crate::io::backend::FileBackend; +use crate::fixtures::TestFile; use crate::io::backend::SyncMode; use crate::io::backend::SyncPoint; use crate::managed_memory::ManagedMemory; @@ -33,8 +27,6 @@ use crate::managed_memory::ManagedMemoryLimits; use crate::managed_memory::aligned_buffer_capacity; use crate::stats::recording::Recorder; -static FILE_ID: AtomicU64 = AtomicU64::new(1); - async fn wait_for_registered_read_waiters(engine: &BackendIoEngine, expected: usize) { for _ in 0..100 { let actual = engine @@ -73,39 +65,6 @@ async fn read_wait_error(waiter: tokio::task::JoinHandle>) } } -struct TestFile { - path: PathBuf, -} - -impl TestFile { - fn new() -> Self { - let id = FILE_ID.fetch_add(1, Ordering::Relaxed); - let path = - env::temp_dir().join(format!("cache2-io-engine-{}-{id}.bin", std::process::id())); - Self { path } - } - - fn backend(&self) -> Arc { - Arc::new(FileBackend::open(&self.path).unwrap()) - } - - fn file(&self) -> File { - OpenOptions::new() - .read(true) - .write(true) - .create(true) - .truncate(false) - .open(&self.path) - .unwrap() - } -} - -impl Drop for TestFile { - fn drop(&mut self) { - let _ = fs::remove_file(&self.path); - } -} - #[derive(Default)] struct BlockingState { entered: usize, @@ -317,7 +276,7 @@ fn aligned_buffer_has_stable_alignment() { #[test] fn posix_engine_round_trips_owned_buffers_and_drains() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = BackendIoEngine::new(file.backend(), 4).unwrap(); let managed_memory = managed_memory(); let input = b"owned async positioned I/O"; @@ -382,8 +341,8 @@ fn posix_engine_reports_progress_before_a_terminal_short_io_error() { #[tokio::test] async fn async_request_is_woken_by_driver_completion() { - let file = TestFile::new(); - file.file().set_len(4096).unwrap(); + let file = TestFile::new("io-engine"); + file.open().set_len(4096).unwrap(); let engine: Arc = Arc::new(BackendIoEngine::new(file.backend(), 2).unwrap()); let managed_memory = managed_memory(); let request = submit_cache_io( @@ -453,8 +412,8 @@ async fn dropping_async_wait_requests_bounded_cancellation() { #[tokio::test] async fn reserved_read_latency_includes_time_before_submission() { for activity_counters_enabled in [false, true] { - let file = TestFile::new(); - file.file().set_len(4096).unwrap(); + let file = TestFile::new("io-engine"); + file.open().set_len(4096).unwrap(); let engine = BackendIoEngine::new_with_workers_and_activity_counters( file.backend(), 1, @@ -553,7 +512,7 @@ async fn read_slot_waits_for_cancelled_request_to_release_physical_capacity() { #[tokio::test] async fn read_slot_wait_is_woken_by_engine_shutdown() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); let slot = engine.try_reserve_read().unwrap(); let mut waiters = Vec::new(); @@ -576,7 +535,7 @@ async fn read_slot_wait_is_woken_by_engine_shutdown() { #[tokio::test(flavor = "current_thread")] async fn queued_read_reservation_precedes_new_immediate_read() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); let held = engine.try_reserve_read().unwrap(); let queued = spawn_registered_read_slot_waiter(&engine, Duration::from_secs(1), 1).await; @@ -595,7 +554,7 @@ async fn queued_read_reservation_precedes_new_immediate_read() { #[tokio::test(flavor = "current_thread")] async fn queued_read_reservations_are_fifo() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); let held = engine.try_reserve_read().unwrap(); @@ -618,7 +577,7 @@ async fn queued_read_reservations_are_fifo() { #[tokio::test(flavor = "current_thread")] async fn queued_reads_use_every_released_engine_slot() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 2).unwrap()); let held: Vec<_> = (0..2).map(|_| engine.try_reserve_read().unwrap()).collect(); let first = spawn_registered_read_slot_waiter(&engine, Duration::from_secs(1), 1).await; @@ -637,7 +596,7 @@ async fn queued_reads_use_every_released_engine_slot() { #[tokio::test(flavor = "current_thread")] async fn timed_out_queue_head_passes_priority_to_next_read() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); let held = engine.try_reserve_read().unwrap(); @@ -655,7 +614,7 @@ async fn timed_out_queue_head_passes_priority_to_next_read() { #[tokio::test(flavor = "current_thread")] async fn cancelled_queue_head_passes_priority_to_next_read() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); let held = engine.try_reserve_read().unwrap(); @@ -706,10 +665,10 @@ async fn async_read_deadline_keeps_other_slots_available() { #[cfg(unix)] #[test] fn posix_engine_routes_only_aligned_record_io_to_direct() { - let buffered = TestFile::new(); - let direct = TestFile::new(); - let buffered_file = buffered.file(); - let direct_file = direct.file(); + let buffered = TestFile::new("io-engine"); + let direct = TestFile::new("io-engine"); + let buffered_file = buffered.open(); + let direct_file = direct.open(); buffered_file.set_len(8192).unwrap(); direct_file.set_len(8192).unwrap(); let engine = @@ -754,7 +713,7 @@ fn posix_engine_routes_only_aligned_record_io_to_direct() { #[test] fn unfenced_write_state_remains_unsafe_after_shutdown() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = BackendIoEngine::new(file.backend(), 1).unwrap(); assert!(!engine.has_unfenced_writes()); @@ -842,7 +801,7 @@ fn completion_deadline_keeps_an_issued_write_counted_until_target_completion() { #[test] fn engine_request_capacity_is_hard_bounded() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); assert!(matches!( BackendIoEngine::new(file.backend(), MAX_IO_REQUESTS_PER_ENGINE + 1), Err(error) if error.kind() == io::ErrorKind::InvalidInput @@ -852,8 +811,8 @@ fn engine_request_capacity_is_hard_bounded() { #[cfg(unix)] #[test] fn configured_posix_engine_shares_its_worker_capacity() { - let file = TestFile::new(); - let files = RuntimeFileSet::new(file.file(), None); + let file = TestFile::new("io-engine"); + let files = RuntimeFileSet::new(file.open(), None); let engine = build_file_engine(files, IoEngineConfig::Posix { workers: 4 }, false, false).unwrap(); @@ -869,7 +828,7 @@ fn configured_posix_engine_shares_its_worker_capacity() { #[test] fn disabled_io_statistics_skip_cumulative_engine_counters() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = BackendIoEngine::new_with_workers_and_activity_counters(file.backend(), 1, 1, false, false) .unwrap(); @@ -902,7 +861,7 @@ fn slot_state_tracks_full_write_capacity() { #[test] fn unused_read_reservation_releases_its_engine_slot() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = BackendIoEngine::new(file.backend(), 1).unwrap(); let slot = engine.try_reserve_read().unwrap(); assert_eq!(engine.in_flight(), 1); @@ -918,7 +877,7 @@ fn unused_read_reservation_releases_its_engine_slot() { #[test] fn nowait_submission_does_not_wait_for_the_shutdown_fence() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = BackendIoEngine::new(file.backend(), 1).unwrap(); let managed_memory = managed_memory(); let fence = engine @@ -1142,7 +1101,7 @@ fn quarantined_completion_does_not_return_a_potentially_live_buffer() { #[test] fn io_histograms_include_failures_when_activity_counters_are_disabled() { - let file = TestFile::new(); + let file = TestFile::new("io-engine"); let engine = BackendIoEngine::new_with_workers_and_activity_counters(file.backend(), 1, 1, false, false) .unwrap(); diff --git a/cache2/src/io/engine/uring.rs b/cache2/src/io/engine/uring.rs index ff679a1..1d3f1ea 100644 --- a/cache2/src/io/engine/uring.rs +++ b/cache2/src/io/engine/uring.rs @@ -33,7 +33,6 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use std::sync::mpsc; use std::sync::mpsc::Receiver; -use std::time::Instant; use hashcrew::xxhash::Xxh3_64Builder; use io_uring::IoUring; @@ -45,36 +44,33 @@ use io_uring::types; use crate::config::runtime::IoUringEngineConfig; use crate::io::backend::RuntimeFileSet; use crate::io::backend::RuntimeIoPath; +use crate::io::backend::RuntimeIoStats; use crate::io::backend::RuntimeIoStatsHandle; +use crate::io::backend::retry_interrupted; +#[cfg(test)] use crate::io::engine::CompletionState; use crate::io::engine::CompletionStatus; use crate::io::engine::DriverCommand; use crate::io::engine::DriverWake; -use crate::io::engine::EngineIoSnapshot; #[cfg(test)] use crate::io::engine::IO_QUEUE_ENTRY_RESERVATION_BYTES; #[cfg(test)] use crate::io::engine::IoBuffer; use crate::io::engine::IoEngine; use crate::io::engine::IoOperation; -use crate::io::engine::IoRequest; #[cfg(test)] use crate::io::engine::MAX_IO_REQUESTS_PER_ENGINE; use crate::io::engine::OperationKind; -use crate::io::engine::ReadSlot; -use crate::io::engine::ReadSlotWaiter; use crate::io::engine::RequestId; use crate::io::engine::RuntimeInner; use crate::io::engine::RuntimeShared; use crate::io::engine::ShutdownPhase; use crate::io::engine::ShutdownState; -use crate::io::engine::SubmitError; use crate::io::engine::SubmitState; use crate::io::engine::Task; #[cfg(test)] use crate::io::engine::io_uring_extra_memory_bytes; use crate::managed_memory::CACHE_THREAD_STACK_BYTES; -use crate::stats::recording::IoTiming; const CANCEL_CQE_BIT: u64 = 1_u64 << 63; const INTERNAL_CQE_BIT: u64 = 1_u64 << 62; @@ -84,7 +80,6 @@ const LINUX_EINTR: i32 = 4; const LINUX_ECANCELED: i32 = 125; const LINUX_POLLIN: u32 = 0x0001; const FATAL_DRAIN_ROUNDS: usize = 64; -const MAX_INTERRUPTED_RETRIES: usize = 4; const MAX_WAKE_ATTEMPTS: usize = 4; const MAX_COMMANDS_PER_DRAIN: usize = 64; @@ -246,92 +241,12 @@ impl UringIoEngine { } impl IoEngine for UringIoEngine { - fn set_latency_recorder(&self, recorder: IoTiming) { - assert!( - self.inner.shared.latency.set(recorder).is_ok(), - "I/O recorder installed twice" - ); - } - - fn try_reserve_read(&self) -> io::Result { - self.inner.try_reserve_read() - } - - fn read_slot_waiter(&self) -> ReadSlotWaiter { - self.inner.read_slot_waiter() - } - - fn submit_reserved_read( - &self, - slot: ReadSlot, - operation: IoOperation, - ) -> Result { - self.inner.submit_reserved_read(slot, operation) - } - - #[cfg(test)] - fn submit(&self, operation: IoOperation) -> Result { - self.inner.submit(operation) - } - - #[cfg(test)] - fn submit_wait(&self, operation: IoOperation) -> Result { - self.inner.submit_wait(operation) - } - - fn submit_wait_controlled( - &self, - operation: IoOperation, - cancelled: &AtomicBool, - deadline: Option, - ) -> Result { - self.inner - .submit_wait_controlled(operation, cancelled, deadline) - } - - fn wake_slot_waiters(&self) { - self.inner.shared.wake_slot_waiters(); - } - - fn cancel(&self, request_id: RequestId, state: &CompletionState) -> io::Result { - self.inner.cancel(request_id, state) - } - - fn shutdown(&self) -> io::Result<()> { - self.inner.shutdown() - } - - fn in_flight(&self) -> usize { - self.inner.shared.total_in_flight() - } - - #[cfg(test)] - fn direct_active(&self) -> bool { - self.io_stats.snapshot().direct_active - } - - fn stop_accepting_requests(&self) { - self.inner.stop_accepting_requests(); - } - - fn writes_in_flight(&self) -> usize { - self.inner.shared.writes_in_flight() - } - - fn has_unfenced_writes(&self) -> bool { - self.inner.shared.has_unfenced_writes() + fn inner(&self) -> &Arc { + &self.inner } - #[cfg(test)] - fn mark_unfenced_writes_for_test(&self) { - self.inner.shared.mark_unfenced_writes(); - } - - fn stats(&self) -> EngineIoSnapshot { - EngineIoSnapshot { - requests: self.inner.shared.snapshot(), - runtime: self.io_stats.snapshot(), - } + fn runtime_io_stats(&self) -> RuntimeIoStats { + self.io_stats.snapshot() } } @@ -1032,21 +947,6 @@ impl UringDriver { } } -fn retry_interrupted(mut operation: impl FnMut() -> io::Result) -> io::Result { - let mut retries = 0_usize; - loop { - match operation() { - Err(error) - if error.kind() == io::ErrorKind::Interrupted - && retries < MAX_INTERRUPTED_RETRIES => - { - retries += 1; - } - result => return result, - } - } -} - impl Task { fn operation_is_empty(&self) -> bool { match &self.operation { diff --git a/cache2/src/lib.rs b/cache2/src/lib.rs index fb4010a..8c73851 100644 --- a/cache2/src/lib.rs +++ b/cache2/src/lib.rs @@ -60,6 +60,7 @@ pub use self::snapshot::RegionSnapshot; pub use self::snapshot::StartupMode; mod checksum; +mod codec; mod hashing; mod io; mod managed_memory; diff --git a/cache2/src/region/index/storage/mod.rs b/cache2/src/region/index/storage/mod.rs index 2a88474..0f95e46 100644 --- a/cache2/src/region/index/storage/mod.rs +++ b/cache2/src/region/index/storage/mod.rs @@ -40,11 +40,11 @@ use std::sync::atomic::Ordering; use self::page_format::PAGE_CHECKSUM_OFFSET; use self::page_format::encode_page_header; use self::page_format::page_checksum; -use self::page_format::put_u32; -#[cfg(test)] -use self::page_format::put_u64; use self::page_format::read_u64; use self::page_format::validate_page_header; +use crate::codec::put_u32; +#[cfg(test)] +use crate::codec::put_u64; use crate::region::index::packed::INDEX_CANDIDATES; use crate::region::index::packed::IndexEntry; use crate::region::index::packed::MAX_INDEX_PARTITIONS; @@ -117,6 +117,33 @@ pub struct IndexPartitionRange { pub slot_count: usize, } +impl IndexPartitionRange { + fn global_slot(&self, slot: usize) -> Result { + if slot >= self.slot_count { + return Err(IndexStorageError::SlotOutOfBounds { + slot, + slot_count: self.slot_count, + }); + } + self.first_slot + .checked_add(slot) + .ok_or(IndexStorageError::SizeOverflow) + } + + #[cfg(test)] + fn global_page(&self, page: usize) -> Result { + if page >= self.page_count { + return Err(IndexStorageError::PageOutOfBounds { + page, + page_count: self.page_count, + }); + } + self.first_page + .checked_add(page) + .ok_or(IndexStorageError::SizeOverflow) + } +} + /// Builds the stable page-balanced partition directory for one slot capacity. /// /// The partition count is the greatest usable power of two bounded by the physical @@ -883,30 +910,12 @@ impl IndexStorage { } fn global_slot(&self, slot: usize) -> Result { - if slot >= self.range.slot_count { - return Err(IndexStorageError::SlotOutOfBounds { - slot, - slot_count: self.range.slot_count, - }); - } - self.range - .first_slot - .checked_add(slot) - .ok_or(IndexStorageError::SizeOverflow) + self.range.global_slot(slot) } #[cfg(test)] fn global_page(&self, page: usize) -> Result { - if page >= self.range.page_count { - return Err(IndexStorageError::PageOutOfBounds { - page, - page_count: self.range.page_count, - }); - } - self.range - .first_page - .checked_add(page) - .ok_or(IndexStorageError::SizeOverflow) + self.range.global_page(page) } } @@ -1193,16 +1202,7 @@ impl IndexPartitionReadGuard<'_> { } pub fn global_slot(&self, slot: usize) -> Result { - if slot >= self.range.slot_count { - return Err(IndexStorageError::SlotOutOfBounds { - slot, - slot_count: self.range.slot_count, - }); - } - self.range - .first_slot - .checked_add(slot) - .ok_or(IndexStorageError::SizeOverflow) + self.range.global_slot(slot) } } @@ -1221,16 +1221,7 @@ impl IndexPartitionWriteGuard<'_> { } pub fn global_slot(&self, slot: usize) -> Result { - if slot >= self.range.slot_count { - return Err(IndexStorageError::SlotOutOfBounds { - slot, - slot_count: self.range.slot_count, - }); - } - self.range - .first_slot - .checked_add(slot) - .ok_or(IndexStorageError::SizeOverflow) + self.range.global_slot(slot) } pub fn replace_observed( @@ -1828,20 +1819,14 @@ unsafe impl Sync for Mapping {} #[cfg(test)] mod tests { - use std::env; - use std::fs::OpenOptions; use std::io::Read; use std::io::Seek; use std::io::SeekFrom; - use std::path::PathBuf; - use std::sync::atomic::AtomicU64; - use std::sync::atomic::Ordering; use super::*; + use crate::fixtures::TestFile; use crate::fixtures::assert_golden; - static NEXT_TEST_FILE: AtomicU64 = AtomicU64::new(0); - const fn binding(generation: u64) -> IndexImageBinding { IndexImageBinding { generation, @@ -1849,34 +1834,6 @@ mod tests { } } - struct TestFile { - path: PathBuf, - file: File, - } - - impl TestFile { - fn create() -> Self { - let id = NEXT_TEST_FILE.fetch_add(1, Ordering::Relaxed); - let path = env::temp_dir().join(format!( - "cache2-index-image-{}-{id}.tmp", - std::process::id() - )); - let file = OpenOptions::new() - .create_new(true) - .read(true) - .write(true) - .open(&path) - .unwrap(); - Self { path, file } - } - } - - impl Drop for TestFile { - fn drop(&mut self) { - let _ = std::fs::remove_file(&self.path); - } - } - fn sample_slot(seed: u64) -> IndexSlot { let location = PackedLocation::new( (seed % 64) as u32, @@ -2023,18 +1980,19 @@ mod tests { let (source, values) = populated_partitioned_storage(); let expected_partition_stats = source.partition_stats().unwrap(); - let mut test_file = TestFile::create(); + let test_file = TestFile::new("index-image"); + let mut file = test_file.create_new(); let written = source - .write_warm_image(&mut test_file.file, binding(GENERATION)) + .write_warm_image(&mut file, binding(GENERATION)) .unwrap(); assert_eq!(written.pages_written, 3); assert_eq!(written.slots_written, PARTITIONED_SLOT_COUNT); assert_eq!(written.bytes_written, (3 * INDEX_IMAGE_PAGE_SIZE) as u64); assert_eq!(written.physical_stats, source.physical_stats().unwrap()); - test_file.file.sync_all().unwrap(); + file.sync_all().unwrap(); let recovered = PartitionedIndexStorage::map_private( - &test_file.file, + &file, 0, PARTITIONED_SLOT_COUNT, binding(GENERATION), @@ -2158,11 +2116,12 @@ mod tests { .unwrap(); image[INDEX_IMAGE_PAGE_SIZE + INDEX_IMAGE_PAGE_HEADER_SIZE + 7] ^= 0x80; - let mut test_file = TestFile::create(); - test_file.file.write_all(&image).unwrap(); - test_file.file.sync_all().unwrap(); + let test_file = TestFile::new("index-image"); + let mut file = test_file.create_new(); + file.write_all(&image).unwrap(); + file.sync_all().unwrap(); let recovered = PartitionedIndexStorage::map_private( - &test_file.file, + &file, 0, SLOT_COUNT, binding(GENERATION), @@ -2209,11 +2168,12 @@ mod tests { let checksum = page_checksum(page); put_u32(page, PAGE_CHECKSUM_OFFSET, checksum); - let mut test_file = TestFile::create(); - test_file.file.write_all(&image).unwrap(); - test_file.file.sync_all().unwrap(); + let test_file = TestFile::new("index-image"); + let mut file = test_file.create_new(); + file.write_all(&image).unwrap(); + file.sync_all().unwrap(); let recovered = PartitionedIndexStorage::map_private( - &test_file.file, + &file, 0, SLOT_COUNT, binding(GENERATION), @@ -2300,10 +2260,11 @@ mod tests { #[test] fn mapped_physical_stats_must_fit_the_slot_capacity() { - let test_file = TestFile::create(); + let test_file = TestFile::new("index-image"); + let file = test_file.create_new(); assert!(matches!( IndexStorage::map_private( - &test_file.file, + &file, 0, 1, binding(1), @@ -2321,17 +2282,16 @@ mod tests { const PREFIX: usize = INDEX_IMAGE_PAGE_SIZE; let source = IndexStorage::anonymous(1).unwrap(); - let mut test_file = TestFile::create(); - test_file.file.set_len(PREFIX as u64).unwrap(); - test_file.file.seek(SeekFrom::Start(PREFIX as u64)).unwrap(); - source - .write_warm_image(&mut test_file.file, binding(47)) - .unwrap(); - test_file.file.sync_all().unwrap(); + let test_file = TestFile::new("index-image"); + let mut file = test_file.create_new(); + file.set_len(PREFIX as u64).unwrap(); + file.seek(SeekFrom::Start(PREFIX as u64)).unwrap(); + source.write_warm_image(&mut file, binding(47)).unwrap(); + file.sync_all().unwrap(); assert!(matches!( IndexStorage::map_private( - &test_file.file, + &file, (PREFIX + 1) as u64, 1, binding(47), @@ -2343,7 +2303,7 @@ mod tests { )); assert!(matches!( IndexStorage::map_private( - &test_file.file, + &file, PREFIX as u64, 1, IndexImageBinding { @@ -2370,16 +2330,17 @@ mod tests { .write_slot(INDEX_IMAGE_SLOTS_PER_PAGE + 2, sample_slot(2)) .unwrap(); - let mut test_file = TestFile::create(); - test_file.file.set_len(PREFIX as u64).unwrap(); - test_file.file.seek(SeekFrom::Start(PREFIX as u64)).unwrap(); + let test_file = TestFile::new("index-image"); + let mut file = test_file.create_new(); + file.set_len(PREFIX as u64).unwrap(); + file.seek(SeekFrom::Start(PREFIX as u64)).unwrap(); source - .write_warm_image(&mut test_file.file, binding(GENERATION)) + .write_warm_image(&mut file, binding(GENERATION)) .unwrap(); - test_file.file.sync_all().unwrap(); + file.sync_all().unwrap(); let mut recovered = IndexStorage::map_private( - &test_file.file, + &file, PREFIX as u64, SLOT_COUNT, binding(GENERATION), @@ -2403,14 +2364,12 @@ mod tests { assert_eq!(recovered.read_slot(0).unwrap(), private_value); drop(recovered); - test_file - .file - .seek(SeekFrom::Start( - (PREFIX + INDEX_IMAGE_PAGE_HEADER_SIZE) as u64, - )) - .unwrap(); + file.seek(SeekFrom::Start( + (PREFIX + INDEX_IMAGE_PAGE_HEADER_SIZE) as u64, + )) + .unwrap(); let mut encoded = [0_u8; INDEX_IMAGE_SLOT_SIZE]; - test_file.file.read_exact(&mut encoded).unwrap(); + file.read_exact(&mut encoded).unwrap(); assert_eq!(IndexSlot::decode(&encoded), expected); } @@ -2431,18 +2390,14 @@ mod tests { )); let mut image = Vec::new(); source.write_warm_image(&mut image, binding(7)).unwrap(); - let mut test_file = TestFile::create(); - test_file.file.write_all(&image).unwrap(); - test_file.file.sync_all().unwrap(); + let test_file = TestFile::new("index-image"); + let mut file = test_file.create_new(); + file.write_all(&image).unwrap(); + file.sync_all().unwrap(); - let recovered = IndexStorage::map_private( - &test_file.file, - 0, - 1, - binding(8), - IndexPhysicalStats::default(), - ) - .unwrap(); + let recovered = + IndexStorage::map_private(&file, 0, 1, binding(8), IndexPhysicalStats::default()) + .unwrap(); assert_eq!( recovered.page_validation_state(0).unwrap(), PageValidationState::Unchecked @@ -2459,7 +2414,7 @@ mod tests { )); let recovered = IndexStorage::map_private( - &test_file.file, + &file, 0, 1, IndexImageBinding { diff --git a/cache2/src/region/index/storage/page_format.rs b/cache2/src/region/index/storage/page_format.rs index e6571a3..4b9ec0e 100644 --- a/cache2/src/region/index/storage/page_format.rs +++ b/cache2/src/region/index/storage/page_format.rs @@ -12,7 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::checksum::Crc32c; +use crate::codec::crc32c_with_zeroed_u32; +use crate::codec::put_u16; +use crate::codec::put_u32; +use crate::codec::put_u64; use crate::region::index::storage::CorruptPageReason; use crate::region::index::storage::IndexImageBinding; use crate::region::index::storage::IndexStorageError; @@ -195,11 +198,8 @@ pub fn validate_page_header( } pub fn page_checksum(page: &[u8; INDEX_IMAGE_PAGE_SIZE]) -> u32 { - let mut checksum = Crc32c::new(); - checksum.update(&page[..PAGE_CHECKSUM_OFFSET]); - checksum.update(&[0_u8; size_of::()]); - checksum.update(&page[PAGE_CHECKSUM_OFFSET + size_of::()..]); - checksum.finish() + crc32c_with_zeroed_u32(page, PAGE_CHECKSUM_OFFSET) + .expect("index page checksum field is in bounds") } fn read_u16(input: &[u8], offset: usize) -> u16 { @@ -225,15 +225,3 @@ pub fn read_u64(input: &[u8], offset: usize) -> u64 { .expect("fixed u64 field is in bounds"), ) } - -fn put_u16(output: &mut [u8], offset: usize, value: u16) { - output[offset..offset + 2].copy_from_slice(&value.to_le_bytes()); -} - -pub fn put_u32(output: &mut [u8], offset: usize, value: u32) { - output[offset..offset + 4].copy_from_slice(&value.to_le_bytes()); -} - -pub fn put_u64(output: &mut [u8], offset: usize, value: u64) { - output[offset..offset + 8].copy_from_slice(&value.to_le_bytes()); -} diff --git a/cache2/src/region/mod.rs b/cache2/src/region/mod.rs index d34e365..aa95545 100644 --- a/cache2/src/region/mod.rs +++ b/cache2/src/region/mod.rs @@ -647,7 +647,7 @@ impl FileRegionCore { match submit_read(engine, slot, descriptor, buffer) { Ok(pending) => Ok(pending), Err(error) => { - if !is_read_availability_error(error.kind()) { + if !is_read_pressure(error.kind()) { self.health .enter_miss_only_with_error("record_read_submit_failed", &error); } @@ -663,7 +663,7 @@ impl FileRegionCore { ) -> io::Result> { let hash = completion.descriptor.hash; if let Err(error) = completion.result { - if !is_read_availability_error(error.kind()) { + if !is_read_pressure(error.kind()) { self.health .enter_miss_only_with_error("record_read_completion_failed", &error); } @@ -1172,7 +1172,10 @@ impl FileRegionCore { } } -fn is_read_availability_error(kind: io::ErrorKind) -> bool { +/// Classifies transient read-availability failures: the caller may retry on an +/// alternate lane or surface a busy miss, but the failure says nothing about +/// the stored data and must not trip Region health. +pub fn is_read_pressure(kind: io::ErrorKind) -> bool { matches!( kind, io::ErrorKind::OutOfMemory diff --git a/cache2/src/region/record/mod.rs b/cache2/src/region/record/mod.rs index e238eea..132a0ac 100644 --- a/cache2/src/region/record/mod.rs +++ b/cache2/src/region/record/mod.rs @@ -17,8 +17,14 @@ //! These types are deliberately encoded field-by-field. Their Rust layout is //! not part of the disk format. -use crate::checksum::Crc32c; use crate::checksum::crc32c; +use crate::codec::crc32c_with_zeroed_u32_matches; +use crate::codec::get_u16; +use crate::codec::get_u32; +use crate::codec::get_u64; +use crate::codec::put_u16; +use crate::codec::put_u32; +use crate::codec::put_u64; pub mod codec; @@ -90,7 +96,7 @@ impl RecordHeader { if input.len() != RECORD_HEADER_SIZE || input.get(..RECORD_HEADER_MAGIC.len())? != RECORD_HEADER_MAGIC || get_u16(input, RECORD_VERSION_OFFSET)? != RECORD_FORMAT_VERSION - || !checksum_matches(input, RECORD_HEADER_CRC_OFFSET) + || !crc32c_with_zeroed_u32_matches(input, RECORD_HEADER_CRC_OFFSET) { return None; } @@ -136,61 +142,6 @@ fn checked_align_up(value: usize, alignment: usize) -> Option { .map(|rounded| rounded & !(alignment - 1)) } -fn checksum_matches(input: &[u8], checksum_offset: usize) -> bool { - let Some(expected) = get_u32(input, checksum_offset) else { - return false; - }; - let Some(after_checksum) = checksum_offset.checked_add(size_of::()) else { - return false; - }; - let (Some(before), Some(after)) = (input.get(..checksum_offset), input.get(after_checksum..)) - else { - return false; - }; - - let mut checksum = Crc32c::new(); - checksum.update(before); - checksum.update(&[0; size_of::()]); - checksum.update(after); - checksum.finish() == expected -} - -fn get_u16(input: &[u8], offset: usize) -> Option { - let bytes: [u8; size_of::()] = input - .get(offset..offset.checked_add(size_of::())?)? - .try_into() - .ok()?; - Some(u16::from_le_bytes(bytes)) -} - -fn get_u32(input: &[u8], offset: usize) -> Option { - let bytes: [u8; size_of::()] = input - .get(offset..offset.checked_add(size_of::())?)? - .try_into() - .ok()?; - Some(u32::from_le_bytes(bytes)) -} - -fn get_u64(input: &[u8], offset: usize) -> Option { - let bytes: [u8; size_of::()] = input - .get(offset..offset.checked_add(size_of::())?)? - .try_into() - .ok()?; - Some(u64::from_le_bytes(bytes)) -} - -fn put_u16(output: &mut [u8], offset: usize, value: u16) { - output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); -} - -fn put_u32(output: &mut [u8], offset: usize, value: u32) { - output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); -} - -fn put_u64(output: &mut [u8], offset: usize, value: u64) { - output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); -} - #[cfg(test)] mod tests { use super::*; diff --git a/cache2/src/region/recovery/metadata.rs b/cache2/src/region/recovery/metadata.rs index b6cc473..c0a8579 100644 --- a/cache2/src/region/recovery/metadata.rs +++ b/cache2/src/region/recovery/metadata.rs @@ -21,7 +21,10 @@ use std::fmt; use std::mem; -use crate::checksum::Crc32c; +use crate::codec::crc32c_with_zeroed_u32; +use crate::codec::put_u16; +use crate::codec::put_u32; +use crate::codec::put_u64; use crate::region::index::packed::MAX_INDEX_PARTITIONS; use crate::region::index::packed::MAX_PACKED_REGION_COUNT; use crate::region::index::packed::MAX_PACKED_REGION_SIZE; @@ -855,17 +858,12 @@ fn validate_envelope_shape( } fn finish_page(page: &mut [u8]) { - put_u32(page, PAGE_CRC_OFFSET, 0); let checksum = page_crc(page); put_u32(page, PAGE_CRC_OFFSET, checksum); } fn page_crc(page: &[u8]) -> u32 { - let mut checksum = Crc32c::new(); - checksum.update(&page[..PAGE_CRC_OFFSET]); - checksum.update(&[0_u8; 4]); - checksum.update(&page[PAGE_CRC_OFFSET + 4..]); - checksum.finish() + crc32c_with_zeroed_u32(page, PAGE_CRC_OFFSET).expect("metadata page CRC field is in bounds") } #[allow(clippy::too_many_arguments)] @@ -1175,42 +1173,15 @@ fn put_id(output: &mut [u8], offset: usize, id: PersistentId) { } fn get_u16(input: &[u8], offset: usize) -> Result { - let bytes = input - .get(offset..offset + 2) - .ok_or(RegionMetadataError::InvalidLength)? - .try_into() - .map_err(|_| RegionMetadataError::InvalidLength)?; - Ok(u16::from_le_bytes(bytes)) + crate::codec::get_u16(input, offset).ok_or(RegionMetadataError::InvalidLength) } fn get_u32(input: &[u8], offset: usize) -> Result { - let bytes = input - .get(offset..offset + 4) - .ok_or(RegionMetadataError::InvalidLength)? - .try_into() - .map_err(|_| RegionMetadataError::InvalidLength)?; - Ok(u32::from_le_bytes(bytes)) + crate::codec::get_u32(input, offset).ok_or(RegionMetadataError::InvalidLength) } fn get_u64(input: &[u8], offset: usize) -> Result { - let bytes = input - .get(offset..offset + 8) - .ok_or(RegionMetadataError::InvalidLength)? - .try_into() - .map_err(|_| RegionMetadataError::InvalidLength)?; - Ok(u64::from_le_bytes(bytes)) -} - -fn put_u16(output: &mut [u8], offset: usize, value: u16) { - output[offset..offset + 2].copy_from_slice(&value.to_le_bytes()); -} - -fn put_u32(output: &mut [u8], offset: usize, value: u32) { - output[offset..offset + 4].copy_from_slice(&value.to_le_bytes()); -} - -fn put_u64(output: &mut [u8], offset: usize, value: u64) { - output[offset..offset + 8].copy_from_slice(&value.to_le_bytes()); + crate::codec::get_u64(input, offset).ok_or(RegionMetadataError::InvalidLength) } #[cfg(test)] diff --git a/cache2/src/region/recovery/mod.rs b/cache2/src/region/recovery/mod.rs index c184fd2..efe5cf3 100644 --- a/cache2/src/region/recovery/mod.rs +++ b/cache2/src/region/recovery/mod.rs @@ -20,8 +20,14 @@ //! `CLEAN`. This module performs no I/O; callers must write the returned page //! to the selected slot and provide the required `fdatasync` barrier. -use crate::checksum::Crc32c; -use crate::checksum::crc32c; +use crate::codec::crc32c_with_zeroed_u32; +use crate::codec::crc32c_with_zeroed_u32_matches; +use crate::codec::get_u16; +use crate::codec::get_u32; +use crate::codec::get_u64; +use crate::codec::put_u16; +use crate::codec::put_u32; +use crate::codec::put_u64; use crate::region::index::packed::MAX_PACKED_REGION_COUNT; use crate::region::index::packed::MAX_PACKED_REGION_SIZE; use crate::region::index::storage::page_format::INDEX_IMAGE_PAGE_SIZE; @@ -877,62 +883,14 @@ fn get_id(input: &[u8], offset: usize) -> Option { PersistentId::from_bytes(input.get(offset..offset + 16)?.try_into().ok()?) } -fn put_u16(output: &mut [u8], offset: usize, value: u16) { - output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); -} - -fn put_u32(output: &mut [u8], offset: usize, value: u32) { - output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); -} - -fn put_u64(output: &mut [u8], offset: usize, value: u64) { - output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); -} - -fn get_u16(input: &[u8], offset: usize) -> Option { - Some(u16::from_le_bytes( - input - .get(offset..offset + size_of::())? - .try_into() - .ok()?, - )) -} - -fn get_u32(input: &[u8], offset: usize) -> Option { - Some(u32::from_le_bytes( - input - .get(offset..offset + size_of::())? - .try_into() - .ok()?, - )) -} - -fn get_u64(input: &[u8], offset: usize) -> Option { - Some(u64::from_le_bytes( - input - .get(offset..offset + size_of::())? - .try_into() - .ok()?, - )) -} - fn write_page_crc(page: &mut [u8; RECOVERY_PAGE_SIZE]) { - put_u32(page, PAGE_CRC_OFFSET, 0); - let checksum = crc32c(page); + let checksum = crc32c_with_zeroed_u32(page, PAGE_CRC_OFFSET) + .expect("recovery page CRC field is in bounds"); put_u32(page, PAGE_CRC_OFFSET, checksum); } fn page_crc_matches(page: &[u8]) -> bool { - if page.len() != RECOVERY_PAGE_SIZE { - return false; - } - let Some(expected) = get_u32(page, PAGE_CRC_OFFSET) else { - return false; - }; - let mut checksum = Crc32c::new(); - checksum.update(&page[..PAGE_CRC_OFFSET]); - checksum.update(&[0; size_of::()]); - checksum.finish() == expected + page.len() == RECOVERY_PAGE_SIZE && crc32c_with_zeroed_u32_matches(page, PAGE_CRC_OFFSET) } #[cfg(test)] diff --git a/cache2/src/region/runtime/mod.rs b/cache2/src/region/runtime/mod.rs index 13f20f1..e97d7a6 100644 --- a/cache2/src/region/runtime/mod.rs +++ b/cache2/src/region/runtime/mod.rs @@ -84,6 +84,7 @@ use crate::region::index::packed::PackedLocation; use crate::region::index::storage::page_format::INDEX_IMAGE_PAGE_SIZE; #[cfg(test)] use crate::region::index::storage::page_format::INDEX_IMAGE_SLOTS_PER_PAGE; +use crate::region::is_read_pressure; use crate::region::reader::PendingRead; #[cfg(test)] use crate::region::reader::ReadCandidate; @@ -2157,17 +2158,6 @@ fn write_overload_error() -> io::Error { io::Error::new(io::ErrorKind::WouldBlock, "write path is busy") } -fn is_read_pressure(kind: io::ErrorKind) -> bool { - matches!( - kind, - io::ErrorKind::OutOfMemory - | io::ErrorKind::WouldBlock - | io::ErrorKind::TimedOut - | io::ErrorKind::Interrupted - | io::ErrorKind::BrokenPipe - ) -} - fn staging_runtime_error(error: StagingError) -> io::Error { io::Error::new(io::ErrorKind::InvalidData, error.to_string()) } @@ -2198,8 +2188,7 @@ mod tests { use std::task::Waker; use super::*; - use crate::io::backend::FileBackend; - use crate::io::backend::IoBackend; + use crate::fixtures::TestFile; use crate::io::engine::BackendIoEngine; static LANE_TEST_ID: AtomicU64 = AtomicU64::new(1); @@ -2218,12 +2207,8 @@ mod tests { #[test] fn read_lane_uses_one_bounded_alternate_on_primary_pressure() { - let id = LANE_TEST_ID.fetch_add(1, Ordering::Relaxed); - let path = env::temp_dir().join(format!( - "cache2-read-lane-{}-{id}.cache", - std::process::id() - )); - let backend: Arc = Arc::new(FileBackend::open(&path).unwrap()); + let file = TestFile::new("read-lane"); + let backend = file.backend(); let engines: Box<[Arc]> = vec![ Arc::new(BackendIoEngine::new(Arc::clone(&backend), 1).unwrap()) as Arc, Arc::new(BackendIoEngine::new(Arc::clone(&backend), 1).unwrap()) as Arc, @@ -2255,17 +2240,12 @@ mod tests { } drop(engines); drop(backend); - std::fs::remove_file(path).unwrap(); } #[test] fn hot_read_route_rotates_pressure_fallback_across_all_lanes() { - let id = LANE_TEST_ID.fetch_add(1, Ordering::Relaxed); - let path = env::temp_dir().join(format!( - "cache2-read-lane-rotation-{}-{id}.cache", - std::process::id() - )); - let backend: Arc = Arc::new(FileBackend::open(&path).unwrap()); + let file = TestFile::new("read-lane-rotation"); + let backend = file.backend(); let engines: Box<[Arc]> = (0..4) .map(|_| { Arc::new(BackendIoEngine::new(Arc::clone(&backend), 1).unwrap()) @@ -2289,7 +2269,6 @@ mod tests { } drop(engines); drop(backend); - std::fs::remove_file(path).unwrap(); } #[test] diff --git a/cache2/src/region/runtime/shutdown_tests.rs b/cache2/src/region/runtime/shutdown_tests.rs index 597f76b..056a79a 100644 --- a/cache2/src/region/runtime/shutdown_tests.rs +++ b/cache2/src/region/runtime/shutdown_tests.rs @@ -19,16 +19,13 @@ use std::sync::mpsc; use super::*; use crate::IoEngineOptions; use crate::io::backend::IoBackend; +use crate::io::backend::RuntimeIoStats; use crate::io::backend::SyncMode; use crate::io::backend::SyncPoint; use crate::io::backend::WritePoint; use crate::io::engine::BackendIoEngine; -use crate::io::engine::CompletionState; -use crate::io::engine::EngineIoSnapshot; use crate::io::engine::IoRequest; -use crate::io::engine::ReadSlotWaiter; -use crate::io::engine::RequestId; -use crate::io::engine::SubmitError; +use crate::io::engine::RuntimeInner; #[derive(Default)] struct BlockedReadState { @@ -104,52 +101,12 @@ struct RacingEngine { } impl IoEngine for RacingEngine { - fn set_latency_recorder(&self, recorder: crate::stats::recording::IoTiming) { - self.inner.set_latency_recorder(recorder); - } - fn try_reserve_read(&self) -> io::Result { - self.inner.try_reserve_read() - } - - fn read_slot_waiter(&self) -> ReadSlotWaiter { - self.inner.read_slot_waiter() - } - - fn submit_reserved_read( - &self, - slot: ReadSlot, - op: IoOperation, - ) -> Result { - self.inner.submit_reserved_read(slot, op) - } - - fn submit(&self, op: IoOperation) -> Result { - self.inner.submit(op) + fn inner(&self) -> &Arc { + self.inner.inner() } - fn submit_wait(&self, op: IoOperation) -> Result { - self.inner.submit_wait(op) - } - - fn submit_wait_controlled( - &self, - op: IoOperation, - cancel: &AtomicBool, - deadline: Option, - ) -> Result { - self.inner.submit_wait_controlled(op, cancel, deadline) - } - - fn wake_slot_waiters(&self) { - self.inner.wake_slot_waiters(); - } - - fn cancel(&self, id: RequestId, state: &CompletionState) -> io::Result { - self.inner.cancel(id, state) - } - - fn shutdown(&self) -> io::Result<()> { - self.inner.shutdown() + fn runtime_io_stats(&self) -> RuntimeIoStats { + self.inner.runtime_io_stats() } fn in_flight(&self) -> usize { @@ -171,30 +128,6 @@ impl IoEngine for RacingEngine { } observed } - - fn direct_active(&self) -> bool { - false - } - - fn stop_accepting_requests(&self) { - self.inner.stop_accepting_requests(); - } - - fn writes_in_flight(&self) -> usize { - self.inner.writes_in_flight() - } - - fn has_unfenced_writes(&self) -> bool { - self.inner.has_unfenced_writes() - } - - fn mark_unfenced_writes_for_test(&self) { - self.inner.mark_unfenced_writes_for_test(); - } - - fn stats(&self) -> EngineIoSnapshot { - self.inner.stats() - } } #[test] From 70e236697b5ee7fb4b9a9f6d6a419b79c1ebfe77 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 20 Sep 2026 15:30:29 +0800 Subject: [PATCH 2/7] fix: reject invalid benchmark enum environment values Use the shared setting reader for I/O mode and L1 eviction policy so non-UTF-8 values fail instead of selecting defaults. Reuse boolean parsing for IOPOLL and keep the generic setting helper private. Add a subprocess regression test that supplies invalid environment bytes without mutating the parallel test process. --- benchmarks/src/config.rs | 64 +++++++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/benchmarks/src/config.rs b/benchmarks/src/config.rs index 1891af3..6560070 100644 --- a/benchmarks/src/config.rs +++ b/benchmarks/src/config.rs @@ -158,10 +158,7 @@ pub fn env_usize_list(name: &str, default: &[usize]) -> io::Result> /// Reads an I/O mode setting (`buffered` or `direct`, default `buffered`). pub fn parse_io_mode(name: &str) -> io::Result { - match env::var(name) - .unwrap_or_else(|_| "buffered".to_owned()) - .as_str() - { + match setting::(name)?.as_deref().unwrap_or("buffered") { "buffered" => Ok(IoMode::Buffered), "direct" => Ok(IoMode::Direct), value => Err(invalid(format!("unsupported I/O mode: {value}"))), @@ -170,10 +167,7 @@ pub fn parse_io_mode(name: &str) -> io::Result { /// Reads an L1 eviction policy setting (`clock` or `s3-fifo`, default `clock`). pub fn parse_l1_eviction_policy(name: &str) -> io::Result { - match env::var(name) - .unwrap_or_else(|_| "clock".to_owned()) - .as_str() - { + match setting::(name)?.as_deref().unwrap_or("clock") { "clock" => Ok(L1EvictionPolicy::Clock), "s3-fifo" => Ok(L1EvictionPolicy::S3Fifo), value => Err(invalid(format!("unsupported L1 eviction policy: {value}"))), @@ -190,15 +184,7 @@ fn io_uring_pool( let mut options = IoUringPoolOptions::default(); options.rings = setting(&format!("{prefix}_RINGS"))?.unwrap_or(rings); options.max_in_flight = setting(&format!("{prefix}_MAX_IN_FLIGHT"))?.unwrap_or(max_in_flight); - options.io_poll = match setting::(&format!("{prefix}_IOPOLL"))?.as_deref() { - None | Some("false" | "0") => false, - Some("true" | "1") => true, - Some(_) => { - return Err(invalid(format!( - "{prefix}_IOPOLL must be true, false, 1, or 0" - ))); - } - }; + options.io_poll = env_bool(&format!("{prefix}_IOPOLL"), false)?; let idle = setting(&format!("{prefix}_SQPOLL_MS"))?; let cpu = setting(&format!("{prefix}_SQPOLL_CPU"))?; if let Some(idle) = idle { @@ -214,7 +200,7 @@ fn io_uring_pool( } /// Reads an optional typed setting, reporting the raw value on parse failure. -pub fn setting(name: &str) -> io::Result> { +fn setting(name: &str) -> io::Result> { match env::var(name) { Ok(value) => value .parse() @@ -229,3 +215,45 @@ pub fn setting(name: &str) -> io::Result> { pub fn invalid(message: impl Into) -> io::Error { io::Error::new(io::ErrorKind::InvalidInput, message.into()) } + +#[cfg(all(test, unix))] +mod tests { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + use std::process::Command; + + use super::*; + + #[test] + fn non_utf8_enum_settings_are_rejected() { + const SETTING: &str = "CACHE2_TEST_NON_UTF8_ENUM_SETTING"; + + if env::var_os(SETTING).is_some() { + assert_eq!( + parse_io_mode(SETTING).unwrap_err().kind(), + io::ErrorKind::InvalidInput + ); + assert_eq!( + parse_l1_eviction_policy(SETTING).unwrap_err().kind(), + io::ErrorKind::InvalidInput + ); + return; + } + + // Set the child's environment without mutating the parallel test process. + let output = Command::new(env::current_exe().unwrap()) + .args([ + "--exact", + "config::tests::non_utf8_enum_settings_are_rejected", + ]) + .env(SETTING, OsStr::from_bytes(b"\xff")) + .output() + .unwrap(); + assert!( + output.status.success(), + "child test failed:\n{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } +} From 9465cd463c818372ffa6a3736b223a964e067bf2 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 20 Sep 2026 15:30:40 +0800 Subject: [PATCH 3/7] refactor: borrow I/O runtime state without exposing Arc Return a plain RuntimeInner reference from IoEngine accessors. Default operations only borrow the runtime, while each implementation retains its own Arc ownership. --- cache2/src/io/engine/mod.rs | 2 +- cache2/src/io/engine/posix.rs | 2 +- cache2/src/io/engine/uring.rs | 2 +- cache2/src/region/runtime/shutdown_tests.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cache2/src/io/engine/mod.rs b/cache2/src/io/engine/mod.rs index 9453609..2c57330 100644 --- a/cache2/src/io/engine/mod.rs +++ b/cache2/src/io/engine/mod.rs @@ -954,7 +954,7 @@ fn submit_cache_io_until( pub trait IoEngine: Send + Sync { /// Shared submission, completion, and bookkeeping state behind the /// engine-specific driver. - fn inner(&self) -> &Arc; + fn inner(&self) -> &RuntimeInner; /// Backend path and direct-I/O counters reported with the engine stats. fn runtime_io_stats(&self) -> RuntimeIoStats; diff --git a/cache2/src/io/engine/posix.rs b/cache2/src/io/engine/posix.rs index 670841c..189c040 100644 --- a/cache2/src/io/engine/posix.rs +++ b/cache2/src/io/engine/posix.rs @@ -163,7 +163,7 @@ impl BackendIoEngine { } impl IoEngine for BackendIoEngine { - fn inner(&self) -> &Arc { + fn inner(&self) -> &RuntimeInner { &self.inner } diff --git a/cache2/src/io/engine/uring.rs b/cache2/src/io/engine/uring.rs index 1d3f1ea..f5a0edb 100644 --- a/cache2/src/io/engine/uring.rs +++ b/cache2/src/io/engine/uring.rs @@ -241,7 +241,7 @@ impl UringIoEngine { } impl IoEngine for UringIoEngine { - fn inner(&self) -> &Arc { + fn inner(&self) -> &RuntimeInner { &self.inner } diff --git a/cache2/src/region/runtime/shutdown_tests.rs b/cache2/src/region/runtime/shutdown_tests.rs index 056a79a..e6dd1a9 100644 --- a/cache2/src/region/runtime/shutdown_tests.rs +++ b/cache2/src/region/runtime/shutdown_tests.rs @@ -101,7 +101,7 @@ struct RacingEngine { } impl IoEngine for RacingEngine { - fn inner(&self) -> &Arc { + fn inner(&self) -> &RuntimeInner { self.inner.inner() } From 4a7d2900a0a02102d65acc5d6708b499ab039c8f Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 20 Sep 2026 15:30:40 +0800 Subject: [PATCH 4/7] refactor: reuse shared field readers for index pages Delegate fixed-width index readers to the shared codec and retain local expects for their internal bounds contract. Existing golden fixtures cover the persisted representation. --- .../src/region/index/storage/page_format.rs | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/cache2/src/region/index/storage/page_format.rs b/cache2/src/region/index/storage/page_format.rs index 4b9ec0e..edd67fb 100644 --- a/cache2/src/region/index/storage/page_format.rs +++ b/cache2/src/region/index/storage/page_format.rs @@ -13,6 +13,9 @@ // limitations under the License. use crate::codec::crc32c_with_zeroed_u32; +use crate::codec::get_u16; +use crate::codec::get_u32; +use crate::codec::get_u64; use crate::codec::put_u16; use crate::codec::put_u32; use crate::codec::put_u64; @@ -203,25 +206,13 @@ pub fn page_checksum(page: &[u8; INDEX_IMAGE_PAGE_SIZE]) -> u32 { } fn read_u16(input: &[u8], offset: usize) -> u16 { - u16::from_le_bytes( - input[offset..offset + 2] - .try_into() - .expect("fixed u16 field is in bounds"), - ) + get_u16(input, offset).expect("fixed u16 field is in bounds") } fn read_u32(input: &[u8], offset: usize) -> u32 { - u32::from_le_bytes( - input[offset..offset + 4] - .try_into() - .expect("fixed u32 field is in bounds"), - ) + get_u32(input, offset).expect("fixed u32 field is in bounds") } pub fn read_u64(input: &[u8], offset: usize) -> u64 { - u64::from_le_bytes( - input[offset..offset + 8] - .try_into() - .expect("fixed u64 field is in bounds"), - ) + get_u64(input, offset).expect("fixed u64 field is in bounds") } From 8454850dbd57e6db597f09801a75174d3dd9dcf0 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 20 Sep 2026 16:43:51 +0800 Subject: [PATCH 5/7] test: coordinate shutdown races without an engine wrapper Inject a competing read at the shutdown snapshot boundary through a per-runtime test callback. Keep issued requests observable and verify that late submissions are rejected by the admission fence. Both shutdown tests pass; temporarily removing the fence makes the late-read regression fail. --- cache2/src/region/runtime/mod.rs | 11 +++ cache2/src/region/runtime/shutdown_tests.rs | 82 ++++++++------------- 2 files changed, 42 insertions(+), 51 deletions(-) diff --git a/cache2/src/region/runtime/mod.rs b/cache2/src/region/runtime/mod.rs index e97d7a6..4f6b522 100644 --- a/cache2/src/region/runtime/mod.rs +++ b/cache2/src/region/runtime/mod.rs @@ -446,6 +446,8 @@ struct RunningShared { write_flush_threshold_bytes: usize, align_reads_for_direct_io: bool, activity_counters: bool, + #[cfg(test)] + after_io_snapshot: Mutex>>, } #[derive(Default)] @@ -1499,6 +1501,8 @@ fn start_running( write_flush_threshold_bytes: runtime.write_flush_threshold_bytes, align_reads_for_direct_io: runtime.io_mode == IoMode::Direct, activity_counters: runtime.stats.activity_counters, + #[cfg(test)] + after_io_snapshot: Mutex::new(None), }); // Inspect the recovered queue before workers can contend with foreground // mutations. Fresh caches have no sealed Regions and need no wakeup. @@ -2069,6 +2073,13 @@ fn stop_running(mut owner: RunningOwner) -> io::Result { .engines() .map(|engine| engine.in_flight()) .sum::(); + #[cfg(test)] + { + let after_snapshot = owner.shared.after_io_snapshot.lock().unwrap().take(); + if let Some(after_snapshot) = after_snapshot { + after_snapshot(); + } + } let writes_in_flight = owner .shared .engines() diff --git a/cache2/src/region/runtime/shutdown_tests.rs b/cache2/src/region/runtime/shutdown_tests.rs index e6dd1a9..11a0f46 100644 --- a/cache2/src/region/runtime/shutdown_tests.rs +++ b/cache2/src/region/runtime/shutdown_tests.rs @@ -13,19 +13,16 @@ // limitations under the License. use std::env; -use std::sync::atomic::AtomicBool; use std::sync::mpsc; use super::*; use crate::IoEngineOptions; use crate::io::backend::IoBackend; -use crate::io::backend::RuntimeIoStats; use crate::io::backend::SyncMode; use crate::io::backend::SyncPoint; use crate::io::backend::WritePoint; use crate::io::engine::BackendIoEngine; use crate::io::engine::IoRequest; -use crate::io::engine::RuntimeInner; #[derive(Default)] struct BlockedReadState { @@ -92,44 +89,6 @@ impl IoBackend for BlockedRead { } } -struct RacingEngine { - inner: BackendIoEngine, - backend: Arc, - managed_memory: Arc, - inject: AtomicBool, - pending: Mutex>, -} - -impl IoEngine for RacingEngine { - fn inner(&self) -> &RuntimeInner { - self.inner.inner() - } - - fn runtime_io_stats(&self) -> RuntimeIoStats { - self.inner.runtime_io_stats() - } - - fn in_flight(&self) -> usize { - let observed = self.inner.in_flight(); - if self.inject.swap(false, Ordering::AcqRel) { - // Schedule a competing read immediately after the idle observation. - if let Ok(slot) = self.inner.try_reserve_read() { - let buffer = - IoBuffer::for_read(self.managed_memory.try_read_buffer(4096).unwrap(), 4096) - .unwrap(); - if let Ok(request) = self - .inner - .submit_reserved_read(slot, IoOperation::read(buffer, 0)) - { - *self.pending.lock().unwrap() = Some(request); - self.backend.wait_started(); - } - } - } - observed - } -} - #[test] fn late_read_must_not_pin_close() { assert_close_does_not_wait_for_read(false); @@ -184,16 +143,29 @@ fn assert_close_does_not_wait_for_read(submit_before_close: bool) { // Reuse a stopped runtime's fixed resources without unrelated workers. let shared = Arc::get_mut(&mut plane.shared).unwrap(); let backend = Arc::new(BlockedRead::default()); - let engine = Arc::new(RacingEngine { - inner: BackendIoEngine::new(backend.clone(), 1).unwrap(), - backend: backend.clone(), - managed_memory: shared.managed_memory.clone(), - inject: AtomicBool::new(true), - pending: Mutex::new(None), - }); + let engine = Arc::new(BackendIoEngine::new(backend.clone(), 1).unwrap()); + let read_engine = Arc::clone(&engine); + let read_backend = Arc::clone(&backend); + let managed_memory = Arc::clone(&shared.managed_memory); + let submit_read = move || -> io::Result { + let slot = read_engine.try_reserve_read()?; + let buffer = + IoBuffer::for_read(managed_memory.try_read_buffer(4096).unwrap(), 4096).unwrap(); + let request = read_engine + .submit_reserved_read(slot, IoOperation::read(buffer, 0)) + .map_err(|error| error.error)?; + read_backend.wait_started(); + Ok(request) + }; + let (submitted, submission) = mpsc::channel(); if submit_before_close { - assert_eq!(engine.in_flight(), 0); - assert_eq!(engine.inner.in_flight(), 1); + submitted.send(submit_read()).unwrap(); + assert_eq!(engine.in_flight(), 1); + } else { + // Exercise the interval between the idle snapshot and synchronous shutdown. + *shared.after_io_snapshot.get_mut().unwrap() = Some(Box::new(move || { + submitted.send(submit_read()).unwrap(); + })); } shared.read_engines = vec![engine.clone() as Arc].into_boxed_slice(); shared.write_engines = Box::new([]); @@ -213,10 +185,18 @@ fn assert_close_does_not_wait_for_read(submit_before_close: bool) { backend.release(); thread.join().unwrap(); engine.shutdown().unwrap(); - assert!(!engine.inject.load(Ordering::Acquire)); std::fs::remove_dir_all(root).unwrap(); assert!( matches!(result, Ok(Ok(false))), "close synchronously joined a blocked read" ); + let submitted = submission.recv_timeout(Duration::from_secs(1)).unwrap(); + if submit_before_close { + assert!(matches!( + submitted.unwrap().wait().status, + crate::io::engine::CompletionStatus::Completed + )); + } else { + assert_eq!(submitted.unwrap_err().kind(), io::ErrorKind::BrokenPipe); + } } From 8c7e856ae0ac43e4835d5c02c4d798448ebfd96d Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 20 Sep 2026 16:52:08 +0800 Subject: [PATCH 6/7] refactor: share one concrete I/O engine across drivers Replace the engine trait and RuntimeInner wrappers with a concrete IoEngine. Keep driver selection in constructors and share request admission, completion, cancellation, statistics, and shutdown through Arc. Use shared file statistics directly and retain workers as they start so partial startup failures use the same shutdown path. Preserve request-owned buffers and slots through physical completion. --- ARCHITECTURE.md | 2 + cache2/src/io/backend.rs | 42 +-- cache2/src/io/engine/mod.rs | 287 +++++++++----------- cache2/src/io/engine/posix.rs | 181 +++++------- cache2/src/io/engine/tests.rs | 88 +++--- cache2/src/io/engine/uring.rs | 225 ++++++--------- cache2/src/region/appender.rs | 10 +- cache2/src/region/file_backend/tests.rs | 7 +- cache2/src/region/mod.rs | 8 +- cache2/src/region/reader.rs | 14 +- cache2/src/region/runtime/mod.rs | 49 ++-- cache2/src/region/runtime/shutdown_tests.rs | 6 +- 12 files changed, 371 insertions(+), 548 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6585121..c61a08c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -112,6 +112,8 @@ The source Region stays pinned until accepted replacement writes finish. Conditi Reads and writes use independent bounded engine pools. Reclaim has separate read lanes. POSIX uses positioned worker I/O; optional io_uring uses fixed-depth rings. Buffered I/O is the default. Direct mode aligns runtime record I/O and keeps control, recovery, and unavoidable remainder operations buffered. Locks cover bounded in-memory work and release before device I/O. +Each lane uses one concrete `IoEngine` for admission, submission, cancellation, statistics, and shutdown. Driver-specific constructors start POSIX workers or an io_uring driver behind the same bounded command and completion protocol. Callers share the engine through `Arc`; its final owner joins the workers. Submitted requests retain their buffers and capacity until actual completion, independently of the caller's wait deadline. + ### Memory The managed-memory limit covers the index mapping, heat bits, L1, append buffers, reclaim buffers, metadata, cache-owned thread stacks, recovery scratch, and transient reads. Total deployment memory additionally includes allocator metadata, Tokio, process overhead, and the kernel page cache. `CacheConfig::new` rejects invalid or insufficient memory budgets before file access; actual allocation can still fail during open. diff --git a/cache2/src/io/backend.rs b/cache2/src/io/backend.rs index 835c0c5..ec0642e 100644 --- a/cache2/src/io/backend.rs +++ b/cache2/src/io/backend.rs @@ -127,7 +127,7 @@ impl RuntimeIoDirectionCounters { } impl RuntimeIoStatsHandle { - fn new(direct_active: bool) -> Self { + pub fn new(direct_active: bool) -> Self { Self { inner: Arc::new(RuntimeIoCounters { direct_active, @@ -229,28 +229,10 @@ impl RuntimeFileSet { self.stats.record(direction, path, length); } - #[cfg(any( - test, - all( - feature = "io-uring", - target_os = "linux", - any( - target_arch = "x86_64", - target_arch = "aarch64", - target_arch = "riscv64", - target_arch = "loongarch64", - target_arch = "powerpc64" - ) - ) - ))] pub fn stats_handle(&self) -> RuntimeIoStatsHandle { self.stats.clone() } - pub fn set_activity_counters_enabled(&self, enabled: bool) { - self.stats.set_activity_counters_enabled(enabled); - } - pub fn try_clone(&self) -> io::Result { Ok(Self { buffered: self.buffered.try_clone()?, @@ -340,9 +322,6 @@ pub trait IoBackend: Send + Sync { fn sync(&self, point: SyncPoint, mode: SyncMode) -> io::Result<()>; fn try_lock_exclusive(&self) -> io::Result<()>; fn unlock(&self) -> io::Result<()>; - fn runtime_io_stats(&self) -> RuntimeIoStats { - RuntimeIoStats::default() - } } /// Buffered descriptor access needed by recovery-control code. @@ -450,10 +429,6 @@ impl FileBackend { let direct = self.direct.as_ref().map(File::try_clone).transpose()?; Ok(RuntimeFileSet::with_direct(buffered, direct)) } - - pub const fn direct_active(&self) -> bool { - self.direct.is_some() - } } #[cfg(target_os = "macos")] @@ -611,13 +586,6 @@ impl IoBackend for FileBackend { Err(io::Error::last_os_error()) } } - - fn runtime_io_stats(&self) -> RuntimeIoStats { - RuntimeIoStats { - direct_active: self.direct_active(), - ..RuntimeIoStats::default() - } - } } #[cfg(unix)] @@ -705,10 +673,6 @@ impl IoBackend for RuntimeFileBackend { "runtime file backend does not own cache locking", )) } - - fn runtime_io_stats(&self) -> RuntimeIoStats { - self.files.stats.snapshot() - } } #[cfg(target_os = "linux")] @@ -1114,7 +1078,7 @@ mod tests { assert_eq!(stats.write.buffered.operations, 1); assert_eq!(stats.write.buffered.bytes, 32); - cloned.set_activity_counters_enabled(false); + cloned.stats_handle().set_activity_counters_enabled(false); files.record( RuntimeIoDirection::Read, RuntimeIoPath::Direct, @@ -1170,7 +1134,7 @@ mod tests { assert_eq!(backend.write_at(WritePoint::Record, &[], 0).unwrap(), 0); assert_eq!( - backend.runtime_io_stats(), + backend.files.stats_handle().snapshot(), RuntimeIoStats { direct_active: true, write: RuntimeIoDirectionStats { diff --git a/cache2/src/io/engine/mod.rs b/cache2/src/io/engine/mod.rs index 2c57330..f853ba0 100644 --- a/cache2/src/io/engine/mod.rs +++ b/cache2/src/io/engine/mod.rs @@ -32,6 +32,8 @@ use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU64; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; +use std::sync::mpsc; +use std::sync::mpsc::Receiver; use std::sync::mpsc::SyncSender; use std::sync::mpsc::TrySendError; use std::task::Context; @@ -46,7 +48,6 @@ use asyncband::semaphore::Semaphore; #[cfg(unix)] use crate::config::runtime::IoEngineConfig; -use crate::io::backend::IoBackend; #[cfg(unix)] use crate::io::backend::RuntimeFileSet; #[cfg(all( @@ -74,6 +75,7 @@ use crate::io::backend::RuntimeIoDirection; ))] use crate::io::backend::RuntimeIoPath; use crate::io::backend::RuntimeIoStats; +use crate::io::backend::RuntimeIoStatsHandle; use crate::io::backend::WritePoint; use crate::managed_memory::BufferLease; use crate::snapshot::CacheIoDirectionSnapshot; @@ -93,12 +95,18 @@ mod posix; ))] mod uring; -/// Reference engine: a small fixed worker pool executes exact operations -/// through the existing fault-injectable positioned-I/O backend. -#[derive(Clone)] -pub struct BackendIoEngine { - inner: Arc, - backend: Arc, +/// Bounded request admission, submission, and driver lifecycle for one I/O lane. +/// POSIX workers and io_uring drivers share this command and completion protocol. +/// Callers share the engine through `Arc`; the final owner shuts down its workers. +pub struct IoEngine { + shared: Arc, + commands: SyncSender, + submit_state: Arc>, + next_request_id: AtomicU64, + wake: Option>, + workers: Mutex>>>, + shutdown: ShutdownState, + io_stats: RuntimeIoStatsHandle, } const IO_BUFFER_ALIGNMENT: usize = 4096; @@ -756,7 +764,7 @@ impl BoundedIoRequest { self.request.id() } - pub fn wait(self, engine: &dyn IoEngine) -> Result { + pub fn wait(self, engine: &IoEngine) -> Result { let request = match self.request.wait_until(self.deadline) { Ok(completion) => return Ok(completion), Err(request) => request, @@ -786,7 +794,7 @@ impl BoundedIoRequest { pub async fn wait_async( self, - engine: Arc, + engine: Arc, tokio_handle: &tokio::runtime::Handle, ) -> Result { let mut request = AsyncRequestGuard::new(self.request, engine); @@ -822,11 +830,11 @@ impl BoundedIoRequest { struct AsyncRequestGuard { request: Option, - engine: Arc, + engine: Arc, } impl AsyncRequestGuard { - fn new(request: IoRequest, engine: Arc) -> Self { + fn new(request: IoRequest, engine: Arc) -> Self { Self { request: Some(request), engine, @@ -898,7 +906,7 @@ impl IoDeadlineExceeded { /// Submit one cache-device request with a hard end-to-end deadline. pub fn submit_cache_io( - engine: &dyn IoEngine, + engine: &IoEngine, operation: IoOperation, ) -> Result { submit_cache_io_with_timeout(engine, operation, CACHE_IO_COMPLETION_TIMEOUT) @@ -906,7 +914,7 @@ pub fn submit_cache_io( /// Submits background I/O with its configured admission and completion budget. pub fn submit_cache_io_with_timeout( - engine: &dyn IoEngine, + engine: &IoEngine, operation: IoOperation, timeout: Duration, ) -> Result { @@ -918,7 +926,7 @@ pub fn submit_cache_io_with_timeout( /// Submits a read whose engine slot was reserved before allocating its buffer. pub fn submit_cache_read( - engine: &dyn IoEngine, + engine: &IoEngine, slot: ReadSlot, operation: IoOperation, ) -> Result { @@ -935,7 +943,7 @@ pub fn submit_cache_read( } fn submit_cache_io_until( - engine: &dyn IoEngine, + engine: &IoEngine, operation: IoOperation, deadline: Instant, cancel_grace: Duration, @@ -951,122 +959,6 @@ fn submit_cache_io_until( }) } -pub trait IoEngine: Send + Sync { - /// Shared submission, completion, and bookkeeping state behind the - /// engine-specific driver. - fn inner(&self) -> &RuntimeInner; - /// Backend path and direct-I/O counters reported with the engine stats. - fn runtime_io_stats(&self) -> RuntimeIoStats; - - /// Installed once during construction, before any requests are admitted. - fn set_latency_recorder(&self, recorder: crate::stats::recording::IoTiming) { - assert!( - self.inner().shared.latency.set(recorder).is_ok(), - "I/O recorder installed twice" - ); - } - - fn try_reserve_read(&self) -> io::Result { - self.inner().try_reserve_read() - } - - fn read_slot_waiter(&self) -> ReadSlotWaiter { - self.inner().read_slot_waiter() - } - - fn submit_reserved_read( - &self, - slot: ReadSlot, - operation: IoOperation, - ) -> Result { - self.inner().submit_reserved_read(slot, operation) - } - - #[cfg(test)] - fn submit(&self, operation: IoOperation) -> Result { - self.inner().submit(operation) - } - - #[cfg(test)] - fn submit_wait(&self, operation: IoOperation) -> Result { - self.inner().submit_wait(operation) - } - - fn submit_wait_controlled( - &self, - operation: IoOperation, - cancelled: &AtomicBool, - deadline: Option, - ) -> Result { - self.inner() - .submit_wait_controlled(operation, cancelled, deadline) - } - - fn wake_slot_waiters(&self) { - self.inner().shared.wake_slot_waiters(); - } - - fn cancel(&self, request_id: RequestId, state: &CompletionState) -> io::Result { - self.inner().cancel(request_id, state) - } - - fn shutdown(&self) -> io::Result<()> { - self.inner().shutdown() - } - - fn in_flight(&self) -> usize { - self.inner().shared.total_in_flight() - } - - #[cfg(test)] - fn direct_active(&self) -> bool { - self.runtime_io_stats().direct_active - } - - /// Permanently stop accepting requests after a target operation missed both its - /// deadline and cancellation grace period. - fn stop_accepting_requests(&self) { - self.inner().stop_accepting_requests(); - } - - fn writes_in_flight(&self) -> usize { - self.inner().shared.writes_in_flight() - } - - /// True means a failed driver could not fence an issued write. - /// The cache must retain its exclusive file lock for process lifetime. - fn has_unfenced_writes(&self) -> bool { - self.inner().shared.has_unfenced_writes() - } - - #[cfg(test)] - fn mark_unfenced_writes_for_test(&self) { - self.inner().shared.mark_unfenced_writes(); - } - - fn stats(&self) -> EngineIoSnapshot { - EngineIoSnapshot { - requests: self.inner().shared.snapshot(), - runtime: self.runtime_io_stats(), - } - } - - #[cfg(test)] - fn read_exact_at(&self, buffer: IoBuffer, offset: u64) -> Result { - self.submit(IoOperation::read(buffer, offset)) - } - - #[cfg(test)] - fn write_all_at( - &self, - point: WritePoint, - buffer: IoBuffer, - offset: u64, - ) -> Result { - self.submit(IoOperation::write(point, buffer, offset)) - } -} - struct IoSlot { shared: Arc, write: bool, @@ -1652,16 +1544,6 @@ struct ShutdownState { stopped: Condvar, } -pub(crate) struct RuntimeInner { - shared: Arc, - commands: SyncSender, - submit_state: Arc>, - next_request_id: AtomicU64, - wake: Option>, - workers: Mutex>>>, - shutdown: ShutdownState, -} - #[derive(Clone, Copy)] enum SlotMode<'a> { #[cfg(test)] @@ -1674,15 +1556,98 @@ enum SlotMode<'a> { }, } -impl RuntimeInner { - fn validate_max_in_flight(max_in_flight: usize) -> io::Result<()> { +impl IoEngine { + /// Installed once during construction, before any requests are admitted. + pub fn set_latency_recorder(&self, recorder: crate::stats::recording::IoTiming) { + assert!( + self.shared.latency.set(recorder).is_ok(), + "I/O recorder installed twice" + ); + } + + pub fn wake_slot_waiters(&self) { + self.shared.wake_slot_waiters(); + } + + pub fn in_flight(&self) -> usize { + self.shared.total_in_flight() + } + + pub fn writes_in_flight(&self) -> usize { + self.shared.writes_in_flight() + } + + /// True means a failed driver could not fence an issued write. + /// The cache must retain its exclusive file lock for process lifetime. + pub fn has_unfenced_writes(&self) -> bool { + self.shared.has_unfenced_writes() + } + + #[cfg(test)] + pub fn mark_unfenced_writes_for_test(&self) { + self.shared.mark_unfenced_writes(); + } + + pub fn stats(&self) -> EngineIoSnapshot { + EngineIoSnapshot { + requests: self.shared.snapshot(), + runtime: self.io_stats.snapshot(), + } + } + + #[cfg(test)] + pub fn read_exact_at(&self, buffer: IoBuffer, offset: u64) -> Result { + self.submit(IoOperation::read(buffer, offset)) + } + + #[cfg(test)] + pub fn write_all_at( + &self, + point: WritePoint, + buffer: IoBuffer, + offset: u64, + ) -> Result { + self.submit(IoOperation::write(point, buffer, offset)) + } + + fn with_command_channel( + max_in_flight: usize, + activity_counters_enabled: bool, + read_wait_enabled: bool, + io_stats: RuntimeIoStatsHandle, + ) -> io::Result<(Self, Receiver)> { if !(1..=MAX_IO_REQUESTS_PER_ENGINE).contains(&max_in_flight) { return Err(io::Error::new( io::ErrorKind::InvalidInput, format!("I/O requests per engine must be in 1..={MAX_IO_REQUESTS_PER_ENGINE}"), )); } - Ok(()) + let command_capacity = max_in_flight + .checked_mul(2) + .and_then(|depth| depth.checked_add(1)) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "queue size overflow"))?; + let (commands, receiver) = mpsc::sync_channel(command_capacity); + io_stats.set_activity_counters_enabled(activity_counters_enabled); + Ok(( + Self { + shared: Arc::new(RuntimeShared::new( + max_in_flight, + activity_counters_enabled, + read_wait_enabled, + )), + commands, + submit_state: Arc::new(RwLock::new(SubmitState { accepting: true })), + next_request_id: AtomicU64::new(1), + wake: None, + workers: Mutex::new(Vec::new()), + shutdown: ShutdownState { + phase: Mutex::new(ShutdownPhase::Running), + stopped: Condvar::new(), + }, + io_stats, + }, + receiver, + )) } fn next_request_id(&self) -> RequestId { @@ -1701,11 +1666,11 @@ impl RuntimeInner { } #[cfg(test)] - fn submit(&self, operation: IoOperation) -> Result { + pub fn submit(&self, operation: IoOperation) -> Result { self.submit_inner(operation, SlotMode::Try) } - fn try_reserve_read(&self) -> io::Result { + pub fn try_reserve_read(&self) -> io::Result { let permit = self .shared .read_slot_admission @@ -1715,13 +1680,13 @@ impl RuntimeInner { self.shared.try_reserve_read_slot(permit) } - fn read_slot_waiter(&self) -> ReadSlotWaiter { + pub fn read_slot_waiter(&self) -> ReadSlotWaiter { ReadSlotWaiter { shared: Arc::clone(&self.shared), } } - fn submit_reserved_read( + pub fn submit_reserved_read( &self, slot: ReadSlot, operation: IoOperation, @@ -1745,11 +1710,11 @@ impl RuntimeInner { } #[cfg(test)] - fn submit_wait(&self, operation: IoOperation) -> Result { + pub fn submit_wait(&self, operation: IoOperation) -> Result { self.submit_inner(operation, SlotMode::Wait) } - fn submit_wait_controlled( + pub fn submit_wait_controlled( &self, operation: IoOperation, cancelled: &AtomicBool, @@ -1949,7 +1914,8 @@ impl RuntimeInner { Ok(true) } - fn stop_accepting_requests(&self) { + /// Permanently stop accepting requests before shutdown or after an unfenced timeout. + pub fn stop_accepting_requests(&self) { let mut submit_state = self .submit_state .write() @@ -1962,7 +1928,7 @@ impl RuntimeInner { } } - fn shutdown(&self) -> io::Result<()> { + pub fn shutdown(&self) -> io::Result<()> { let leader = { let mut phase = lock_unpoisoned(&self.shutdown.phase); loop { @@ -2027,7 +1993,7 @@ impl RuntimeInner { } } -impl Drop for RuntimeInner { +impl Drop for IoEngine { fn drop(&mut self) { let _ = self.shutdown(); } @@ -2039,16 +2005,16 @@ pub fn build_file_engine( config: IoEngineConfig, activity_counters_enabled: bool, read_wait_enabled: bool, -) -> io::Result> { +) -> io::Result> { match config { - IoEngineConfig::Posix { workers } => BackendIoEngine::new_with_files_and_workers( + IoEngineConfig::Posix { workers } => posix::start( files, workers, workers, activity_counters_enabled, read_wait_enabled, ) - .map(|engine| Arc::new(engine) as Arc), + .map(Arc::new), IoEngineConfig::IoUring(config) => { #[cfg(all( feature = "io-uring", @@ -2062,13 +2028,8 @@ pub fn build_file_engine( ) ))] { - uring::UringIoEngine::new_with_files( - files, - config, - activity_counters_enabled, - read_wait_enabled, - ) - .map(|engine| Arc::new(engine) as Arc) + uring::start(files, config, activity_counters_enabled, read_wait_enabled) + .map(Arc::new) } #[cfg(not(all( feature = "io-uring", diff --git a/cache2/src/io/engine/posix.rs b/cache2/src/io/engine/posix.rs index 189c040..fe2b19a 100644 --- a/cache2/src/io/engine/posix.rs +++ b/cache2/src/io/engine/posix.rs @@ -16,12 +16,8 @@ use std::io; use std::panic; use std::panic::AssertUnwindSafe; use std::sync::Arc; -use std::sync::Condvar; use std::sync::Mutex; -use std::sync::RwLock; -use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; -use std::sync::mpsc; use std::sync::mpsc::Receiver; use crate::io::backend::IoBackend; @@ -29,147 +25,106 @@ use crate::io::backend::IoBackend; use crate::io::backend::RuntimeFileBackend; #[cfg(unix)] use crate::io::backend::RuntimeFileSet; -use crate::io::backend::RuntimeIoStats; +use crate::io::backend::RuntimeIoStatsHandle; use crate::io::backend::read_exact_at_uninit_with_progress; use crate::io::backend::write_all_at_with_progress; -use crate::io::engine::BackendIoEngine; use crate::io::engine::CompletionStatus; use crate::io::engine::DriverCommand; use crate::io::engine::IoEngine; use crate::io::engine::IoOperation; -use crate::io::engine::RuntimeInner; use crate::io::engine::RuntimeShared; -use crate::io::engine::ShutdownPhase; -use crate::io::engine::ShutdownState; -use crate::io::engine::SubmitState; use crate::io::engine::lock_unpoisoned; use crate::managed_memory::CACHE_THREAD_STACK_BYTES; -impl BackendIoEngine { - #[cfg(unix)] - #[cfg(test)] - pub fn new_with_files(files: RuntimeFileSet, max_in_flight: usize) -> io::Result { - let backend: Arc = Arc::new(RuntimeFileBackend::new(files)); - Self::new(backend, max_in_flight) - } - - #[cfg(unix)] - pub fn new_with_files_and_workers( - files: RuntimeFileSet, - max_in_flight: usize, - worker_count: usize, - activity_counters_enabled: bool, - read_wait_enabled: bool, - ) -> io::Result { - files.set_activity_counters_enabled(activity_counters_enabled); - let backend: Arc = Arc::new(RuntimeFileBackend::new(files)); - Self::new_with_workers_and_activity_counters( - backend, - max_in_flight, - worker_count, - activity_counters_enabled, - read_wait_enabled, - ) - } +#[cfg(unix)] +pub fn start( + files: RuntimeFileSet, + max_in_flight: usize, + worker_count: usize, + activity_counters_enabled: bool, + read_wait_enabled: bool, +) -> io::Result { + let io_stats = files.stats_handle(); + let backend = Arc::new(RuntimeFileBackend::new(files)); + start_backend( + backend, + io_stats, + max_in_flight, + worker_count, + activity_counters_enabled, + read_wait_enabled, + ) +} - #[cfg(test)] - pub fn new(backend: Arc, max_in_flight: usize) -> io::Result { - Self::new_with_workers_and_activity_counters( - backend, - max_in_flight, - max_in_flight.min(4), - true, - false, - ) +#[cfg(test)] +impl IoEngine { + pub fn for_test(backend: Arc, max_in_flight: usize) -> io::Result { + Self::for_test_with_options(backend, max_in_flight, max_in_flight.min(4), true, false) } - #[cfg(test)] - pub fn new_with_read_wait( + pub fn for_test_with_read_wait( backend: Arc, max_in_flight: usize, ) -> io::Result { - Self::new_with_workers_and_activity_counters( - backend, - max_in_flight, - max_in_flight.min(4), - true, - true, - ) + Self::for_test_with_options(backend, max_in_flight, max_in_flight.min(4), true, true) } - pub fn new_with_workers_and_activity_counters( + pub fn for_test_with_options( backend: Arc, max_in_flight: usize, worker_count: usize, activity_counters_enabled: bool, read_wait_enabled: bool, ) -> io::Result { - RuntimeInner::validate_max_in_flight(max_in_flight)?; - if worker_count == 0 || worker_count > max_in_flight { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "POSIX I/O worker count must not exceed the request limit", - )); - } - let shared = Arc::new(RuntimeShared::new( + start_backend( + backend, + RuntimeIoStatsHandle::new(false), max_in_flight, + worker_count, activity_counters_enabled, read_wait_enabled, - )); - let command_capacity = max_in_flight - .checked_mul(2) - .and_then(|depth| depth.checked_add(1)) - .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "queue size overflow"))?; - let (commands, receiver) = mpsc::sync_channel(command_capacity); - let receiver = Arc::new(Mutex::new(receiver)); - let mut workers = Vec::with_capacity(worker_count); - for worker_index in 0..worker_count { - let worker_backend = Arc::clone(&backend); - let worker_shared = Arc::clone(&shared); - let worker_receiver = Arc::clone(&receiver); - let spawn_result = std::thread::Builder::new() - .name(format!("cache2-sync-io-{worker_index}")) - .stack_size(CACHE_THREAD_STACK_BYTES) - .spawn(move || backend_driver(worker_backend, worker_shared, worker_receiver)); - match spawn_result { - Ok(worker) => workers.push(worker), - Err(error) => { - for _ in 0..workers.len() { - let _ = commands.send(DriverCommand::Shutdown); - } - for worker in workers { - let _ = worker.join(); - } - return Err(error); - } - } - } - Ok(Self { - inner: Arc::new(RuntimeInner { - shared, - commands, - submit_state: Arc::new(RwLock::new(SubmitState { accepting: true })), - next_request_id: AtomicU64::new(1), - wake: None, - workers: Mutex::new(workers), - shutdown: ShutdownState { - phase: Mutex::new(ShutdownPhase::Running), - stopped: Condvar::new(), - }, - }), - backend, - }) + ) } } -impl IoEngine for BackendIoEngine { - fn inner(&self) -> &RuntimeInner { - &self.inner +fn start_backend( + backend: Arc, + io_stats: RuntimeIoStatsHandle, + max_in_flight: usize, + worker_count: usize, + activity_counters_enabled: bool, + read_wait_enabled: bool, +) -> io::Result { + let (mut engine, receiver) = IoEngine::with_command_channel( + max_in_flight, + activity_counters_enabled, + read_wait_enabled, + io_stats, + )?; + if worker_count == 0 || worker_count > max_in_flight { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "POSIX I/O worker count must not exceed the request limit", + )); } - - fn runtime_io_stats(&self) -> RuntimeIoStats { - self.backend.runtime_io_stats() + let receiver = Arc::new(Mutex::new(receiver)); + engine + .workers + .get_mut() + .unwrap() + .reserve_exact(worker_count); + for worker_index in 0..worker_count { + let worker_backend = Arc::clone(&backend); + let worker_shared = Arc::clone(&engine.shared); + let worker_receiver = Arc::clone(&receiver); + let worker = std::thread::Builder::new() + .name(format!("cache2-sync-io-{worker_index}")) + .stack_size(CACHE_THREAD_STACK_BYTES) + .spawn(move || backend_driver(worker_backend, worker_shared, worker_receiver))?; + // Retain each worker immediately so engine Drop joins it if a later spawn fails. + engine.workers.get_mut().unwrap().push(worker); } + Ok(engine) } fn backend_driver( diff --git a/cache2/src/io/engine/tests.rs b/cache2/src/io/engine/tests.rs index 5f48788..8d98f82 100644 --- a/cache2/src/io/engine/tests.rs +++ b/cache2/src/io/engine/tests.rs @@ -20,6 +20,7 @@ use crate::IoOutcome; use crate::IoRole; use crate::StatsOptions; use crate::fixtures::TestFile; +use crate::io::backend::IoBackend; use crate::io::backend::SyncMode; use crate::io::backend::SyncPoint; use crate::managed_memory::ManagedMemory; @@ -27,10 +28,9 @@ use crate::managed_memory::ManagedMemoryLimits; use crate::managed_memory::aligned_buffer_capacity; use crate::stats::recording::Recorder; -async fn wait_for_registered_read_waiters(engine: &BackendIoEngine, expected: usize) { +async fn wait_for_registered_read_waiters(engine: &IoEngine, expected: usize) { for _ in 0..100 { let actual = engine - .inner .shared .read_slot_admission .as_ref() @@ -44,7 +44,7 @@ async fn wait_for_registered_read_waiters(engine: &BackendIoEngine, expected: us } async fn spawn_registered_read_slot_waiter( - engine: &BackendIoEngine, + engine: &IoEngine, timeout: Duration, expected_waiters: usize, ) -> tokio::task::JoinHandle> { @@ -277,7 +277,7 @@ fn aligned_buffer_has_stable_alignment() { #[test] fn posix_engine_round_trips_owned_buffers_and_drains() { let file = TestFile::new("io-engine"); - let engine = BackendIoEngine::new(file.backend(), 4).unwrap(); + let engine = IoEngine::for_test(file.backend(), 4).unwrap(); let managed_memory = managed_memory(); let input = b"owned async positioned I/O"; let write = engine @@ -316,7 +316,7 @@ fn posix_engine_round_trips_owned_buffers_and_drains() { #[test] fn posix_engine_reports_progress_before_a_terminal_short_io_error() { - let engine = BackendIoEngine::new(Arc::new(ShortThenErrorBackend::default()), 2).unwrap(); + let engine = IoEngine::for_test(Arc::new(ShortThenErrorBackend::default()), 2).unwrap(); let managed_memory = managed_memory(); let read = engine @@ -343,7 +343,7 @@ fn posix_engine_reports_progress_before_a_terminal_short_io_error() { async fn async_request_is_woken_by_driver_completion() { let file = TestFile::new("io-engine"); file.open().set_len(4096).unwrap(); - let engine: Arc = Arc::new(BackendIoEngine::new(file.backend(), 2).unwrap()); + let engine = Arc::new(IoEngine::for_test(file.backend(), 2).unwrap()); let managed_memory = managed_memory(); let request = submit_cache_io( engine.as_ref(), @@ -364,7 +364,7 @@ async fn async_request_is_woken_by_driver_completion() { #[tokio::test] async fn dropping_async_wait_requests_bounded_cancellation() { let backend = Arc::new(BlockingBackend::default()); - let engine: Arc = Arc::new(BackendIoEngine::new(backend.clone(), 1).unwrap()); + let engine = Arc::new(IoEngine::for_test(backend.clone(), 1).unwrap()); let recorder = Arc::new( Recorder::new(StatsOptions { io_latency: true, @@ -414,14 +414,9 @@ async fn reserved_read_latency_includes_time_before_submission() { for activity_counters_enabled in [false, true] { let file = TestFile::new("io-engine"); file.open().set_len(4096).unwrap(); - let engine = BackendIoEngine::new_with_workers_and_activity_counters( - file.backend(), - 1, - 1, - activity_counters_enabled, - true, - ) - .unwrap(); + let engine = + IoEngine::for_test_with_options(file.backend(), 1, 1, activity_counters_enabled, true) + .unwrap(); let recorder = Arc::new( Recorder::new(StatsOptions { io_latency: true, @@ -469,8 +464,7 @@ async fn reserved_read_latency_includes_time_before_submission() { #[tokio::test] async fn read_slot_waits_for_cancelled_request_to_release_physical_capacity() { let backend = Arc::new(BlockingBackend::default()); - let engine: Arc = - Arc::new(BackendIoEngine::new_with_read_wait(backend.clone(), 1).unwrap()); + let engine = Arc::new(IoEngine::for_test_with_read_wait(backend.clone(), 1).unwrap()); let managed_memory = managed_memory(); let slot = engine.try_reserve_read().unwrap(); let request = submit_cache_read( @@ -513,7 +507,7 @@ async fn read_slot_waits_for_cancelled_request_to_release_physical_capacity() { #[tokio::test] async fn read_slot_wait_is_woken_by_engine_shutdown() { let file = TestFile::new("io-engine"); - let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); + let engine = Arc::new(IoEngine::for_test_with_read_wait(file.backend(), 1).unwrap()); let slot = engine.try_reserve_read().unwrap(); let mut waiters = Vec::new(); for expected in 1..=3 { @@ -536,7 +530,7 @@ async fn read_slot_wait_is_woken_by_engine_shutdown() { #[tokio::test(flavor = "current_thread")] async fn queued_read_reservation_precedes_new_immediate_read() { let file = TestFile::new("io-engine"); - let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); + let engine = Arc::new(IoEngine::for_test_with_read_wait(file.backend(), 1).unwrap()); let held = engine.try_reserve_read().unwrap(); let queued = spawn_registered_read_slot_waiter(&engine, Duration::from_secs(1), 1).await; @@ -555,7 +549,7 @@ async fn queued_read_reservation_precedes_new_immediate_read() { #[tokio::test(flavor = "current_thread")] async fn queued_read_reservations_are_fifo() { let file = TestFile::new("io-engine"); - let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); + let engine = Arc::new(IoEngine::for_test_with_read_wait(file.backend(), 1).unwrap()); let held = engine.try_reserve_read().unwrap(); let first = spawn_registered_read_slot_waiter(&engine, Duration::from_secs(1), 1).await; @@ -578,7 +572,7 @@ async fn queued_read_reservations_are_fifo() { #[tokio::test(flavor = "current_thread")] async fn queued_reads_use_every_released_engine_slot() { let file = TestFile::new("io-engine"); - let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 2).unwrap()); + let engine = Arc::new(IoEngine::for_test_with_read_wait(file.backend(), 2).unwrap()); let held: Vec<_> = (0..2).map(|_| engine.try_reserve_read().unwrap()).collect(); let first = spawn_registered_read_slot_waiter(&engine, Duration::from_secs(1), 1).await; let second = spawn_registered_read_slot_waiter(&engine, Duration::from_secs(1), 2).await; @@ -597,7 +591,7 @@ async fn queued_reads_use_every_released_engine_slot() { #[tokio::test(flavor = "current_thread")] async fn timed_out_queue_head_passes_priority_to_next_read() { let file = TestFile::new("io-engine"); - let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); + let engine = Arc::new(IoEngine::for_test_with_read_wait(file.backend(), 1).unwrap()); let held = engine.try_reserve_read().unwrap(); let first = spawn_registered_read_slot_waiter(&engine, Duration::from_millis(20), 1).await; @@ -615,7 +609,7 @@ async fn timed_out_queue_head_passes_priority_to_next_read() { #[tokio::test(flavor = "current_thread")] async fn cancelled_queue_head_passes_priority_to_next_read() { let file = TestFile::new("io-engine"); - let engine = Arc::new(BackendIoEngine::new_with_read_wait(file.backend(), 1).unwrap()); + let engine = Arc::new(IoEngine::for_test_with_read_wait(file.backend(), 1).unwrap()); let held = engine.try_reserve_read().unwrap(); let first = spawn_registered_read_slot_waiter(&engine, Duration::from_secs(1), 1).await; @@ -637,7 +631,7 @@ async fn cancelled_queue_head_passes_priority_to_next_read() { #[tokio::test] async fn async_read_deadline_keeps_other_slots_available() { let backend = Arc::new(BlockingBackend::default()); - let engine: Arc = Arc::new(BackendIoEngine::new(backend.clone(), 2).unwrap()); + let engine = Arc::new(IoEngine::for_test(backend.clone(), 2).unwrap()); let managed_memory = managed_memory(); let request = submit_cache_io_until( engine.as_ref(), @@ -671,9 +665,14 @@ fn posix_engine_routes_only_aligned_record_io_to_direct() { let direct_file = direct.open(); buffered_file.set_len(8192).unwrap(); direct_file.set_len(8192).unwrap(); - let engine = - BackendIoEngine::new_with_files(RuntimeFileSet::new(buffered_file, Some(direct_file)), 2) - .unwrap(); + let engine = posix::start( + RuntimeFileSet::new(buffered_file, Some(direct_file)), + 2, + 2, + true, + false, + ) + .unwrap(); let managed_memory = managed_memory(); let aligned = vec![0x5a; 4096]; @@ -702,7 +701,7 @@ fn posix_engine_routes_only_aligned_record_io_to_direct() { CompletionStatus::Completed )); - assert!(engine.direct_active()); + assert!(engine.stats().runtime.direct_active); let stats = engine.stats(); assert_eq!(stats.runtime.write.direct.operations, 1); assert_eq!(stats.runtime.write.direct.bytes, 4096); @@ -714,7 +713,7 @@ fn posix_engine_routes_only_aligned_record_io_to_direct() { #[test] fn unfenced_write_state_remains_unsafe_after_shutdown() { let file = TestFile::new("io-engine"); - let engine = BackendIoEngine::new(file.backend(), 1).unwrap(); + let engine = IoEngine::for_test(file.backend(), 1).unwrap(); assert!(!engine.has_unfenced_writes()); engine.mark_unfenced_writes_for_test(); @@ -727,7 +726,7 @@ fn unfenced_write_state_remains_unsafe_after_shutdown() { #[test] fn read_completion_deadline_retains_only_its_bounded_slot() { let backend = Arc::new(BlockingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 1).unwrap(); let managed_memory = managed_memory(); let deadline = Instant::now() + Duration::from_millis(20); let request = submit_cache_io_until( @@ -761,7 +760,7 @@ fn read_completion_deadline_retains_only_its_bounded_slot() { #[test] fn completion_deadline_keeps_an_issued_write_counted_until_target_completion() { let backend = Arc::new(BlockingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 1).unwrap(); let managed_memory = managed_memory(); let request = submit_cache_io_until( &engine, @@ -803,7 +802,7 @@ fn completion_deadline_keeps_an_issued_write_counted_until_target_completion() { fn engine_request_capacity_is_hard_bounded() { let file = TestFile::new("io-engine"); assert!(matches!( - BackendIoEngine::new(file.backend(), MAX_IO_REQUESTS_PER_ENGINE + 1), + IoEngine::for_test(file.backend(), MAX_IO_REQUESTS_PER_ENGINE + 1), Err(error) if error.kind() == io::ErrorKind::InvalidInput )); } @@ -829,9 +828,7 @@ fn configured_posix_engine_shares_its_worker_capacity() { #[test] fn disabled_io_statistics_skip_cumulative_engine_counters() { let file = TestFile::new("io-engine"); - let engine = - BackendIoEngine::new_with_workers_and_activity_counters(file.backend(), 1, 1, false, false) - .unwrap(); + let engine = IoEngine::for_test_with_options(file.backend(), 1, 1, false, false).unwrap(); let managed_memory = managed_memory(); let completion = engine .write_all_at( @@ -862,7 +859,7 @@ fn slot_state_tracks_full_write_capacity() { #[test] fn unused_read_reservation_releases_its_engine_slot() { let file = TestFile::new("io-engine"); - let engine = BackendIoEngine::new(file.backend(), 1).unwrap(); + let engine = IoEngine::for_test(file.backend(), 1).unwrap(); let slot = engine.try_reserve_read().unwrap(); assert_eq!(engine.in_flight(), 1); assert_eq!( @@ -878,10 +875,9 @@ fn unused_read_reservation_releases_its_engine_slot() { #[test] fn nowait_submission_does_not_wait_for_the_shutdown_fence() { let file = TestFile::new("io-engine"); - let engine = BackendIoEngine::new(file.backend(), 1).unwrap(); + let engine = IoEngine::for_test(file.backend(), 1).unwrap(); let managed_memory = managed_memory(); let fence = engine - .inner .submit_state .write() .unwrap_or_else(|poisoned| poisoned.into_inner()); @@ -899,7 +895,7 @@ fn nowait_submission_does_not_wait_for_the_shutdown_fence() { #[test] fn backend_workers_execute_independent_reads_concurrently() { let backend = Arc::new(BlockingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 2).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 2).unwrap(); let managed_memory = managed_memory(); let first = engine .read_exact_at(read_buffer(&managed_memory, 1), 0) @@ -923,7 +919,7 @@ fn backend_workers_execute_independent_reads_concurrently() { #[test] fn submit_wait_blocks_at_engine_capacity_and_resumes() { let backend = Arc::new(BlockingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = Arc::new(IoEngine::for_test(backend.clone(), 1).unwrap()); let managed_memory = managed_memory(); let first = engine .read_exact_at(read_buffer(&managed_memory, 1), 0) @@ -972,7 +968,7 @@ fn submit_wait_blocks_at_engine_capacity_and_resumes() { #[test] fn controlled_slot_wait_observes_cancel_wake_and_absolute_deadline() { let backend = Arc::new(BlockingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = Arc::new(IoEngine::for_test(backend.clone(), 1).unwrap()); let managed_memory = managed_memory(); let first = engine .read_exact_at(read_buffer(&managed_memory, 1), 0) @@ -1028,7 +1024,7 @@ fn controlled_slot_wait_observes_cancel_wake_and_absolute_deadline() { #[test] fn backend_panic_completes_the_request_and_worker_survives() { - let engine = BackendIoEngine::new(Arc::new(PanicOnceBackend::new()), 1).unwrap(); + let engine = IoEngine::for_test(Arc::new(PanicOnceBackend::new()), 1).unwrap(); let managed_memory = managed_memory(); let failed = engine .read_exact_at(read_buffer(&managed_memory, 1), 0) @@ -1102,9 +1098,7 @@ fn quarantined_completion_does_not_return_a_potentially_live_buffer() { #[test] fn io_histograms_include_failures_when_activity_counters_are_disabled() { let file = TestFile::new("io-engine"); - let engine = - BackendIoEngine::new_with_workers_and_activity_counters(file.backend(), 1, 1, false, false) - .unwrap(); + let engine = IoEngine::for_test_with_options(file.backend(), 1, 1, false, false).unwrap(); let recorder = Arc::new( Recorder::new(StatsOptions { io_latency: true, @@ -1133,7 +1127,7 @@ fn io_histograms_include_failures_when_activity_counters_are_disabled() { #[test] fn configured_background_read_deadline_expires_and_retains_owned_buffer() { let backend = Arc::new(BlockingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 2).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 2).unwrap(); let managed_memory = managed_memory(); let request = submit_cache_io_with_timeout( &engine, @@ -1152,7 +1146,7 @@ fn configured_background_read_deadline_expires_and_retains_owned_buffer() { #[test] fn background_read_can_complete_after_default_deadline() { let backend = Arc::new(BlockingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 1).unwrap(); let managed_memory = managed_memory(); let request = submit_cache_io_with_timeout( &engine, diff --git a/cache2/src/io/engine/uring.rs b/cache2/src/io/engine/uring.rs index f5a0edb..eaebcc0 100644 --- a/cache2/src/io/engine/uring.rs +++ b/cache2/src/io/engine/uring.rs @@ -23,11 +23,8 @@ use std::os::unix::net::UnixStream; use std::panic; use std::panic::AssertUnwindSafe; use std::sync::Arc; -use std::sync::Condvar; -use std::sync::Mutex; use std::sync::RwLock; use std::sync::atomic::AtomicBool; -use std::sync::atomic::AtomicU64; #[cfg(test)] use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; @@ -44,8 +41,6 @@ use io_uring::types; use crate::config::runtime::IoUringEngineConfig; use crate::io::backend::RuntimeFileSet; use crate::io::backend::RuntimeIoPath; -use crate::io::backend::RuntimeIoStats; -use crate::io::backend::RuntimeIoStatsHandle; use crate::io::backend::retry_interrupted; #[cfg(test)] use crate::io::engine::CompletionState; @@ -62,10 +57,7 @@ use crate::io::engine::IoOperation; use crate::io::engine::MAX_IO_REQUESTS_PER_ENGINE; use crate::io::engine::OperationKind; use crate::io::engine::RequestId; -use crate::io::engine::RuntimeInner; use crate::io::engine::RuntimeShared; -use crate::io::engine::ShutdownPhase; -use crate::io::engine::ShutdownState; use crate::io::engine::SubmitState; use crate::io::engine::Task; #[cfg(test)] @@ -114,140 +106,98 @@ impl DriverWake for SocketWake { } } -/// Linux `io_uring` engine. The ring and every raw buffer pointer are owned -/// by one driver thread; callers communicate only through bounded commands. -#[derive(Clone)] -pub struct UringIoEngine { - inner: Arc, - io_stats: RuntimeIoStatsHandle, -} - -impl UringIoEngine { - pub fn new_with_files( - files: RuntimeFileSet, - config: IoUringEngineConfig, - activity_counters_enabled: bool, - read_wait_enabled: bool, - ) -> io::Result { - let max_in_flight = config.max_in_flight; - RuntimeInner::validate_max_in_flight(max_in_flight)?; - files.set_activity_counters_enabled(activity_counters_enabled); - let io_stats = files.stats_handle(); - let ring_entries = max_in_flight - .checked_add(2) - .and_then(usize::checked_next_power_of_two) - .ok_or_else(|| { - io::Error::new(io::ErrorKind::InvalidInput, "ring queue size overflow") - })?; - let ring_entries = u32::try_from(ring_entries).map_err(|_| { - io::Error::new(io::ErrorKind::InvalidInput, "ring queue size exceeds u32") - })?; - let completion_entries = ring_entries.checked_mul(2).ok_or_else(|| { - io::Error::new( - io::ErrorKind::InvalidInput, - "completion queue size overflow", - ) - })?; - let mut builder = IoUring::builder(); - builder.setup_cqsize(completion_entries).dontfork(); - if config.io_poll { - builder.setup_iopoll(); - } - if let Some(sq_poll) = config.sq_poll { - builder.setup_sqpoll(sq_poll.idle_millis); - if let Some(cpu) = sq_poll.cpu { - builder.setup_sqpoll_cpu(cpu); - } - } - let ring = builder.build(ring_entries)?; - if !ring.params().is_feature_nodrop() { - return Err(io::Error::new( - io::ErrorKind::Unsupported, - "kernel io_uring can drop completion entries", - )); - } - if config.sq_poll.is_some() && !ring.params().is_feature_sqpoll_nonfixed() { - return Err(io::Error::new( - io::ErrorKind::Unsupported, - "kernel io_uring SQPOLL requires registered files", - )); - } - let mut probe = Probe::new(); - ring.submitter().register_probe(&mut probe)?; - let required = [ - opcode::Read::CODE, - opcode::Write::CODE, - opcode::AsyncCancel::CODE, - opcode::PollAdd::CODE, - ]; - if required.iter().any(|opcode| !probe.is_supported(*opcode)) { - return Err(io::Error::new( - io::ErrorKind::Unsupported, - "kernel io_uring lacks a required file opcode", - )); +/// Starts a driver that owns the ring and all submitted buffer pointers. +pub fn start( + files: RuntimeFileSet, + config: IoUringEngineConfig, + activity_counters_enabled: bool, + read_wait_enabled: bool, +) -> io::Result { + let max_in_flight = config.max_in_flight; + let (mut engine, receiver) = IoEngine::with_command_channel( + max_in_flight, + activity_counters_enabled, + read_wait_enabled, + files.stats_handle(), + )?; + let ring_entries = max_in_flight + .checked_add(2) + .and_then(usize::checked_next_power_of_two) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "ring queue size overflow"))?; + let ring_entries = u32::try_from(ring_entries) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "ring queue size exceeds u32"))?; + let completion_entries = ring_entries.checked_mul(2).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "completion queue size overflow", + ) + })?; + let mut builder = IoUring::builder(); + builder.setup_cqsize(completion_entries).dontfork(); + if config.io_poll { + builder.setup_iopoll(); + } + if let Some(sq_poll) = config.sq_poll { + builder.setup_sqpoll(sq_poll.idle_millis); + if let Some(cpu) = sq_poll.cpu { + builder.setup_sqpoll_cpu(cpu); } - - let (wake_sender, wake_receiver) = UnixStream::pair()?; - wake_sender.set_nonblocking(true)?; - wake_receiver.set_nonblocking(true)?; - let wake_pending = Arc::new(AtomicBool::new(false)); - let wake: Arc = Arc::new(SocketWake { - sender: wake_sender, - pending: Arc::clone(&wake_pending), - }); - let shared = Arc::new(RuntimeShared::new( - max_in_flight, - activity_counters_enabled, - read_wait_enabled, + } + let ring = builder.build(ring_entries)?; + if !ring.params().is_feature_nodrop() { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "kernel io_uring can drop completion entries", )); - let command_capacity = max_in_flight - .checked_mul(2) - .and_then(|depth| depth.checked_add(1)) - .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "queue size overflow"))?; - let (commands, receiver) = mpsc::sync_channel(command_capacity); - let submit_state = Arc::new(RwLock::new(SubmitState { accepting: true })); - let worker_shared = Arc::clone(&shared); - let worker_submit_state = Arc::clone(&submit_state); - let worker = std::thread::Builder::new() - .name("cache2-uring-io".into()) - .stack_size(CACHE_THREAD_STACK_BYTES) - .spawn(move || { - uring_driver( - files, - ring, - wake_receiver, - wake_pending, - worker_shared, - worker_submit_state, - receiver, - ) - })?; - Ok(Self { - inner: Arc::new(RuntimeInner { - shared, - commands, - submit_state, - next_request_id: AtomicU64::new(1), - wake: Some(wake), - workers: Mutex::new(vec![worker]), - shutdown: ShutdownState { - phase: Mutex::new(ShutdownPhase::Running), - stopped: Condvar::new(), - }, - }), - io_stats, - }) } -} - -impl IoEngine for UringIoEngine { - fn inner(&self) -> &RuntimeInner { - &self.inner + if config.sq_poll.is_some() && !ring.params().is_feature_sqpoll_nonfixed() { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "kernel io_uring SQPOLL requires registered files", + )); } - - fn runtime_io_stats(&self) -> RuntimeIoStats { - self.io_stats.snapshot() + let mut probe = Probe::new(); + ring.submitter().register_probe(&mut probe)?; + let required = [ + opcode::Read::CODE, + opcode::Write::CODE, + opcode::AsyncCancel::CODE, + opcode::PollAdd::CODE, + ]; + if required.iter().any(|opcode| !probe.is_supported(*opcode)) { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "kernel io_uring lacks a required file opcode", + )); } + + let (wake_sender, wake_receiver) = UnixStream::pair()?; + wake_sender.set_nonblocking(true)?; + wake_receiver.set_nonblocking(true)?; + let wake_pending = Arc::new(AtomicBool::new(false)); + let wake: Arc = Arc::new(SocketWake { + sender: wake_sender, + pending: Arc::clone(&wake_pending), + }); + engine.wake = Some(wake); + let worker_shared = Arc::clone(&engine.shared); + let worker_submit_state = Arc::clone(&engine.submit_state); + let worker = std::thread::Builder::new() + .name("cache2-uring-io".into()) + .stack_size(CACHE_THREAD_STACK_BYTES) + .spawn(move || { + uring_driver( + files, + ring, + wake_receiver, + wake_pending, + worker_shared, + worker_submit_state, + receiver, + ) + })?; + engine.workers.get_mut().unwrap().push(worker); + Ok(engine) } struct Flight { @@ -1019,6 +969,7 @@ fn build_target_entry( #[cfg(test)] mod tests { + use std::sync::atomic::AtomicU64; use std::task::Wake; use std::task::Waker; diff --git a/cache2/src/region/appender.rs b/cache2/src/region/appender.rs index 2b5af5e..19d49cb 100644 --- a/cache2/src/region/appender.rs +++ b/cache2/src/region/appender.rs @@ -77,7 +77,7 @@ pub struct RegionSpanCompletion { } impl RegionSpanFlight { - pub fn wait(self, engine: &dyn IoEngine) -> RegionSpanCompletion { + pub fn wait(self, engine: &IoEngine) -> RegionSpanCompletion { let completion = match self.request.wait(engine) { Ok(completion) => completion, Err(timeout) => { @@ -132,7 +132,7 @@ impl RegionSpanFlight { // allocation; boxing it would violate that overload-path property. #[allow(clippy::result_large_err)] pub fn submit_span( - engine: &dyn IoEngine, + engine: &IoEngine, geometry: DataGeometry, span: RegionWriteSpan, buffer: IoBuffer, @@ -245,7 +245,7 @@ mod tests { use crate::io::backend::IoBackend; use crate::io::backend::SyncMode; use crate::io::backend::SyncPoint; - use crate::io::engine::BackendIoEngine; + use crate::io::engine::IoEngine; use crate::managed_memory::BufferLease; #[derive(Default)] @@ -311,7 +311,7 @@ mod tests { #[test] fn span_write_preserves_owned_buffer_and_maps_region_offset_exactly() { let backend = Arc::new(RecordingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 1).unwrap(); let mut lease = BufferLease::try_fixed(4096).unwrap(); lease.prepare(4096).unwrap().fill(0x5a); @@ -343,7 +343,7 @@ mod tests { #[test] fn invalid_span_returns_the_only_buffer_without_submitting_io() { let backend = Arc::new(RecordingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 1).unwrap(); let mut invalid = span(); invalid.end_offset += 1; let buffer = IoBuffer::for_write(BufferLease::try_fixed(4096).unwrap(), 4096).unwrap(); diff --git a/cache2/src/region/file_backend/tests.rs b/cache2/src/region/file_backend/tests.rs index 6ee5af8..45a473e 100644 --- a/cache2/src/region/file_backend/tests.rs +++ b/cache2/src/region/file_backend/tests.rs @@ -44,7 +44,6 @@ use crate::io::backend::testing::FaultBackend; use crate::io::backend::testing::FaultEvent; use crate::io::backend::testing::FaultHandle; use crate::io::backend::testing::kill_process; -use crate::io::engine::BackendIoEngine; use crate::io::engine::IoEngine; use crate::managed_memory::ManagedMemory; use crate::managed_memory::ManagedMemoryLimits; @@ -902,7 +901,7 @@ fn completed_owned_span_publishes_index_without_a_steady_state_sync() { let directory = TestDirectory::new(); let (backend, faults) = FaultBackend::open(&directory.files.data).unwrap(); backend.set_len(data.geometry.data_file_len).unwrap(); - let engine = BackendIoEngine::new(Arc::new(backend), 2).unwrap(); + let engine = IoEngine::for_test(Arc::new(backend), 2).unwrap(); let value = vec![0x5a; 16 * 1024]; let mut first = None; let mut last = None; @@ -1061,7 +1060,7 @@ fn same_hash_candidate_requires_full_key() { let directory = TestDirectory::new(); let (backend, _) = FaultBackend::open(&directory.files.data).unwrap(); backend.set_len(data.geometry.data_file_len).unwrap(); - let engine = BackendIoEngine::new(Arc::new(backend), 1).unwrap(); + let engine = IoEngine::for_test(Arc::new(backend), 1).unwrap(); let owner_key = b"collision-owner"; let foreign_key = b"collision-foreign"; let value = b"owner-value-must-not-leak"; @@ -1201,7 +1200,7 @@ fn failed_span_write_never_publishes_and_latches_miss_only() { let directory = TestDirectory::new(); let (backend, faults) = FaultBackend::open(&directory.files.data).unwrap(); backend.set_len(data.geometry.data_file_len).unwrap(); - let engine = BackendIoEngine::new(Arc::new(backend), 1).unwrap(); + let engine = IoEngine::for_test(Arc::new(backend), 1).unwrap(); let hash = hash_key(data.hash_seed, b"key"); let record_bytes = required_record_bytes(b"key".len(), 16 * 1024).unwrap(); let RegionStageValue::Staged { .. } = runtime diff --git a/cache2/src/region/mod.rs b/cache2/src/region/mod.rs index aa95545..e33d59d 100644 --- a/cache2/src/region/mod.rs +++ b/cache2/src/region/mod.rs @@ -608,7 +608,7 @@ impl FileRegionCore { #[cfg(test)] fn read_value( &self, - engine: &dyn IoEngine, + engine: &IoEngine, geometry: DataGeometry, buffer: BufferLease, hash_seed: u64, @@ -626,7 +626,7 @@ impl FileRegionCore { #[cfg(test)] fn read_value_from_descriptor( &self, - engine: &dyn IoEngine, + engine: &IoEngine, slot: ReadSlot, buffer: BufferLease, descriptor: ReadDescriptor, @@ -639,7 +639,7 @@ impl FileRegionCore { pub fn submit_value_read( &self, - engine: &dyn IoEngine, + engine: &IoEngine, slot: ReadSlot, buffer: BufferLease, descriptor: ReadDescriptor, @@ -943,7 +943,7 @@ impl FileRegionCore { pub fn flush_staging_shard( &self, staging: &RegionStaging, - engine: &dyn IoEngine, + engine: &IoEngine, shard_id: usize, ) -> io::Result> { let shard_mutation = self.lock_shard_mutation(shard_id)?; diff --git a/cache2/src/region/reader.rs b/cache2/src/region/reader.rs index 6095066..5e57d2c 100644 --- a/cache2/src/region/reader.rs +++ b/cache2/src/region/reader.rs @@ -88,7 +88,7 @@ impl ReadCompletion { impl PendingRead { #[cfg(test)] - pub fn wait(self, engine: &dyn IoEngine) -> ReadCompletion { + pub fn wait(self, engine: &IoEngine) -> ReadCompletion { let Self { descriptor, request_id, @@ -100,7 +100,7 @@ impl PendingRead { pub async fn wait_async( self, - engine: Arc, + engine: Arc, tokio_handle: &tokio::runtime::Handle, ) -> ReadCompletion { let Self { @@ -180,7 +180,7 @@ impl PendingRead { /// Validation happens before the lease is prepared or submitted. Any rejected /// operation drops its lease immediately. pub fn submit_read( - engine: &dyn IoEngine, + engine: &IoEngine, slot: ReadSlot, descriptor: ReadDescriptor, buffer: BufferLease, @@ -326,7 +326,7 @@ mod tests { use crate::io::backend::SyncMode; use crate::io::backend::SyncPoint; use crate::io::backend::WritePoint; - use crate::io::engine::BackendIoEngine; + use crate::io::engine::IoEngine; use crate::managed_memory::ManagedMemory; use crate::managed_memory::ManagedMemoryLimits; use crate::region::index::packed::PackedLocation; @@ -397,7 +397,7 @@ mod tests { #[test] fn unaligned_record_uses_one_aligned_read_and_returns_its_exact_slice() { let backend = Arc::new(RecordingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 1).unwrap(); let managed_memory = ManagedMemory::try_new(ManagedMemoryLimits { memory_limit_bytes: DIRECT_IO_ALIGNMENT, reserved_memory_bytes: 0, @@ -439,7 +439,7 @@ mod tests { #[test] fn buffered_record_uses_one_size_class_upper_bound_read() { let backend = Arc::new(RecordingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 1).unwrap(); let managed_memory = ManagedMemory::try_new(ManagedMemoryLimits { memory_limit_bytes: DIRECT_IO_ALIGNMENT, reserved_memory_bytes: 0, @@ -491,7 +491,7 @@ mod tests { #[test] fn invalid_entry_is_rejected_before_allocating_or_issuing_io() { let backend = Arc::new(RecordingBackend::default()); - let engine = BackendIoEngine::new(backend.clone(), 1).unwrap(); + let engine = IoEngine::for_test(backend.clone(), 1).unwrap(); let invalid = entry(PackedLocation::new(geometry().region_count, 0, 32).unwrap()); let error = describe_read(geometry(), 7, candidate(invalid), true).unwrap_err(); diff --git a/cache2/src/region/runtime/mod.rs b/cache2/src/region/runtime/mod.rs index 4f6b522..4771bf3 100644 --- a/cache2/src/region/runtime/mod.rs +++ b/cache2/src/region/runtime/mod.rs @@ -312,14 +312,14 @@ enum PreparedGet { } struct PendingGet { - engine: Arc, + engine: Arc, read: PendingRead, read_token: MemoryReadToken, hash: u64, } struct WaitingGet { - engine: Arc, + engine: Arc, slot_waiter: ReadSlotWaiter, descriptor: ReadDescriptor, read_token: MemoryReadToken, @@ -329,7 +329,7 @@ struct WaitingGet { } struct ReservedGet { - engine: Arc, + engine: Arc, slot: ReadSlot, descriptor: ReadDescriptor, read_token: MemoryReadToken, @@ -430,11 +430,11 @@ struct RunningOwner { struct RunningShared { core: Arc, - read_engines: Box<[Arc]>, + read_engines: Box<[Arc]>, read_lane_cursor: AtomicUsize, read_waiters: Option>, - write_engines: Box<[Arc]>, - reclaim_engines: Box<[Arc]>, + write_engines: Box<[Arc]>, + reclaim_engines: Box<[Arc]>, reclaim_control: ReclaimControl, reclaim_io_timeout: Duration, managed_memory: Arc, @@ -508,11 +508,11 @@ impl ReclaimControl { } impl RunningShared { - fn write_engine_for(&self, route: u64) -> &Arc { + fn write_engine_for(&self, route: u64) -> &Arc { &self.write_engines[route_hash(route, self.write_engines.len())] } - fn try_reserve_read(&self, route: u64) -> io::Result<(Arc, ReadSlot)> { + fn try_reserve_read(&self, route: u64) -> io::Result<(Arc, ReadSlot)> { try_reserve_read_lane(&self.read_engines, route, &self.read_lane_cursor) } @@ -546,7 +546,7 @@ impl RunningShared { }) } - fn engines(&self) -> impl Iterator> { + fn engines(&self) -> impl Iterator> { self.read_engines .iter() .chain(self.write_engines.iter()) @@ -555,11 +555,11 @@ impl RunningShared { } fn try_reserve_read_lane( - engines: &[Arc], + engines: &[Arc], route: u64, pressure_cursor: &AtomicUsize, -) -> io::Result<(Arc, ReadSlot)> { - let reserve = |lane: usize| -> io::Result<(Arc, ReadSlot)> { +) -> io::Result<(Arc, ReadSlot)> { + let reserve = |lane: usize| -> io::Result<(Arc, ReadSlot)> { let slot = engines[lane].try_reserve_read()?; Ok((Arc::clone(&engines[lane]), slot)) }; @@ -1332,9 +1332,9 @@ impl RegionDataPlane { } fn aggregate_io_stats( - read_engines: &[Arc], - write_engines: &[Arc], - reclaim_engines: &[Arc], + read_engines: &[Arc], + write_engines: &[Arc], + reclaim_engines: &[Arc], ) -> CacheIoSnapshot { let mut aggregate = CacheIoSnapshot::default(); for (engine_index, engine) in read_engines.iter().chain(write_engines).enumerate() { @@ -1582,7 +1582,7 @@ fn build_engine_pool( runtime: &RuntimeOptions, topology: IoPoolTopology, read_wait_enabled: bool, -) -> io::Result]>> { +) -> io::Result]>> { let mut source = Some(files); let engine_count = topology.engine_count(); let mut engines = Vec::new(); @@ -2141,7 +2141,7 @@ fn stop_running(mut owner: RunningOwner) -> io::Result { result.map(|()| false) } -fn reap_engine_after_target_fence(engine: &Arc) { +fn reap_engine_after_target_fence(engine: &Arc) { let reaper_engine = Arc::clone(engine); let spawn = std::thread::Builder::new() .name("cache2-io-reaper".to_owned()) @@ -2200,7 +2200,7 @@ mod tests { use super::*; use crate::fixtures::TestFile; - use crate::io::engine::BackendIoEngine; + use crate::io::engine::IoEngine; static LANE_TEST_ID: AtomicU64 = AtomicU64::new(1); @@ -2220,9 +2220,9 @@ mod tests { fn read_lane_uses_one_bounded_alternate_on_primary_pressure() { let file = TestFile::new("read-lane"); let backend = file.backend(); - let engines: Box<[Arc]> = vec![ - Arc::new(BackendIoEngine::new(Arc::clone(&backend), 1).unwrap()) as Arc, - Arc::new(BackendIoEngine::new(Arc::clone(&backend), 1).unwrap()) as Arc, + let engines: Box<[Arc]> = vec![ + Arc::new(IoEngine::for_test(Arc::clone(&backend), 1).unwrap()), + Arc::new(IoEngine::for_test(Arc::clone(&backend), 1).unwrap()), ] .into_boxed_slice(); let pressure_cursor = AtomicUsize::new(0); @@ -2257,11 +2257,8 @@ mod tests { fn hot_read_route_rotates_pressure_fallback_across_all_lanes() { let file = TestFile::new("read-lane-rotation"); let backend = file.backend(); - let engines: Box<[Arc]> = (0..4) - .map(|_| { - Arc::new(BackendIoEngine::new(Arc::clone(&backend), 1).unwrap()) - as Arc - }) + let engines: Box<[Arc]> = (0..4) + .map(|_| Arc::new(IoEngine::for_test(Arc::clone(&backend), 1).unwrap())) .collect::>() .into_boxed_slice(); let pressure_cursor = AtomicUsize::new(0); diff --git a/cache2/src/region/runtime/shutdown_tests.rs b/cache2/src/region/runtime/shutdown_tests.rs index 11a0f46..54d3eef 100644 --- a/cache2/src/region/runtime/shutdown_tests.rs +++ b/cache2/src/region/runtime/shutdown_tests.rs @@ -21,7 +21,7 @@ use crate::io::backend::IoBackend; use crate::io::backend::SyncMode; use crate::io::backend::SyncPoint; use crate::io::backend::WritePoint; -use crate::io::engine::BackendIoEngine; +use crate::io::engine::IoEngine; use crate::io::engine::IoRequest; #[derive(Default)] @@ -143,7 +143,7 @@ fn assert_close_does_not_wait_for_read(submit_before_close: bool) { // Reuse a stopped runtime's fixed resources without unrelated workers. let shared = Arc::get_mut(&mut plane.shared).unwrap(); let backend = Arc::new(BlockedRead::default()); - let engine = Arc::new(BackendIoEngine::new(backend.clone(), 1).unwrap()); + let engine = Arc::new(IoEngine::for_test(backend.clone(), 1).unwrap()); let read_engine = Arc::clone(&engine); let read_backend = Arc::clone(&backend); let managed_memory = Arc::clone(&shared.managed_memory); @@ -167,7 +167,7 @@ fn assert_close_does_not_wait_for_read(submit_before_close: bool) { submitted.send(submit_read()).unwrap(); })); } - shared.read_engines = vec![engine.clone() as Arc].into_boxed_slice(); + shared.read_engines = vec![engine.clone()].into_boxed_slice(); shared.write_engines = Box::new([]); shared.reclaim_engines = Box::new([]); shared.shards = Box::new([]); From 7090d46a2bc665ac0ad495366cbd9a9221c29ea7 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 20 Sep 2026 22:55:16 +0800 Subject: [PATCH 7/7] refactor: unify CRC32C computation over byte segments Expose one checksum function over borrowed slices and remove the incremental wrapper and checksum helpers from the field codec. Keep checksum layouts and comparisons within each persistent format, with shared encode/decode calculations and existing length validation. Migrate all callers and retain reference, golden-format, and property coverage. Validated with cargo x check, cargo x test, cargo x lint, and Linux io-uring all-targets check and Clippy. --- cache2/src/checksum.rs | 75 +++++++------------ cache2/src/codec.rs | 28 ------- cache2/src/property_tests.rs | 11 ++- .../src/region/index/storage/page_format.rs | 9 ++- cache2/src/region/mod.rs | 4 +- cache2/src/region/record/codec.rs | 7 +- cache2/src/region/record/mod.rs | 14 ++-- cache2/src/region/recovery/metadata.rs | 8 +- cache2/src/region/recovery/mod.rs | 12 +-- 9 files changed, 61 insertions(+), 107 deletions(-) diff --git a/cache2/src/checksum.rs b/cache2/src/checksum.rs index 595f303..d72b4e4 100644 --- a/cache2/src/checksum.rs +++ b/cache2/src/checksum.rs @@ -12,57 +12,33 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! CRC32C used by the on-disk format. +//! CRC32C over borrowed byte segments used by the on-disk formats. //! //! The dependency selects hardware acceleration when the host supports it and -//! retains a portable software fallback. This wrapper keeps the cache's codec -//! API and checksum values independent of that implementation detail. +//! retains a portable software fallback. Each format selects its checksum +//! input, including any fields represented as zero bytes. use hashcrew::crc::Crc32Iscsi; -use hashcrew::crc::crc32_iscsi; -/// Computes the standard CRC32C checksum of `bytes`. -pub fn crc32c(bytes: &[u8]) -> u32 { - crc32_iscsi(bytes) -} - -/// Incremental CRC32C state, useful for checksumming a key and value without first joining them in -/// a temporary allocation. -pub struct Crc32c { - digest: Crc32Iscsi, -} - -impl Crc32c { - pub fn new() -> Self { - Self { - digest: Crc32Iscsi::new(), - } - } - - pub fn update(&mut self, bytes: &[u8]) { - self.digest.update(bytes); - } - - pub fn finish(self) -> u32 { - self.digest.digest() - } -} - -impl Default for Crc32c { - fn default() -> Self { - Self::new() +/// Computes CRC32C over the concatenation of `parts` without allocating or copying. +/// Segment boundaries do not affect the result; empty input returns zero. +pub fn crc32c(parts: &[&[u8]]) -> u32 { + let mut checksum = Crc32Iscsi::new(); + for part in parts { + checksum.update(part); } + checksum.digest() } #[cfg(test)] mod tests { - use crate::checksum::Crc32c; use crate::checksum::crc32c; #[test] fn matches_the_crc32c_check_value() { - assert_eq!(crc32c(b"123456789"), 0xe306_9283); - assert_eq!(crc32c(b""), 0); + assert_eq!(crc32c(&[b"123456789"]), 0xe306_9283); + assert_eq!(crc32c(&[b""]), 0); + assert_eq!(crc32c(&[]), 0); } #[test] @@ -72,13 +48,10 @@ mod tests { for len in [0, 1, 44, 48, 4092, 4096, 65_537] { let input = &bytes[offset..offset + len]; let expected = crc_fast::crc32_iscsi(input); - assert_eq!(crc32c(input), expected); + assert_eq!(crc32c(&[input]), expected); for split in [0, len.min(44), len.min(56), len / 2, len] { - let mut checksum = Crc32c::new(); - checksum.update(&input[..split]); - checksum.update(&[]); - checksum.update(&input[split..]); - assert_eq!(checksum.finish(), expected, "len={len}, split={split}"); + let checksum = crc32c(&[&input[..split], &[], &input[split..]]); + assert_eq!(checksum, expected, "len={len}, split={split}"); } } } @@ -86,13 +59,15 @@ mod tests { // Record headers, index pages, and recovery pages zero their checksum // field without concatenating the surrounding slices. for (len, checksum_offset) in [(48, 44), (4096, 56), (4096, 4092)] { - let mut page = bytes[..len].to_vec(); - page[checksum_offset..checksum_offset + 4].fill(0); - let mut checksum = Crc32c::new(); - checksum.update(&page[..checksum_offset]); - checksum.update(&[0; 4]); - checksum.update(&page[checksum_offset + 4..]); - assert_eq!(checksum.finish(), crc_fast::crc32_iscsi(&page)); + let page = &bytes[..len]; + let mut expected = page.to_vec(); + expected[checksum_offset..checksum_offset + 4].fill(0); + let checksum = crc32c(&[ + &page[..checksum_offset], + &[0; 4], + &page[checksum_offset + 4..], + ]); + assert_eq!(checksum, crc_fast::crc32_iscsi(&expected)); } } } diff --git a/cache2/src/codec.rs b/cache2/src/codec.rs index 00f7ce8..7974a0f 100644 --- a/cache2/src/codec.rs +++ b/cache2/src/codec.rs @@ -19,8 +19,6 @@ //! input instead of panicking, while writes panic because encoders size their //! buffers from the same constants as their field offsets. -use crate::checksum::Crc32c; - /// Reads the little-endian `u16` at `offset`, or returns `None` when the field /// range falls outside `input`. pub fn get_u16(input: &[u8], offset: usize) -> Option { @@ -74,29 +72,3 @@ pub fn put_u32(output: &mut [u8], offset: usize, value: u32) { pub fn put_u64(output: &mut [u8], offset: usize, value: u64) { output[offset..offset + size_of::()].copy_from_slice(&value.to_le_bytes()); } - -/// Returns the CRC32C of `input` with the little-endian `u32` checksum field at -/// `checksum_offset` treated as zero, or `None` when the field range falls -/// outside `input`. -/// -/// Persistent headers and pages checksum their image with the checksum field -/// itself zeroed, so writers and readers cover the same bytes without copying. -pub fn crc32c_with_zeroed_u32(input: &[u8], checksum_offset: usize) -> Option { - let field_end = checksum_offset.checked_add(size_of::())?; - let before = input.get(..checksum_offset)?; - let after = input.get(field_end..)?; - let mut checksum = Crc32c::new(); - checksum.update(before); - checksum.update(&[0; size_of::()]); - checksum.update(after); - Some(checksum.finish()) -} - -/// Returns whether the stored checksum field at `checksum_offset` matches -/// [`crc32c_with_zeroed_u32`]. -pub fn crc32c_with_zeroed_u32_matches(input: &[u8], checksum_offset: usize) -> bool { - let Some(expected) = get_u32(input, checksum_offset) else { - return false; - }; - crc32c_with_zeroed_u32(input, checksum_offset) == Some(expected) -} diff --git a/cache2/src/property_tests.rs b/cache2/src/property_tests.rs index 382b386..f846596 100644 --- a/cache2/src/property_tests.rs +++ b/cache2/src/property_tests.rs @@ -20,7 +20,6 @@ use std::collections::BTreeSet; use quickcheck::Gen; use quickcheck::QuickCheck; -use crate::checksum::Crc32c; use crate::checksum::crc32c; use crate::hashing::FixedPrehashedMap; use crate::region::index::ReclaimIndexAction; @@ -142,11 +141,11 @@ fn exercise_record_roundtrip(input: &[u8]) { assert_eq!(header.value_len as usize, value.len()); assert_eq!(header.seqno, expected_seqno); assert_eq!(header.key_hash, hash); - assert_eq!(header.payload_crc, crc32c(&payload)); + assert_eq!(header.payload_crc, crc32c(&[&payload])); assert_eq!(header.region_generation, region_created_seqno); assert_eq!(header.record_len, record_bytes); - let mut chunked_crc = Crc32c::new(); + let mut chunks = Vec::new(); let mut remaining = payload.as_slice(); for control_byte in control { if remaining.is_empty() { @@ -154,11 +153,11 @@ fn exercise_record_roundtrip(input: &[u8]) { } let chunk_len = usize::from(control_byte) % remaining.len() + 1; let (chunk, tail) = remaining.split_at(chunk_len); - chunked_crc.update(chunk); + chunks.push(chunk); remaining = tail; } - chunked_crc.update(remaining); - assert_eq!(chunked_crc.finish(), header.payload_crc); + chunks.push(remaining); + assert_eq!(crc32c(&chunks), header.payload_crc); let payload_end = RECORD_HEADER_SIZE + payload.len(); assert_eq!(&destination[RECORD_HEADER_SIZE..payload_end], payload); diff --git a/cache2/src/region/index/storage/page_format.rs b/cache2/src/region/index/storage/page_format.rs index edd67fb..7736d39 100644 --- a/cache2/src/region/index/storage/page_format.rs +++ b/cache2/src/region/index/storage/page_format.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::codec::crc32c_with_zeroed_u32; +use crate::checksum::crc32c; use crate::codec::get_u16; use crate::codec::get_u32; use crate::codec::get_u64; @@ -201,8 +201,11 @@ pub fn validate_page_header( } pub fn page_checksum(page: &[u8; INDEX_IMAGE_PAGE_SIZE]) -> u32 { - crc32c_with_zeroed_u32(page, PAGE_CHECKSUM_OFFSET) - .expect("index page checksum field is in bounds") + crc32c(&[ + &page[..PAGE_CHECKSUM_OFFSET], + &[0; 4], + &page[PAGE_CHECKSUM_OFFSET + 4..], + ]) } fn read_u16(input: &[u8], offset: usize) -> u16 { diff --git a/cache2/src/region/mod.rs b/cache2/src/region/mod.rs index d5763c6..fa3af82 100644 --- a/cache2/src/region/mod.rs +++ b/cache2/src/region/mod.rs @@ -459,7 +459,7 @@ impl FileRegionCore { ) })?; let payload_valid = - crc32c(&bytes[header_end..payload_end]) == header.payload_crc; + crc32c(&[&bytes[header_end..payload_end]]) == header.payload_crc; let within_budget = u64::from(rewrite_bytes) <= reinsert_budget; let budget_exhausted = payload_valid && !within_budget; let reinserted = payload_valid @@ -719,7 +719,7 @@ impl FileRegionCore { if encoded_key != key { return Ok(None); } - if crc32c(&record[RECORD_HEADER_SIZE..payload_end]) != header.payload_crc { + if crc32c(&[&record[RECORD_HEADER_SIZE..payload_end]]) != header.payload_crc { return Ok(None); } let value_start = completion.descriptor.record_range.start + RECORD_HEADER_SIZE + key_len; diff --git a/cache2/src/region/record/codec.rs b/cache2/src/region/record/codec.rs index 2a3b588..43fb73a 100644 --- a/cache2/src/region/record/codec.rs +++ b/cache2/src/region/record/codec.rs @@ -23,7 +23,7 @@ use std::fmt; use hashcrew::xxhash::xxh3_64_with_seed; -use crate::checksum::Crc32c; +use crate::checksum::crc32c; #[cfg(test)] use crate::io::backend::DIRECT_IO_ALIGNMENT; use crate::region::index::packed::IndexEntry; @@ -94,13 +94,10 @@ pub struct RecordPayload<'a> { impl<'a> RecordPayload<'a> { pub fn new(key: &'a [u8], value: &'a [u8]) -> Self { - let mut crc = Crc32c::new(); - crc.update(key); - crc.update(value); Self { key, value, - crc: crc.finish(), + crc: crc32c(&[key, value]), } } } diff --git a/cache2/src/region/record/mod.rs b/cache2/src/region/record/mod.rs index 132a0ac..5b56374 100644 --- a/cache2/src/region/record/mod.rs +++ b/cache2/src/region/record/mod.rs @@ -18,7 +18,6 @@ //! not part of the disk format. use crate::checksum::crc32c; -use crate::codec::crc32c_with_zeroed_u32_matches; use crate::codec::get_u16; use crate::codec::get_u32; use crate::codec::get_u64; @@ -87,7 +86,7 @@ impl RecordHeader { ); put_u32(&mut output, RECORD_LEN_OFFSET, self.record_len); - let checksum = crc32c(&output); + let checksum = header_crc(&output); put_u32(&mut output, RECORD_HEADER_CRC_OFFSET, checksum); output } @@ -96,7 +95,7 @@ impl RecordHeader { if input.len() != RECORD_HEADER_SIZE || input.get(..RECORD_HEADER_MAGIC.len())? != RECORD_HEADER_MAGIC || get_u16(input, RECORD_VERSION_OFFSET)? != RECORD_FORMAT_VERSION - || !crc32c_with_zeroed_u32_matches(input, RECORD_HEADER_CRC_OFFSET) + || get_u32(input, RECORD_HEADER_CRC_OFFSET)? != header_crc(input) { return None; } @@ -133,6 +132,10 @@ impl RecordHeader { } } +fn header_crc(header: &[u8]) -> u32 { + crc32c(&[&header[..RECORD_HEADER_CRC_OFFSET], &[0; 4]]) +} + fn checked_align_up(value: usize, alignment: usize) -> Option { if !alignment.is_power_of_two() { return None; @@ -158,7 +161,7 @@ mod tests { value_len: value.len() as u32, seqno: 34, key_hash: 0x1122_3344_5566_7788, - payload_crc: crc32c(&payload), + payload_crc: crc32c(&[&payload]), region_generation: 17, record_len: RecordHeader::aligned_len(key.len(), value.len()).unwrap(), }; @@ -200,8 +203,7 @@ mod tests { RECORD_VERSION_OFFSET, RECORD_FORMAT_VERSION + 1, ); - put_u32(&mut wrong_version, RECORD_HEADER_CRC_OFFSET, 0); - let checksum = crc32c(&wrong_version); + let checksum = header_crc(&wrong_version); put_u32(&mut wrong_version, RECORD_HEADER_CRC_OFFSET, checksum); assert_eq!(RecordHeader::decode(&wrong_version), None); diff --git a/cache2/src/region/recovery/metadata.rs b/cache2/src/region/recovery/metadata.rs index c0a8579..a060ba7 100644 --- a/cache2/src/region/recovery/metadata.rs +++ b/cache2/src/region/recovery/metadata.rs @@ -21,7 +21,7 @@ use std::fmt; use std::mem; -use crate::codec::crc32c_with_zeroed_u32; +use crate::checksum::crc32c; use crate::codec::put_u16; use crate::codec::put_u32; use crate::codec::put_u64; @@ -863,7 +863,11 @@ fn finish_page(page: &mut [u8]) { } fn page_crc(page: &[u8]) -> u32 { - crc32c_with_zeroed_u32(page, PAGE_CRC_OFFSET).expect("metadata page CRC field is in bounds") + crc32c(&[ + &page[..PAGE_CRC_OFFSET], + &[0; 4], + &page[PAGE_CRC_OFFSET + 4..], + ]) } #[allow(clippy::too_many_arguments)] diff --git a/cache2/src/region/recovery/mod.rs b/cache2/src/region/recovery/mod.rs index efe5cf3..af13fe7 100644 --- a/cache2/src/region/recovery/mod.rs +++ b/cache2/src/region/recovery/mod.rs @@ -20,8 +20,7 @@ //! `CLEAN`. This module performs no I/O; callers must write the returned page //! to the selected slot and provide the required `fdatasync` barrier. -use crate::codec::crc32c_with_zeroed_u32; -use crate::codec::crc32c_with_zeroed_u32_matches; +use crate::checksum::crc32c; use crate::codec::get_u16; use crate::codec::get_u32; use crate::codec::get_u64; @@ -884,13 +883,16 @@ fn get_id(input: &[u8], offset: usize) -> Option { } fn write_page_crc(page: &mut [u8; RECOVERY_PAGE_SIZE]) { - let checksum = crc32c_with_zeroed_u32(page, PAGE_CRC_OFFSET) - .expect("recovery page CRC field is in bounds"); + let checksum = page_crc(page); put_u32(page, PAGE_CRC_OFFSET, checksum); } fn page_crc_matches(page: &[u8]) -> bool { - page.len() == RECOVERY_PAGE_SIZE && crc32c_with_zeroed_u32_matches(page, PAGE_CRC_OFFSET) + page.len() == RECOVERY_PAGE_SIZE && get_u32(page, PAGE_CRC_OFFSET) == Some(page_crc(page)) +} + +fn page_crc(page: &[u8]) -> u32 { + crc32c(&[&page[..PAGE_CRC_OFFSET], &[0; 4]]) } #[cfg(test)]