From 2a61c66abfdcf3b3180c3d3aa67147ffe1817b35 Mon Sep 17 00:00:00 2001 From: Beinan Date: Mon, 27 Jul 2026 09:37:07 +0000 Subject: [PATCH] feat(master): serve the experiment list as recently-active plus search Stacked on #210. Retirement (#210) removes cold experiments from the stats table, which is what keeps a scan round proportional to active work. On its own it also removes them from `GET /experiments`, which is only acceptable once they remain findable another way. This closes that. default list the stats table, i.e. experiments written recently enough not to have been retired. A flat list of every experiment ever created is neither renderable nor interesting at tens of thousands. search the stats table, plus the registry for names it no longer holds. Cold matches are observed on demand. detail on a stats miss, resolve through the registry and observe on demand rather than returning 404. So retirement removes an experiment from the *default* view and never makes it unfindable. `observe_cold` deliberately does not write a stats row. Reading about a retired experiment must not make it hot again, or browsing the UI would silently undo retirement and the table would creep back toward holding everything -- the state retirement exists to prevent. It also takes no `MasterState`, so it structurally cannot write one; a refactor that hands it one has to justify itself. Search bounds its on-demand observations by the page size. A query matching thousands of cold experiments must not open thousands of datasets to render one page. Known gap: the two handlers changed here live in `routes.rs`, whose tests all require etcd and are `#[ignore]`d, so CI does not cover them. `observe_cold` is tested in `scanner.rs`, which CI does run. Worth fixing, but as its own change rather than a fourth concern in this one. Co-Authored-By: Claude --- crates/lance-context-master/src/routes.rs | 97 ++++++++++++++++++---- crates/lance-context-master/src/scanner.rs | 54 ++++++++++++ 2 files changed, 136 insertions(+), 15 deletions(-) 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()); + } }