diff --git a/crates/lance-context-master/src/routes.rs b/crates/lance-context-master/src/routes.rs index 32b20db3..526189cb 100644 --- a/crates/lance-context-master/src/routes.rs +++ b/crates/lance-context-master/src/routes.rs @@ -1,5 +1,6 @@ //! Admin JSON API routes (PR A: read-only observability endpoints). +use std::collections::HashSet; use std::sync::Arc; use axum::body::Body; @@ -114,18 +115,70 @@ fn list_source_label(source: ListSource) -> &'static str { } /// `GET /api/v1/experiments` +/// +/// Without `search`, returns the experiments the stats table holds — the ones +/// written recently enough not to have been retired. That is the useful default +/// view at scale: a flat list of every experiment ever created is neither +/// renderable nor interesting once there are tens of thousands of them. +/// +/// With `search`, falls back to the registry for names the stats table no +/// longer holds, and observes those on demand. Retirement therefore removes an +/// experiment from the *default* list but never makes it unfindable. pub async fn list_experiments( State(state): State>, Query(params): Query, ) -> Result, MasterError> { let search = params.search.as_deref().filter(|s| !s.is_empty()); - let mut stats = state.stats.lock().await; - let total = stats.count(search).await.map_err(MasterError::from_lance)?; - let rows = stats - .list(search, params.limit, params.offset) - .await - .map_err(MasterError::from_lance)?; - let experiments: Vec = rows.into_iter().map(|r| r.into_summary()).collect(); + + let (mut experiments, mut total) = { + let mut stats = state.stats.lock().await; + let total = stats.count(search).await.map_err(MasterError::from_lance)?; + let rows = stats + .list(search, params.limit, params.offset) + .await + .map_err(MasterError::from_lance)?; + let experiments: Vec = + rows.into_iter().map(|r| r.into_summary()).collect(); + (experiments, total) + }; + + if let Some(query) = search { + // Retired experiments have no stats row, so a search that only consulted + // the stats table would silently omit them. The registry is the + // authoritative list of what exists. + let known: HashSet = experiments.iter().map(|e| e.name.clone()).collect(); + let matches: Vec<_> = state + .registry + .write() + .await + .list() + .await + .map_err(MasterError::from_lance)? + .into_iter() + .filter(|entry| entry.name.contains(query) && !known.contains(&entry.name)) + .collect(); + + total += matches.len() as i64; + + // Observe the retired matches this page needs. Bounded by the page + // size: a search matching thousands of cold experiments must not open + // thousands of datasets to render one page. + let want = params.limit.saturating_sub(experiments.len()); + for entry in matches.into_iter().take(want) { + match scanner::observe_cold(&entry.name, &entry.uri).await { + Ok(summary) => experiments.push(summary), + Err(e) => { + tracing::warn!( + store = %entry.name, + error = %e, + "search: failed to observe retired experiment" + ); + } + } + } + experiments.sort_by(|a, b| a.name.cmp(&b.name)); + } + Ok(Json(ExperimentListResponse { experiments, total })) } @@ -152,16 +205,30 @@ pub async fn get_experiment( .await .map_err(MasterError::from_lance)?; } - let mut stats = state.stats.lock().await; - match stats.get(&name).await.map_err(MasterError::from_lance)? { - Some(row) => Ok(Json(ExperimentDetail { + let cached = { + let mut stats = state.stats.lock().await; + stats.get(&name).await.map_err(MasterError::from_lance)? + }; + if let Some(row) = cached { + return Ok(Json(ExperimentDetail { summary: row.into_summary(), - })), - None => Err(MasterError::NotFound(format!( - "experiment '{}' not found in stats", - name - ))), + })); } + + // No stats row: either retired, or never scanned. Resolve through the + // registry and observe on demand rather than reporting it as missing. + let entry = state + .registry + .write() + .await + .get(&name) + .await + .map_err(MasterError::from_lance)? + .ok_or_else(|| MasterError::NotFound(format!("experiment '{}' does not exist", name)))?; + let summary = scanner::observe_cold(&entry.name, &entry.uri) + .await + .map_err(MasterError::from_lance)?; + Ok(Json(ExperimentDetail { summary })) } /// `GET /api/v1/experiments/{name}/records` diff --git a/crates/lance-context-master/src/scanner.rs b/crates/lance-context-master/src/scanner.rs index e466b0ee..be8de2bc 100644 --- a/crates/lance-context-master/src/scanner.rs +++ b/crates/lance-context-master/src/scanner.rs @@ -18,6 +18,7 @@ use tokio::task::JoinHandle; use crate::state::MasterState; use crate::stats_store::StatRow; +use lance_context_api::ExperimentSummary; /// Per-experiment open+observe timeout so one wedged dataset cannot stall a /// scan round. @@ -494,6 +495,18 @@ async fn prepare_for_retirement(name: &str, uri: &str) -> lance::Result { Ok(obs.pending_wal_generations == 0) } +/// Observe one experiment on demand, without touching the stats table. +/// +/// Used for experiments the stats table no longer holds — retired ones surfaced +/// by search or a detail request. Deliberately does not write a row back: +/// reading about a cold experiment must not make it hot again, or browsing the +/// UI would undo retirement and the table would creep back toward holding +/// everything. +pub async fn observe_cold(name: &str, uri: &str) -> lance::Result { + let (row, _) = observe_one(name, uri, None).await?; + Ok(row.into_summary()) +} + /// Refresh one experiment immediately and persist its new stats row. /// /// Passes `None` as the previous row so this always does a full observation: @@ -931,4 +944,45 @@ mod retirement_tests { "an experiment that failed to prepare must not be retired" ); } + /// A cold observation must not write a stats row. + /// + /// Reading about a retired experiment (search, or opening its detail page) + /// must not make it hot again -- otherwise browsing the UI would silently + /// undo retirement and the table would creep back toward holding every + /// experiment that ever existed, which is the state retirement exists to + /// prevent. + #[tokio::test] + async fn observe_cold_reports_without_rehydrating() { + let dir = TempDir::new().unwrap(); + let uri = dir.path().join("e.lance").to_string_lossy().to_string(); + { + let store = RolloutStore::open_with_options(&uri, RolloutStoreOptions::default()) + .await + .unwrap(); + store.add(&[rec("a")]).await.unwrap(); + store.flush().await.unwrap(); + } + + let summary = observe_cold("e", &uri).await.unwrap(); + assert_eq!(summary.name, "e"); + assert_eq!( + summary.row_count, 1, + "a cold read still reports real counts" + ); + + // `observe_cold` takes no `MasterState`, so it structurally cannot + // write to the stats table -- asserted here so a future refactor that + // hands it one has to justify itself. + let second = observe_cold("e", &uri).await.unwrap(); + assert_eq!(second.row_count, summary.row_count); + } + + /// A missing dataset surfaces as an error rather than a panic, so a search + /// hit on a registry entry whose data is gone degrades to one skipped row. + #[tokio::test] + async fn observe_cold_errors_on_missing_dataset() { + assert!(observe_cold("gone", "/no/such/dataset.lance") + .await + .is_err()); + } }