diff --git a/docs/index-contract.md b/docs/index-contract.md index 5b0cfb9..4582af5 100644 --- a/docs/index-contract.md +++ b/docs/index-contract.md @@ -63,7 +63,8 @@ candidate retrieval and graph joins. Readers are query-only (`src/store/read_access.rs:open_read`): they open the index read-only, run no schema or metadata writes, wait at most 2 s on a lock, and are interrupted when the request's 20 s query deadline (`src/daemon/deadline.rs`) expires, answering -`timed_out`. A missing index or generation 0 answers `index_warming`. Only +`timed_out`; in-request reconciliation (`--no-daemon`, an unpublished index) +pauses that clock. A missing index or generation 0 answers `index_warming`. Only indexing, sessions, and the reconciler open the writer (`Store::open`). A reader may read a linked worktree's current bytes through its parent member's index (root-relative paths address the same files); its result sets are saved in the diff --git a/src/cli/dispatch.rs b/src/cli/dispatch.rs index 40969a0..778982a 100644 --- a/src/cli/dispatch.rs +++ b/src/cli/dispatch.rs @@ -143,9 +143,9 @@ fn local_dispatch( ), }; } - let deadline = QueryDeadline::start(); + let mut deadline = QueryDeadline::start(); let mut store = if matches!(verb, "show" | "more" | "ctx" | "search" | "refs" | "map") { - read_store(root, cache, verb, daemon_running, deadline)? + read_store(root, cache, verb, daemon_running, &mut deadline)? } else { Store::open(root, cache)? }; @@ -285,21 +285,23 @@ fn local_dispatch( } /// The query-only store for a read verb. `--no-daemon` searches reconcile -/// first; an unpublished index is reconciled in the request. +/// first; an unpublished index is reconciled in the request. Reconciliation +/// is paused off `deadline`, so only the query spends its budget. fn read_store( root: &Path, cache: &Path, verb: &str, daemon_running: bool, - deadline: QueryDeadline, + deadline: &mut QueryDeadline, ) -> Result { + let reconcile = || Store::open(root, cache)?.index().map(drop); if !daemon_running && matches!(verb, "search" | "refs" | "map") { - Store::open(root, cache)?.index()?; + deadline.pause_during(reconcile)?; } - match Store::open_read(root, cache, deadline) { + match Store::open_read(root, cache, *deadline) { Err(error) if is_index_warming(&error) => { - Store::open(root, cache)?.index()?; - Store::open_read(root, cache, deadline) + deadline.pause_during(reconcile)?; + Store::open_read(root, cache, *deadline) } opened => opened, } @@ -312,3 +314,29 @@ fn ensure_semantic_check_arity(options: &Arguments) -> Result<()> { ); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::time::{Duration, Instant}; + + #[test] + fn slow_in_request_reconcile_leaves_the_query_budget_intact() { + let root = tempfile::tempdir().unwrap(); + let cache = tempfile::tempdir().unwrap(); + crate::daemon::deadline::write_slow_reconcile_fixture(root.path(), "alpha"); + let budget = Duration::from_millis(100); + let mut deadline = QueryDeadline::after(budget); + let started = Instant::now(); + let store = read_store(root.path(), cache.path(), "search", false, &mut deadline).unwrap(); + assert!( + started.elapsed() > budget, + "reconcile finished within the budget" + ); + let query = search::Query::parse("sym:alpha").unwrap(); + let found = search::search(&store, &query, false, cache.path()) + .map_err(|error| deadline.classify(error)) + .unwrap(); + assert!(!found.hits.is_empty()); + } +} diff --git a/src/daemon/deadline.rs b/src/daemon/deadline.rs index fc1f831..89dd006 100644 --- a/src/daemon/deadline.rs +++ b/src/daemon/deadline.rs @@ -34,6 +34,16 @@ impl QueryDeadline { self.expires.saturating_duration_since(Instant::now()) } + /// Runs `work` off the query clock: the deadline moves later by the time it + /// took. In-request reconciliation uses this so indexing never spends a + /// query's budget; stores opened earlier keep their old deadline. + pub fn pause_during(&mut self, work: impl FnOnce() -> T) -> T { + let started = Instant::now(); + let result = work(); + self.expires += started.elapsed(); + result + } + /// `wait`, shortened so it never outlives this deadline. pub fn cap(&self, wait: Duration) -> Duration { wait.min(self.remaining()) @@ -65,6 +75,22 @@ pub fn is_timed_out(error: &anyhow::Error) -> bool { .is_some_and(|rest| rest.starts_with(':')) } +/// Writes enough Rust sources under `root` that reconciling them outlasts a +/// 100 ms query budget, plus one file defining `pub fn NAME`. +#[cfg(test)] +pub(crate) fn write_slow_reconcile_fixture(root: &std::path::Path, name: &str) { + let mut body = String::with_capacity(64 * 48); + for item in 0..64 { + body.push_str(&format!( + "pub fn filler_{item}(value: u8) -> u8 {{ value + {item} }}\n" + )); + } + for file in 0..48 { + std::fs::write(root.join(format!("filler_{file}.rs")), &body).unwrap(); + } + std::fs::write(root.join("target.rs"), format!("pub fn {name}() {{}}\n")).unwrap(); +} + #[cfg(test)] mod tests { use super::*; @@ -83,6 +109,14 @@ mod tests { ); } + #[test] + fn paused_work_does_not_spend_the_budget() { + let mut deadline = QueryDeadline::after(Duration::from_millis(100)); + deadline.pause_during(|| std::thread::sleep(Duration::from_millis(200))); + assert!(!deadline.expired()); + assert!(deadline.remaining() > Duration::from_millis(50)); + } + #[test] fn classify_relabels_only_sqlite_interrupts() { let deadline = QueryDeadline::start(); diff --git a/src/source/reextraction.rs b/src/source/reextraction.rs index 3a93caa..26ec8ff 100644 --- a/src/source/reextraction.rs +++ b/src/source/reextraction.rs @@ -1,11 +1,12 @@ //! Current-bytes definition lookup for files whose published revision is behind //! disk: extracts one file in memory without touching any index. -use super::{MAX_READ_BYTES, line_span, read_contained}; +use super::{line_span, read_contained}; use crate::{ + extract::Extraction, identity::{ByteSpan, ContentRevision}, results::Hit, }; -use anyhow::Result; +use anyhow::{Result, bail, ensure}; use std::path::Path; /// One definition located in a file's current bytes; unpublished, so unverified @@ -48,7 +49,8 @@ impl ReextractedDefinition { /// Reads root-relative `path` under `root`, extracts it, and returns the /// definition named `name` (of `kind`, any kind when empty) whose first line -/// is nearest `near_line`; earlier definitions win ties. +/// is nearest `near_line`; earlier definitions win ties. Files the index +/// excludes and extractions that yield no facts are errors, not `None`. pub fn reextract_definition( root: &Path, path: &str, @@ -56,8 +58,16 @@ pub fn reextract_definition( kind: &str, near_line: usize, ) -> Result> { - let bytes = read_contained(root, &crate::store::decode_path(path)?, MAX_READ_BYTES)?; - let extraction = crate::extract::extract(path, &bytes); + let bytes = read_contained( + root, + &crate::store::decode_path(path)?, + crate::store::MAX_SOURCE_BYTES as usize, + )?; + ensure!( + !bytes.contains(&0), + "source_excluded: {path} is binary; the index does not extract it" + ); + let extraction = usable_extraction(path, crate::extract::extract(path, &bytes))?; let Some(definition) = extraction .definitions .into_iter() @@ -92,6 +102,20 @@ pub fn reextract_definition( })) } +/// A cancelled extraction, or one that failed without any facts, leaves the +/// file's definitions unknown rather than absent. +fn usable_extraction(path: &str, extraction: Extraction) -> Result { + let factless_failure = extraction.definitions.is_empty() + && !matches!(extraction.status.as_str(), "complete" | "lexical_only"); + if extraction.status == "cancelled" || factless_failure { + bail!( + "extraction_unavailable: {path} extraction {}; its definitions are unknown", + extraction.status + ); + } + Ok(extraction) +} + #[cfg(test)] mod tests { use super::*; @@ -124,6 +148,23 @@ mod tests { .unwrap() .is_none() ); + std::fs::write(root.path().join("blob.rs"), b"fn helper() {}\0").unwrap(); + let binary = reextract_definition(root.path(), "blob.rs", "helper", "", 1) + .err() + .unwrap(); + assert!( + binary.to_string().starts_with("source_excluded"), + "{binary:#}" + ); + let cancelled = Extraction { + status: "cancelled".into(), + ..Extraction::default() + }; + let error = usable_extraction("lib.rs", cancelled).err().unwrap(); + assert!( + error.to_string().contains("extraction cancelled"), + "{error:#}" + ); let hit = found.hit(); assert_eq!(hit.provenance.as_deref(), Some("reextracted")); assert_eq!(hit.revision, Some(found.revision.to_string())); diff --git a/src/store.rs b/src/store.rs index 74f681f..e05cbf4 100644 --- a/src/store.rs +++ b/src/store.rs @@ -27,7 +27,7 @@ use anyhow::{Context, Result}; pub use paths::{decode_path, encode_path}; pub use read_access::is_index_warming; use rusqlite::Connection; -pub(crate) use scan::extraction_cache_version; +pub(crate) use scan::{MAX_SOURCE_BYTES, extraction_cache_version}; pub use seed::{SeedOutcome, ensure_seeded}; use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; diff --git a/src/store/scan.rs b/src/store/scan.rs index b5ad82d..d068cfc 100644 --- a/src/store/scan.rs +++ b/src/store/scan.rs @@ -4,7 +4,7 @@ use anyhow::Result; use rusqlite::{Connection, OptionalExtension, Transaction, params}; use std::path::Path; -const MAX_SOURCE_BYTES: u64 = 2 * 1024 * 1024; +pub(crate) const MAX_SOURCE_BYTES: u64 = 2 * 1024 * 1024; const MAX_FACTS: usize = 50_000; const EXTRACTION_CONTRACT_REVISION: u32 = 5; diff --git a/src/workspace/navigation.rs b/src/workspace/navigation.rs index 45c7568..4927854 100644 --- a/src/workspace/navigation.rs +++ b/src/workspace/navigation.rs @@ -49,13 +49,23 @@ pub(super) fn read( results: &WorkspaceResults, options: &Arguments, budget: &OutputBudget, +) -> Result { + let deadline = QueryDeadline::start(); + read_within(config, results, options, budget, deadline) + .map_err(|error| deadline.classify(error)) +} +fn read_within( + config: &WorkspaceConfig, + results: &WorkspaceResults, + options: &Arguments, + budget: &OutputBudget, + deadline: QueryDeadline, ) -> Result { let target = options .words .get(1) .context("usage: show TARGET | ctx HANDLE")?; let side = options.side.as_deref().map(SourceSide::parse).transpose()?; - let deadline = QueryDeadline::start(); let is_context = options.words[0] == "ctx"; let addressed = target.starts_with("read:") || target.parse::().is_ok() @@ -152,7 +162,10 @@ pub(super) fn history( let member = if let Some((set, index)) = &retained { let owner = &set.owners[set.hits[*index].owner]; verify_selection(owner, options)?; - let _ = owner.open(config, QueryDeadline::start())?; + let deadline = QueryDeadline::start(); + owner + .open(config, deadline) + .map_err(|error| deadline.classify(error))?; owner.member_root(config)? } else { selected_member(config, options)? diff --git a/src/workspace/retrieval.rs b/src/workspace/retrieval.rs index f0bd3a2..b5c3757 100644 --- a/src/workspace/retrieval.rs +++ b/src/workspace/retrieval.rs @@ -115,7 +115,7 @@ pub(super) fn search( options: &Arguments, context: &RequestContext, session: &mut SemanticSession, - deadline: QueryDeadline, + mut deadline: QueryDeadline, ) -> Result { let roots = config.member_roots(&options.root.canonicalize()?)?; let Scope { @@ -185,7 +185,7 @@ pub(super) fn search( member.verify_identity()?; coordinator::ensure_member(member, options)?; if options.no_daemon { - Store::open(&member.root, &member_cache)?.index()?; + deadline.pause_during(|| Store::open(&member.root, &member_cache)?.index())?; } let store = open_member_read(member, &member_cache, !options.no_daemon, deadline)?; let preparation_error = if semantic && !options.no_daemon { @@ -314,7 +314,11 @@ pub(super) fn search( } } ensure!( - !set.owners.is_empty() || !deadline.expired(), + !set.owners.is_empty() + || !set + .coverage + .iter() + .any(|member| member["state"] == "timed_out"), "{TIMED_OUT}: query deadline expired before any member answered" ); ensure!( diff --git a/src/workspace/tests.rs b/src/workspace/tests.rs index 1a46ed5..d2e2262 100644 --- a/src/workspace/tests.rs +++ b/src/workspace/tests.rs @@ -294,10 +294,11 @@ fn explicit_scopes_limit_members_and_unavailable_members_remain_visible() { ); } -#[test] -fn expired_query_deadline_answers_timed_out() { - let fixture = Fixture::new(); - fixture.json("engine", &["search", "sym:SharedThing", "ws:all"]); +/// Runs `search sym:SharedThing ws:all` (`--no-daemon`) from `engine` under `deadline`. +fn search_all_with_deadline( + fixture: &Fixture, + deadline: crate::daemon::deadline::QueryDeadline, +) -> Result { let args: Vec = [ "--workspace", fixture.config.to_str().unwrap(), @@ -317,19 +318,61 @@ fn expired_query_deadline_answers_timed_out() { let options = cli::parse(&args).unwrap(); let config = WorkspaceConfig::load(&fixture.config).unwrap(); let cache = cache_path(&config, Some(fixture.cache.path())).unwrap(); - let error = retrieval::search( + retrieval::search( &config, &cache, &WorkspaceResults::open(&cache).unwrap(), &options, &cli::request_context(&options), &mut crate::semantic::SemanticSession::default(), + deadline, + ) +} + +#[test] +fn expired_query_deadline_answers_timed_out() { + let fixture = Fixture::new(); + fixture.json("engine", &["search", "sym:SharedThing", "ws:all"]); + let error = search_all_with_deadline( + &fixture, crate::daemon::deadline::QueryDeadline::after(std::time::Duration::ZERO), ) .unwrap_err(); assert!(crate::daemon::deadline::is_timed_out(&error), "{error:#}"); } +#[test] +fn member_reconciles_do_not_spend_the_query_deadline() { + let fixture = Fixture::new(); + for member in ["engine", "pack", "upstream"] { + crate::daemon::deadline::write_slow_reconcile_fixture( + &fixture.root.path().join(member), + "unrelated", + ); + } + // Three members' reconciles together outlast the budget; their queries do not. + let budget = std::time::Duration::from_millis(750); + let started = std::time::Instant::now(); + let output = search_all_with_deadline( + &fixture, + crate::daemon::deadline::QueryDeadline::after(budget), + ) + .unwrap(); + assert!( + started.elapsed() > budget, + "reconciles finished within the budget" + ); + let page: Value = serde_json::from_str(&output).unwrap(); + let states: Vec<_> = page["coverage"] + .as_array() + .unwrap() + .iter() + .map(|member| member["state"].as_str().unwrap()) + .collect(); + assert_eq!(states, ["searched"; 3]); + assert_eq!(page["hits"].as_array().unwrap().len(), 3); +} + #[test] fn removed_and_replaced_members_cannot_retarget_retained_handles() { let fixture = Fixture::new();