From 43f78a3d013a52065b9629aa1459737cde3c7cbe Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sat, 12 Sep 2026 13:00:23 +0200 Subject: [PATCH] fix(peer): preflight legacy migration work Bound legacy traversal by entry count and cooperative elapsed time, and complete the read-only plan before mutating any source or destination. Reuse the catalog-sized legacy-index read ceiling. Test Plan: - just test - just clippy - focused preflight and oversized migration tests - git diff --check --- crates/lanspread-peer/src/migration.rs | 205 ++++++++++++++++++++----- 1 file changed, 169 insertions(+), 36 deletions(-) diff --git a/crates/lanspread-peer/src/migration.rs b/crates/lanspread-peer/src/migration.rs index 68a6999..b49d3a9 100644 --- a/crates/lanspread-peer/src/migration.rs +++ b/crates/lanspread-peer/src/migration.rs @@ -4,9 +4,11 @@ use std::{ path::{Path, PathBuf}, sync::atomic::{AtomicUsize, Ordering}, thread, - time::Instant, + time::{Duration, Instant}, }; +use lanspread_db::content_manifest::{MAX_CATALOG_ENTRIES, MAX_CATALOG_MANIFEST_BYTES}; + use crate::{ game_paths::{ LEGACY_FIRST_START_DONE_FILE, @@ -22,7 +24,69 @@ use crate::{ }; const MIGRATION_CONCURRENCY: usize = 16; -const MAX_LEGACY_LIBRARY_INDEX_BYTES: u64 = 128 * 1024 * 1024; +const MAX_LEGACY_LIBRARY_INDEX_BYTES: u64 = MAX_CATALOG_MANIFEST_BYTES; +const MAX_MIGRATION_SCAN_ENTRIES: usize = MAX_CATALOG_ENTRIES * 10 + 1; +const MAX_MIGRATION_SCAN_DURATION: Duration = Duration::from_mins(10); + +#[derive(Clone, Copy)] +struct MigrationScanLimits { + entry_cap: usize, + time_budget: Duration, +} + +impl MigrationScanLimits { + const PRODUCTION: Self = Self { + entry_cap: MAX_MIGRATION_SCAN_ENTRIES, + time_budget: MAX_MIGRATION_SCAN_DURATION, + }; +} + +struct MigrationGameRoot { + id: String, + path: PathBuf, + unknown_softlan_files: usize, +} + +struct MigrationScanBudget { + limits: MigrationScanLimits, + entries: usize, + started: Instant, +} + +impl MigrationScanBudget { + fn new(limits: MigrationScanLimits) -> Self { + Self { + limits, + entries: 0, + started: Instant::now(), + } + } + + fn charge(&mut self) -> std::io::Result<()> { + self.entries = self.entries.checked_add(1).ok_or_else(|| { + std::io::Error::new(ErrorKind::InvalidData, "migration entry count overflow") + })?; + if self.entries > self.limits.entry_cap { + return Err(std::io::Error::new( + ErrorKind::InvalidData, + format!( + "legacy migration exceeds the {}-entry scan budget", + self.limits.entry_cap + ), + )); + } + if self.started.elapsed() >= self.limits.time_budget { + return Err(std::io::Error::new( + ErrorKind::TimedOut, + format!( + "legacy migration exceeds the {:?} scan-time budget", + self.limits.time_budget + ), + )); + } + Ok(()) + } +} #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Serialize)] pub struct MigrationReport { @@ -52,12 +116,21 @@ impl MigrationReport { /// This is intentionally separate from normal operation: callers should run it /// before starting the peer runtime for a game directory. pub async fn migrate_legacy_state(game_dir: &Path, state_dir: &Path) -> MigrationReport { + // Keep the public asynchronous boundary cooperative before entering the + // finite, lexically scoped filesystem batch. + tokio::task::yield_now().await; + migrate_legacy_state_with_limits(game_dir, state_dir, MigrationScanLimits::PRODUCTION) +} + +fn migrate_legacy_state_with_limits( + game_dir: &Path, + state_dir: &Path, + limits: MigrationScanLimits, +) -> MigrationReport { let started = Instant::now(); let mut report = MigrationReport::default(); - report.merge(migrate_library_index(game_dir, state_dir)); - - let game_roots = match scoped_blocking(|| collect_game_roots(game_dir)) { + let game_roots = match scoped_blocking(|| collect_game_roots(game_dir, limits)) { Ok(game_roots) => game_roots, Err(err) => { if err.kind() != ErrorKind::NotFound { @@ -72,6 +145,10 @@ pub async fn migrate_legacy_state(game_dir: &Path, state_dir: &Path) -> Migratio } }; + // Complete the selected-root enumeration before mutating either legacy or + // current state. A budget failure therefore leaves the prior migration + // generation intact instead of committing a truncated subset. + report.merge(migrate_library_index(game_dir, state_dir)); report.merge(scoped_blocking(|| { migrate_game_roots(state_dir, &game_roots) })); @@ -80,10 +157,15 @@ pub async fn migrate_legacy_state(game_dir: &Path, state_dir: &Path) -> Migratio report } -fn collect_game_roots(game_dir: &Path) -> std::io::Result> { +fn collect_game_roots( + game_dir: &Path, + limits: MigrationScanLimits, +) -> std::io::Result> { let mut roots = Vec::new(); + let mut budget = MigrationScanBudget::new(limits); for entry in fs::read_dir(game_dir)? { let entry = entry?; + budget.charge()?; if !entry.file_type()?.is_dir() { continue; } @@ -95,12 +177,25 @@ fn collect_game_roots(game_dir: &Path) -> std::io::Result continue; } - roots.push((id, entry.path())); + let path = entry.path(); + let unknown_softlan_files = count_unknown_softlan_files(&path, &mut budget)? + .checked_add(count_unknown_softlan_files( + &path.join("local"), + &mut budget, + )?) + .ok_or_else(|| { + std::io::Error::new(ErrorKind::InvalidData, "legacy marker count overflow") + })?; + roots.push(MigrationGameRoot { + id, + path, + unknown_softlan_files, + }); } Ok(roots) } -fn migrate_game_roots(state_dir: &Path, game_roots: &[(String, PathBuf)]) -> MigrationReport { +fn migrate_game_roots(state_dir: &Path, game_roots: &[MigrationGameRoot]) -> MigrationReport { if game_roots.is_empty() { return MigrationReport::default(); } @@ -116,10 +211,10 @@ fn migrate_game_roots(state_dir: &Path, game_roots: &[(String, PathBuf)]) -> Mig let mut report = MigrationReport::default(); loop { let index = next_root.fetch_add(1, Ordering::Relaxed); - let Some((id, root)) = game_roots.get(index) else { + let Some(root) = game_roots.get(index) else { break; }; - report.merge(migrate_game_root(state_dir, id, root)); + report.merge(migrate_game_root(state_dir, root)); } report })); @@ -165,16 +260,21 @@ fn migrate_library_index(game_dir: &Path, state_dir: &Path) -> MigrationReport { report } -fn migrate_game_root(state_dir: &Path, id: &str, root: &Path) -> MigrationReport { +fn migrate_game_root(state_dir: &Path, game_root: &MigrationGameRoot) -> MigrationReport { + let MigrationGameRoot { + id, + path: root, + unknown_softlan_files, + } = game_root; let mut report = MigrationReport { games_checked: 1, + unknown_softlan_files: *unknown_softlan_files, ..MigrationReport::default() }; report.merge(note_legacy_install_intent(root)); report.merge(migrate_setup_marker(state_dir, id, root)); report.merge(delete_if_exists(&root.join(LEGACY_SOFTLAN_INSTALL_MARKER))); - report.merge(note_unknown_softlan_files(root)); report } @@ -241,31 +341,20 @@ fn migrate_setup_marker(state_dir: &Path, id: &str, root: &Path) -> MigrationRep report } -fn note_unknown_softlan_files(root: &Path) -> MigrationReport { - MigrationReport { - unknown_softlan_files: scoped_blocking(|| { - count_unknown_softlan_files(root) + count_unknown_softlan_files(&root.join("local")) - }), - ..MigrationReport::default() - } -} - -fn count_unknown_softlan_files(dir: &Path) -> usize { +fn count_unknown_softlan_files( + dir: &Path, + budget: &mut MigrationScanBudget, +) -> std::io::Result { let mut count = 0; let entries = match fs::read_dir(dir) { Ok(entries) => entries, - Err(err) if err.kind() == ErrorKind::NotFound => return 0, - Err(err) => { - log::warn!( - "Failed to inspect {} for legacy .softlan files: {err}", - dir.display() - ); - return 0; - } + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(0), + Err(err) => return Err(err), }; for entry in entries { - let Ok(entry) = entry else { break }; + let entry = entry?; + budget.charge()?; let Some(name) = entry.file_name().to_str().map(ToOwned::to_owned) else { continue; }; @@ -276,13 +365,9 @@ fn count_unknown_softlan_files(dir: &Path) -> usize { continue; } count += 1; - log::info!( - "Leaving unknown legacy .softlan file in place: {}", - entry.path().display() - ); } - count + Ok(count) } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -616,6 +701,54 @@ mod tests { assert!(!target_path.exists()); } + #[tokio::test] + async fn selected_root_budget_failure_precedes_every_migration_mutation() { + let games = TempDir::new("lanspread-migration-budget-games"); + let state = TempDir::new("lanspread-migration-budget-state"); + let legacy_index = legacy_library_index_path(games.path()); + let legacy_marker = games + .path() + .join("game") + .join(LEGACY_SOFTLAN_INSTALL_MARKER); + write_file(&legacy_index, br#"{"revision":7,"games":{}}"#); + write_file(&legacy_marker, b""); + + let report = migrate_legacy_state_with_limits( + games.path(), + state.path(), + MigrationScanLimits { + entry_cap: 0, + time_budget: MAX_MIGRATION_SCAN_DURATION, + }, + ); + + assert_eq!(report.failures, 1); + assert!(!report.library_index_migrated); + assert!(legacy_index.exists(), "legacy index must remain in place"); + assert!( + legacy_marker.exists(), + "per-game marker must remain in place" + ); + assert!( + !local_library_index_path(state.path()).exists(), + "budget failure must not publish a partial current index" + ); + } + + #[test] + fn migration_scan_time_budget_is_explicit() { + let mut budget = MigrationScanBudget::new(MigrationScanLimits { + entry_cap: 1, + time_budget: Duration::ZERO, + }); + + let error = budget + .charge() + .expect_err("zero-duration migration scan must fail immediately"); + assert_eq!(error.kind(), ErrorKind::TimedOut); + assert!(error.to_string().contains("scan-time budget")); + } + #[tokio::test] async fn existing_app_index_wins_without_reading_oversized_legacy_bytes() { let games = TempDir::new("lanspread-migration-games");