diff --git a/Cargo.toml b/Cargo.toml index 5e1838d..bcb1359 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,6 +58,10 @@ tempfile = "3.23.0" name = "scan_bench" harness = false +[[bench]] +name = "cli_bench" +harness = false + [[bench]] name = "go_bench" harness = false @@ -77,3 +81,8 @@ harness = false [[bench]] name = "shell_bench" harness = false + +[profile.release] +lto = "thin" +codegen-units = 1 +strip = "symbols" diff --git a/benches/cli_bench.rs b/benches/cli_bench.rs new file mode 100644 index 0000000..7d09bf2 --- /dev/null +++ b/benches/cli_bench.rs @@ -0,0 +1,66 @@ +mod common; + +use std::{ + path::Path, + process::{Command, ExitStatus, Stdio}, +}; + +use criterion::{Criterion, criterion_group, criterion_main}; + +fn run_qr(config_dir: &Path, args: &[&str]) -> ExitStatus { + qr_command(config_dir, args) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .unwrap() +} + +fn qr_command(config_dir: &Path, args: &[&str]) -> Command { + let mut command = Command::new(env!("CARGO_BIN_EXE_qr")); + command + .args(args) + .env_clear() + .env("QR_CONFIG_DIR", config_dir) + .env("QR_STATS_ENABLED", "false"); + command +} + +fn cli_benchmarks(c: &mut Criterion) { + let config_tmp = tempfile::tempdir().unwrap(); + c.bench_function("cli/config_path", |b| { + b.iter(|| { + assert!(run_qr(config_tmp.path(), &["config", "path"]).success()); + }); + }); + + let exact = common::go_fixture(1_000); + c.bench_function("cli/go_exact_1000", |b| { + b.iter(|| { + assert!(run_qr(&exact.config_dir, &["go", "service-0042", "--print-path"]).success()); + }); + }); + + let miss = common::go_fixture(5_000); + let miss_args = ["go", "zzzzzzzz", "--print-path"]; + let preflight = qr_command(&miss.config_dir, &miss_args).output().unwrap(); + assert_eq!(preflight.status.code(), Some(1)); + assert!( + String::from_utf8_lossy(&preflight.stderr).contains("No project matching 'zzzzzzzz' found") + ); + c.bench_function("cli/go_no_match_5000", |b| { + b.iter(|| { + assert_eq!(run_qr(&miss.config_dir, &miss_args).code(), Some(1)); + }); + }); + + #[cfg(unix)] + c.bench_function("cli/run_true", |b| { + b.iter(|| { + assert!(run_qr(config_tmp.path(), &["run", "--output", "true"]).success()); + }); + }); +} + +criterion_group!(benches, cli_benchmarks); +criterion_main!(benches); diff --git a/benches/common/mod.rs b/benches/common/mod.rs index a14d5e1..7e8abd9 100644 --- a/benches/common/mod.rs +++ b/benches/common/mod.rs @@ -3,9 +3,10 @@ #![allow(dead_code)] use std::{ + ffi::{OsStr, OsString}, fs, path::PathBuf, - sync::{Mutex, OnceLock}, + sync::{Mutex, MutexGuard, OnceLock}, }; use quick_runner::{ @@ -20,8 +21,39 @@ pub fn env_lock() -> &'static Mutex<()> { LOCK.get_or_init(|| Mutex::new(())) } +pub struct ScopedEnvVar { + _lock: MutexGuard<'static, ()>, + key: &'static str, + previous: Option, +} + +pub fn scoped_env_var(key: &'static str, value: &OsStr) -> ScopedEnvVar { + let lock = env_lock().lock().unwrap(); + let previous = std::env::var_os(key); + unsafe { + std::env::set_var(key, value); + } + ScopedEnvVar { + _lock: lock, + key, + previous, + } +} + +impl Drop for ScopedEnvVar { + fn drop(&mut self) { + unsafe { + match &self.previous { + Some(value) => std::env::set_var(self.key, value), + None => std::env::remove_var(self.key), + } + } + } +} + pub struct ScanFixture { pub _tmp: TempDir, + pub config_dir: PathBuf, pub config: AppConfig, } @@ -29,13 +61,6 @@ pub fn scan_fixture(project_count: usize, nested_dirs: usize) -> ScanFixture { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path().join("workspace"); let cfg_dir = tmp.path().join("cfg"); - let _guard = env_lock().lock().unwrap(); - - unsafe { - std::env::set_var("QR_CONFIG_DIR", &cfg_dir); - std::env::set_var("HOME", tmp.path()); - } - for index in 0..project_count { let mut project_dir = root.join(format!("project-{index:04}")); for depth in 0..nested_dirs { @@ -45,16 +70,66 @@ pub fn scan_fixture(project_count: usize, nested_dirs: usize) -> ScanFixture { fs::write(project_dir.join(".git/config"), "").unwrap(); } - let mut config = AppConfig::load_from_env_with_path(cfg_dir.join("config.toml")).unwrap(); + let mut config = AppConfig::load_file_without_env(&cfg_dir.join("config.toml")).unwrap(); config.projects.roots = vec![root.display().to_string()]; config.projects.scan_depth = nested_dirs + 2; config.stats.db_path = cfg_dir.join("stats.db").display().to_string(); - ScanFixture { _tmp: tmp, config } + ScanFixture { + _tmp: tmp, + config_dir: cfg_dir, + config, + } +} + +pub fn hidden_tree_scan_fixture(entry_count: usize) -> ScanFixture { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("workspace"); + let cfg_dir = tmp.path().join("cfg"); + for index in 0..entry_count { + fs::create_dir_all(root.join(format!(".git/objects/shard-{index:04}/nested"))).unwrap(); + } + + let mut config = AppConfig::load_file_without_env(&cfg_dir.join("config.toml")).unwrap(); + config.projects.roots = vec![root.display().to_string()]; + config.projects.scan_depth = 4; + config.stats.db_path = cfg_dir.join("stats.db").display().to_string(); + + ScanFixture { + _tmp: tmp, + config_dir: cfg_dir, + config, + } +} + +pub fn node_modules_scan_fixture(package_count: usize) -> ScanFixture { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("workspace"); + let cfg_dir = tmp.path().join("cfg"); + let app = root.join("app"); + fs::create_dir_all(&app).unwrap(); + fs::write(app.join("package.json"), "{}").unwrap(); + for index in 0..package_count { + let dependency = app.join(format!("node_modules/package-{index:04}")); + fs::create_dir_all(&dependency).unwrap(); + fs::write(dependency.join("package.json"), "{}").unwrap(); + } + + let mut config = AppConfig::load_file_without_env(&cfg_dir.join("config.toml")).unwrap(); + config.projects.roots = vec![root.display().to_string()]; + config.projects.scan_depth = 3; + config.stats.db_path = cfg_dir.join("stats.db").display().to_string(); + + ScanFixture { + _tmp: tmp, + config_dir: cfg_dir, + config, + } } pub struct GoFixture { pub _tmp: TempDir, + pub config_dir: PathBuf, pub config: AppConfig, pub entries: Vec, } @@ -62,26 +137,21 @@ pub struct GoFixture { pub fn go_fixture(project_count: usize) -> GoFixture { let tmp = tempfile::tempdir().unwrap(); let cfg_dir = tmp.path().join("cfg"); - let _guard = env_lock().lock().unwrap(); let entries = sample_projects(project_count); let cache = ProjectCache { scanned_at_unix_ms: 1, projects: entries.clone(), }; - unsafe { - std::env::set_var("QR_CONFIG_DIR", &cfg_dir); - std::env::set_var("HOME", tmp.path()); - } - - let mut config = AppConfig::load_from_env_with_path(cfg_dir.join("config.toml")).unwrap(); + let mut config = AppConfig::load_file_without_env(&cfg_dir.join("config.toml")).unwrap(); config.projects.roots = vec![tmp.path().join("workspace").display().to_string()]; config.stats.db_path = cfg_dir.join("stats.db").display().to_string(); - config.ensure_parent_dirs().unwrap(); - write_project_cache(&config.cache_path(), &cache).unwrap(); + fs::create_dir_all(&cfg_dir).unwrap(); + write_project_cache(&cfg_dir.join("projects-cache.json"), &cache).unwrap(); GoFixture { _tmp: tmp, + config_dir: cfg_dir, config, entries, } diff --git a/benches/go_bench.rs b/benches/go_bench.rs index b98fe09..036bf1d 100644 --- a/benches/go_bench.rs +++ b/benches/go_bench.rs @@ -21,9 +21,16 @@ fn go_benchmarks(c: &mut Criterion) { ("cache_miss", "does-not-exist"), ] { let fixture = common::go_fixture(1_000); - execute_group.bench_with_input(BenchmarkId::new(label, query), &fixture, |b, fixture| { - b.iter(|| execute(&fixture.config, query).ok()); - }); + { + let _env = common::scoped_env_var("QR_CONFIG_DIR", fixture.config_dir.as_os_str()); + execute_group.bench_with_input( + BenchmarkId::new(label, query), + &fixture, + |b, fixture| { + b.iter(|| execute(&fixture.config, query).ok()); + }, + ); + } } execute_group.finish(); } diff --git a/benches/scan_bench.rs b/benches/scan_bench.rs index cbe7b6a..66b7a4e 100644 --- a/benches/scan_bench.rs +++ b/benches/scan_bench.rs @@ -12,8 +12,27 @@ fn scan_benchmarks(c: &mut Criterion) { ("large", 300, 3), ] { let fixture = common::scan_fixture(projects, nested_dirs); - group.bench_with_input(BenchmarkId::new(label, projects), &fixture, |b, fixture| { - b.iter(|| scan_projects(&fixture.config).unwrap()); + { + let _env = common::scoped_env_var("QR_CONFIG_DIR", fixture.config_dir.as_os_str()); + group.bench_with_input(BenchmarkId::new(label, projects), &fixture, |b, fixture| { + b.iter(|| scan_projects(&fixture.config).unwrap()); + }); + } + } + + let hidden = common::hidden_tree_scan_fixture(1_000); + { + let _env = common::scoped_env_var("QR_CONFIG_DIR", hidden.config_dir.as_os_str()); + group.bench_function("prune_hidden_tree/1000", |b| { + b.iter(|| scan_projects(&hidden.config).unwrap()); + }); + } + + let node_modules = common::node_modules_scan_fixture(1_000); + { + let _env = common::scoped_env_var("QR_CONFIG_DIR", node_modules.config_dir.as_os_str()); + group.bench_function("prune_node_modules/1000", |b| { + b.iter(|| scan_projects(&node_modules.config).unwrap()); }); } diff --git a/docs/superpowers/plans/2026-07-10-frame-budget-performance.md b/docs/superpowers/plans/2026-07-10-frame-budget-performance.md new file mode 100644 index 0000000..c0fc806 --- /dev/null +++ b/docs/superpowers/plans/2026-07-10-frame-budget-performance.md @@ -0,0 +1,182 @@ +# Frame-Budget Performance Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Remove the measured multi-second telemetry tail and pathological scanner traversal while adding process-level performance coverage and a smaller optimized release profile. + +**Architecture:** Keep the current command and project-detection semantics. Telemetry writes get a dedicated short lock-wait policy; explicit stats reads retain the existing policy. The scanner still recognizes `.git` plus all six marker files, but `WalkDir` stops descending into hidden directories and `node_modules`. Criterion gains executable-level and pathological-tree fixtures, and release binaries use thin LTO, one codegen unit, and symbol stripping without changing panic behavior. + +**Tech Stack:** Rust 2024, rusqlite, walkdir, Criterion, Cargo release profiles + +--- + +## Scope decisions + +- Preserve all project markers: `Cargo.toml`, `package.json`, `go.mod`, `pyproject.toml`, `requirements.txt`, and `Makefile`. +- Keep the current synchronous rescan when the project cache is missing, empty, or corrupt. +- Prune hidden descendant directories and `node_modules`; do not add broad names such as `target`, `build`, or `vendor`, which could be legitimate project directories. +- Do not add stats schema migration scaffolding or change the `command_runs` schema. +- Do not use `panic = "abort"`; release tuning must not change failure semantics. + +### Task 1: Fast-fail best-effort telemetry writes + +**Files:** +- Modify: `src/stats_db.rs` +- Modify: `src/main.rs` +- Test: `src/stats_db.rs` + +- [ ] **Step 1: Write a failing lock-contention test** + +Add a unit test that creates the schema, holds `BEGIN IMMEDIATE` on one connection, opens a telemetry connection, attempts a record, and asserts the attempt returns an error in well under the existing three-second timeout. The test must call a new `StatsDb::open_for_telemetry` API so it fails before implementation. + +```rust +#[test] +fn telemetry_record_fails_fast_when_another_writer_holds_the_database() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("stats.db"); + drop(StatsDb::open(&path).unwrap()); + + let lock = Connection::open(&path).unwrap(); + lock.execute_batch("BEGIN IMMEDIATE").unwrap(); + + let started = std::time::Instant::now(); + let result = StatsDb::open_for_telemetry(&path) + .and_then(|db| db.record(&CommandStats::default())); + + assert!(result.is_err()); + assert!(started.elapsed() < std::time::Duration::from_millis(250)); +} +``` + +- [ ] **Step 2: Run the test and verify RED** + +Run: `cargo test stats_db::tests::telemetry_record_fails_fast_when_another_writer_holds_the_database -- --exact` + +Expected: compilation fails because `open_for_telemetry` does not exist. + +- [ ] **Step 3: Implement the short telemetry timeout** + +Refactor `StatsDb::open` through one internal constructor. Keep the existing three-second timeout for explicit opens, and add `open_for_telemetry` with a 5 ms timeout. Do not change WAL, schema initialization, or the schema itself. + +```rust +const DEFAULT_BUSY_TIMEOUT: Duration = Duration::from_secs(3); +const TELEMETRY_BUSY_TIMEOUT: Duration = Duration::from_millis(5); + +pub fn open(path: &Path) -> Result { + Self::open_with_busy_timeout(path, DEFAULT_BUSY_TIMEOUT) +} + +pub fn open_for_telemetry(path: &Path) -> Result { + Self::open_with_busy_timeout(path, TELEMETRY_BUSY_TIMEOUT) +} +``` + +Update `record_stats` in `src/main.rs` to use `open_for_telemetry`. + +- [ ] **Step 4: Verify GREEN** + +Run the focused test again, then run `cargo test stats_db::tests --lib`. + +### Task 2: Prune traversal without removing project detection + +**Files:** +- Modify: `src/scanner.rs` +- Test: `src/scanner.rs` + +- [ ] **Step 1: Write failing hidden-tree and dependency-tree tests** + +Extend scanner coverage with: + +1. A hidden directory containing a visible child with `package.json`; the child must not become a project. +2. An application containing `node_modules/dependency/package.json`; the dependency must not become a project. + +Set `scan_depth` high enough that the current iterator discovers both children, then assert only the intended application project is present. + +- [ ] **Step 2: Run both tests and verify RED** + +Run the two focused scanner tests. Expected: the hidden child and dependency are incorrectly returned as projects. + +- [ ] **Step 3: Add traversal pruning** + +Use `WalkDir::into_iter().filter_entry(...)` before `filter_map`. Descend into the configured root, but reject descendant directories when the name starts with `.` or equals `node_modules`. + +```rust +fn should_descend(entry: &walkdir::DirEntry) -> bool { + if entry.depth() == 0 || !entry.file_type().is_dir() { + return true; + } + entry.file_name().to_str().is_none_or(|name| { + !name.starts_with('.') && name != "node_modules" + }) +} +``` + +Do not modify `PROJECT_MARKERS` or `detect_project`. + +- [ ] **Step 4: Verify GREEN and existing scanner behavior** + +Run the focused tests, then `cargo test scanner::tests --lib`. + +### Task 3: Add executable-level and pathological-tree benchmarks + +**Files:** +- Create: `benches/cli_bench.rs` +- Modify: `benches/common/mod.rs` +- Modify: `benches/scan_bench.rs` +- Modify: `Cargo.toml` + +- [ ] **Step 1: Add a Criterion binary benchmark target** + +Configure `cli_bench` with `harness = false`. Use `env!("CARGO_BIN_EXE_qr")`, isolated config directories, inherited-null stdio, and stats disabled. Benchmark: + +- `qr config path` +- exact `qr go --print-path` over 1,000 cached projects +- no-match `qr go --print-path` over 5,000 cached projects +- `qr run --output true` on Unix + +- [ ] **Step 2: Add pathological scan fixtures** + +Add fixtures for a large hidden `.git`-like subtree and a `node_modules` dependency tree. Benchmark them separately from the existing small/medium/large synthetic project counts so regressions in pruning remain visible. + +- [ ] **Step 3: Compile and smoke-run the benchmarks** + +Run: + +```text +cargo bench --bench cli_bench --no-run +cargo bench --bench scan_bench --no-run +``` + +Then run each benchmark with Criterion's quick mode or a reduced sample size and confirm every command/fixture executes successfully. + +### Task 4: Tune and measure the release profile + +**Files:** +- Modify: `Cargo.toml` + +- [ ] **Step 1: Add release settings** + +```toml +[profile.release] +lto = "thin" +codegen-units = 1 +strip = "symbols" +``` + +- [ ] **Step 2: Build and record the artifact** + +Run `cargo build --release --locked`, record the binary size, and measure `config path`, exact cached `go`, and no-match cached `go` using the same isolated fixtures as the audit. + +- [ ] **Step 3: Verify the complete change** + +Run: + +```text +cargo fmt --all -- --check +cargo clippy --all-targets --locked -- -D warnings +cargo test --locked +cargo bench --bench cli_bench --no-run +cargo bench --bench scan_bench --no-run +``` + +Confirm `git diff --check` and inspect the complete diff before publishing. diff --git a/src/main.rs b/src/main.rs index 22e4fbf..0933a38 100644 --- a/src/main.rs +++ b/src/main.rs @@ -275,7 +275,7 @@ fn run_with_config(command: Commands) -> Result { } fn record_stats(config: &AppConfig, stats: &CommandStats) -> Result<()> { - let db = StatsDb::open(&config.stats_db_path())?; + let db = StatsDb::open_for_telemetry(&config.stats_db_path())?; db.record(stats) } diff --git a/src/scanner.rs b/src/scanner.rs index 8c82eab..344929c 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -1,5 +1,6 @@ use std::{ collections::HashSet, + ffi::OsStr, fs, path::{Path, PathBuf}, time::{SystemTime, UNIX_EPOCH}, @@ -20,6 +21,18 @@ const PROJECT_MARKERS: &[&str] = &[ "Makefile", ]; +fn should_descend_directory_name(name: &OsStr) -> bool { + let is_hidden = name.as_encoded_bytes().first() == Some(&b'.'); + !is_hidden && name != OsStr::new("node_modules") +} + +fn should_descend(entry: &walkdir::DirEntry) -> bool { + if entry.depth() == 0 || !entry.file_type().is_dir() { + return true; + } + should_descend_directory_name(entry.file_name()) +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct ProjectEntry { pub name: String, @@ -49,6 +62,7 @@ pub fn scan_projects(config: &AppConfig) -> Result { .min_depth(0) .max_depth(config.projects.scan_depth) .into_iter() + .filter_entry(should_descend) .filter_map(Result::ok) .filter(|entry| { entry @@ -222,6 +236,35 @@ mod tests { use super::*; use crate::config::AppConfig; use crate::test_env_lock; + use std::ffi::{OsStr, OsString}; + #[cfg(unix)] + use std::os::unix::ffi::OsStringExt; + + struct EnvVarRestoreGuard { + key: &'static str, + previous: Option, + } + + impl EnvVarRestoreGuard { + fn set(key: &'static str, value: &OsStr) -> Self { + let previous = std::env::var_os(key); + unsafe { + std::env::set_var(key, value); + } + Self { key, previous } + } + } + + impl Drop for EnvVarRestoreGuard { + fn drop(&mut self) { + unsafe { + match &self.previous { + Some(value) => std::env::set_var(self.key, value), + None => std::env::remove_var(self.key), + } + } + } + } #[test] fn scanner_uses_git_remote_name_when_available() { @@ -379,6 +422,73 @@ mod tests { } } + #[test] + fn scan_projects_does_not_discover_projects_nested_under_hidden_directories() { + let _guard = test_env_lock().lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("dev"); + let cfg_dir = tmp.path().join("cfg"); + fs::create_dir_all(root.join(".hidden/visible-project")).unwrap(); + fs::write(root.join(".hidden/visible-project/package.json"), "{}").unwrap(); + let _env = EnvVarRestoreGuard::set("QR_CONFIG_DIR", cfg_dir.as_os_str()); + + let mut config = AppConfig::load_from_env_with_path(cfg_dir.join("config.toml")).unwrap(); + config.projects.roots = vec![root.display().to_string()]; + config.projects.scan_depth = 3; + config.stats.db_path = cfg_dir.join("stats.db").display().to_string(); + + let cache = scan_projects(&config).unwrap(); + assert!(cache.projects.is_empty()); + } + + #[cfg(unix)] + #[test] + fn should_not_descend_into_non_utf8_hidden_directory_names() { + let _guard = test_env_lock().lock().unwrap(); + let _original_env = EnvVarRestoreGuard::set("QR_CONFIG_DIR", OsStr::new("pre-existing")); + let tmp = tempfile::tempdir().unwrap(); + + { + let _env = EnvVarRestoreGuard::set("QR_CONFIG_DIR", tmp.path().as_os_str()); + // macOS rejects invalid-byte path components, so exercise the exact + // descendant-directory name predicate that `should_descend` calls. + let visible_name = OsString::from_vec(b"visible-\xff".to_vec()); + let hidden_name = OsString::from_vec(b".hidden-\xff".to_vec()); + + assert!(visible_name.to_str().is_none()); + assert!(hidden_name.to_str().is_none()); + assert!(should_descend_directory_name(&visible_name)); + assert!(!should_descend_directory_name(&hidden_name)); + } + + assert_eq!( + std::env::var_os("QR_CONFIG_DIR"), + Some(OsString::from("pre-existing")) + ); + } + + #[test] + fn scan_projects_does_not_discover_projects_inside_node_modules() { + let _guard = test_env_lock().lock().unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("dev"); + let cfg_dir = tmp.path().join("cfg"); + let app = root.join("app"); + fs::create_dir_all(app.join("node_modules/dependency")).unwrap(); + fs::write(app.join("package.json"), "{}").unwrap(); + fs::write(app.join("node_modules/dependency/package.json"), "{}").unwrap(); + let _env = EnvVarRestoreGuard::set("QR_CONFIG_DIR", cfg_dir.as_os_str()); + + let mut config = AppConfig::load_from_env_with_path(cfg_dir.join("config.toml")).unwrap(); + config.projects.roots = vec![root.display().to_string()]; + config.projects.scan_depth = 4; + config.stats.db_path = cfg_dir.join("stats.db").display().to_string(); + + let cache = scan_projects(&config).unwrap(); + assert_eq!(cache.projects.len(), 1); + assert_eq!(cache.projects[0].path, app.display().to_string()); + } + #[test] fn cache_round_trip_reads_and_writes() { let tmp = tempfile::tempdir().unwrap(); diff --git a/src/stats_db.rs b/src/stats_db.rs index c32b51b..616073a 100644 --- a/src/stats_db.rs +++ b/src/stats_db.rs @@ -1,8 +1,11 @@ -use std::{fs, path::Path}; +use std::{fs, path::Path, time::Duration}; use anyhow::Result; use rusqlite::{Connection, params}; +const DEFAULT_BUSY_TIMEOUT: Duration = Duration::from_secs(3); +const TELEMETRY_BUSY_TIMEOUT: Duration = Duration::from_millis(5); + #[derive(Debug, Clone, Default)] pub struct CommandStats { pub command_type: String, @@ -35,13 +38,21 @@ pub struct StatsDb { impl StatsDb { pub fn open(path: &Path) -> Result { + Self::open_with_busy_timeout(path, DEFAULT_BUSY_TIMEOUT) + } + + pub fn open_for_telemetry(path: &Path) -> Result { + Self::open_with_busy_timeout(path, TELEMETRY_BUSY_TIMEOUT) + } + + fn open_with_busy_timeout(path: &Path, busy_timeout: Duration) -> Result { if let Some(parent) = path.parent() { fs::create_dir_all(parent)?; } let connection = Connection::open(path)?; // Wait for a concurrent writer rather than failing immediately with // "database is locked" — several qr invocations can record at once. - connection.busy_timeout(std::time::Duration::from_secs(3))?; + connection.busy_timeout(busy_timeout)?; // WAL lets a reader (qr stats) run alongside a writer. Best-effort: it is // unsupported on some filesystems, and stats are non-critical. let _ = connection.pragma_update(None, "journal_mode", "WAL"); @@ -157,6 +168,23 @@ fn saturating_u64_from_f64(value: f64) -> u64 { mod tests { use super::*; + #[test] + fn telemetry_record_fails_fast_when_another_writer_holds_the_database() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("stats.db"); + drop(StatsDb::open(&path).unwrap()); + + let lock = Connection::open(&path).unwrap(); + lock.execute_batch("BEGIN IMMEDIATE").unwrap(); + + let started = std::time::Instant::now(); + let result = + StatsDb::open_for_telemetry(&path).and_then(|db| db.record(&CommandStats::default())); + + assert!(result.is_err()); + assert!(started.elapsed() < std::time::Duration::from_millis(250)); + } + #[test] fn stats_summary_aggregates_runs() { let db = StatsDb::open_in_memory().unwrap();