diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fb7f56b..b1808215 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Performance +- **Shard offload paths are precomputed at shard init — the recurring + tick paths no longer allocate (#45).** The 100ms eviction tick, the + memory-pressure cascade, the 10s warm-transition check, and the + cold-orphan sweep each rebuilt `/shard-{id}` via + `format!` + `PathBuf::join` (plus a `PathBuf` clone inside + `effective_disk_offload_dir()`) on every firing, per shard — a + CLAUDE.md hot-path-allocation violation. The event loop's existing + per-shard `disk_offload_dir` (built once at init, `Some` iff + disk-offload is enabled) is now threaded into `run_eviction_tick` + and `handle_memory_pressure` as `Option<&Path>` and used directly by + the warm/orphan tick arms. Cold event-gated builders (save trigger, + reclamation-schedule persist, startup/recovery) are unchanged. - **SCAN cold-plane pages now range-resume from the cursor — no full cold-index filter per page (#368).** The in-RAM cold index (spilled keys under disk-offload) is now ordered by the same `(hash48, key)` diff --git a/src/shard/event_loop.rs b/src/shard/event_loop.rs index 1b3217d1..ae0682e8 100644 --- a/src/shard/event_loop.rs +++ b/src/shard/event_loop.rs @@ -1719,14 +1719,15 @@ impl super::Shard { } // Warm tier transition check (10s interval, disk-offload only) _ = warm_check_interval.0.tick() => { - if server_config.disk_offload_enabled() { + // task/issue #45: `disk_offload_dir` (Some iff offload + // enabled) is precomputed at shard init — no per-tick + // format!/join. + if let Some(shard_dir) = disk_offload_dir.as_deref() { if let Some(ref mut manifest) = shard_manifest { - let shard_dir = server_config.effective_disk_offload_dir() - .join(format!("shard-{}", shard_id)); crate::shard::slice::with_shard(|s| { persistence_tick::check_warm_transitions( &s.vector_store, - &shard_dir, + shard_dir, manifest, server_config.segment_warm_after, server_config.engine_offload_idle_secs, @@ -1758,16 +1759,17 @@ impl super::Shard { std::future::pending::<()>().await; } }, if orphan_sweep_interval.is_some() && server_config.disk_offload_enabled() => { - let shard_dir = server_config - .effective_disk_offload_dir() - .join(format!("shard-{}", shard_id)); - timers::run_cold_orphan_sweep( - &shard_databases, - shard_id, - &shard_dir, - shard_manifest.as_mut(), - cached_clock.ms(), - ); + // task/issue #45: precomputed shard dir; the select guard + // already requires disk-offload, so this is always Some. + if let Some(shard_dir) = disk_offload_dir.as_deref() { + timers::run_cold_orphan_sweep( + &shard_databases, + shard_id, + shard_dir, + shard_manifest.as_mut(), + cached_clock.ms(), + ); + } } // Expire timed-out blocked clients every 10ms _ = block_timeout_interval.0.tick() => { @@ -1812,6 +1814,7 @@ impl super::Shard { &mut wal_writer, &script_cache_rc, &spill_file_id, + disk_offload_dir.as_deref(), &repl_backlog, &mut replica_txs, &repl_offsets, aof_pool.as_ref(), match wal_kv_log_mode { @@ -2392,6 +2395,7 @@ impl super::Shard { &mut wal_writer, &script_cache_rc, &spill_file_id, + disk_offload_dir.as_deref(), &repl_backlog, &mut replica_txs, &repl_offsets, @@ -2466,15 +2470,15 @@ impl super::Shard { } // Warm tier check: every warm_poll_ms ticks if monoio_tick_counter % (warm_poll_ms as u64) == 0 { - if server_config.disk_offload_enabled() { + // task/issue #45: `disk_offload_dir` (Some iff offload + // enabled) is precomputed at shard init — no per-tick + // format!/join. + if let Some(shard_dir) = disk_offload_dir.as_deref() { if let Some(ref mut manifest) = shard_manifest { - let shard_dir = server_config - .effective_disk_offload_dir() - .join(format!("shard-{}", shard_id)); crate::shard::slice::with_shard(|s| { persistence_tick::check_warm_transitions( &s.vector_store, - &shard_dir, + shard_dir, manifest, server_config.segment_warm_after, server_config.engine_offload_idle_secs, @@ -2527,15 +2531,12 @@ impl super::Shard { // Matches the tokio select! branch above. Disabled when interval is 0. if orphan_sweep_interval_secs > 0 && monoio_tick_counter % (orphan_sweep_interval_secs * 1000) == 0 - && server_config.disk_offload_enabled() + && let Some(shard_dir) = disk_offload_dir.as_deref() { - let shard_dir = server_config - .effective_disk_offload_dir() - .join(format!("shard-{}", shard_id)); timers::run_cold_orphan_sweep( &shard_databases, shard_id, - &shard_dir, + shard_dir, shard_manifest.as_mut(), cached_clock.ms(), ); diff --git a/src/shard/persistence_tick.rs b/src/shard/persistence_tick.rs index 92f4161b..d878f68c 100644 --- a/src/shard/persistence_tick.rs +++ b/src/shard/persistence_tick.rs @@ -357,6 +357,13 @@ pub(crate) fn run_eviction_tick( wal_v3_writer: &mut Option, script_cache: &std::rc::Rc>, spill_file_id: &std::rc::Rc>, + // task/issue #45: `/shard-{id}`, precomputed ONCE at shard init + // (event_loop's `disk_offload_dir`) instead of a per-tick + // `format!` + `join` allocation pair on this 100ms path. `None` iff + // disk-offload is disabled — the same gate that nulls `spill_thread` + // and `shard_manifest`, so every consumer below is already + // Some-guarded. + offload_shard_dir: Option<&std::path::Path>, // task #34 (Wave A): `record_reason_del` handles for plain-dropped // eviction victims — threaded straight through to `timers::run_eviction` // and `handle_memory_pressure`'s sync-spill-fallback branch. @@ -514,11 +521,11 @@ pub(crate) fn run_eviction_tick( shard_databases, shard_id, runtime_config, - server_config, shard_manifest, next_file_id, wal_v3_writer, spill_thread, + offload_shard_dir, repl_backlog, replica_txs, repl_state, @@ -537,15 +544,13 @@ pub(crate) fn run_eviction_tick( // its pre-existing fail-close plain-drop (policy-aware: `noeviction` // still OOMs, an evicting policy still frees RAM, just with no cold // copy -- matches PR #273's fail-close discipline). - let eviction_shard_dir = server_config - .effective_disk_offload_dir() - .join(format!("shard-{}", shard_id)); let mut spill_ctx: Option> = None; if server_config.disk_offload_enabled() + && let Some(shard_dir) = offload_shard_dir && let Some(ref mut manifest) = *shard_manifest { spill_ctx = Some(crate::storage::eviction::SpillContext { - shard_dir: &eviction_shard_dir, + shard_dir, manifest, next_file_id, // Placeholder — `run_eviction` restamps per database (#139). @@ -750,11 +755,12 @@ pub(crate) fn handle_memory_pressure( shard_databases: &std::sync::Arc, shard_id: usize, runtime_config: &std::sync::Arc>, - server_config: &std::sync::Arc, shard_manifest: &mut Option, next_file_id: &mut u64, wal_v3: &mut Option, spill_thread: Option<&crate::storage::tiered::spill_thread::SpillThread>, + // task/issue #45: see `run_eviction_tick`. + offload_shard_dir: Option<&std::path::Path>, // task #34 (Wave A): see `run_eviction_tick`. repl_backlog: &crate::replication::backlog::SharedBacklog, replica_txs: &mut Vec, @@ -786,13 +792,12 @@ pub(crate) fn handle_memory_pressure( // stub), which actually returns RAM — and stays reloadable-on-touch. We // pass `warm_after = u64::MAX` so the age-based HOT->WARM path stays // disabled here; only the idle->COLD path fires. - if let Some(ref mut manifest) = *shard_manifest { - let shard_dir = server_config - .effective_disk_offload_dir() - .join(format!("shard-{}", shard_id)); + if let Some(ref mut manifest) = *shard_manifest + && let Some(shard_dir) = offload_shard_dir + { let count = crate::shard::slice::with_shard(|s| { s.vector_store.try_warm_transitions_all_idle( - &shard_dir, + shard_dir, manifest, u64::MAX, PRESSURE_OFFLOAD_IDLE_SECS, @@ -843,11 +848,12 @@ pub(crate) fn handle_memory_pressure( }; if total_mem > budget { let db_count = shard_databases.db_count(); - let shard_dir = server_config - .effective_disk_offload_dir() - .join(format!("shard-{}", shard_id)); + // task/issue #45: `offload_shard_dir` is precomputed at shard + // init. `spill_thread` and `shard_manifest` share its + // disk-offload gate, so both branches below pair their + // existing Some-guard with the dir's. - if let Some(spill_t) = spill_thread { + if let (Some(spill_t), Some(shard_dir)) = (spill_thread, offload_shard_dir) { // Async spill path: background thread does pwrite under // `--appendonly yes` (AOF-backstopped fast path). Under // `--appendonly no` there is no AOF backstop, so @@ -865,7 +871,7 @@ pub(crate) fn handle_memory_pressure( db, &rt, &sender, - &shard_dir, + shard_dir, next_file_id, total_mem, i, @@ -899,9 +905,11 @@ pub(crate) fn handle_memory_pressure( // Sync spill fallback for i in 0..db_count { crate::shard::slice::with_shard_db(i, |db| { - if let Some(ref mut manifest) = *shard_manifest { + if let Some(ref mut manifest) = *shard_manifest + && let Some(shard_dir) = offload_shard_dir + { let mut ctx = crate::storage::eviction::SpillContext { - shard_dir: &shard_dir, + shard_dir, manifest, next_file_id, // #139: this fallback runs inside the