diff --git a/Cargo.lock b/Cargo.lock index 81a5e9b70..b38df0c24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5672,6 +5672,7 @@ dependencies = [ "time", "tokio", "tracing", + "tracing-subscriber", "ulid", "yaml_serde", ] diff --git a/changelog.d/full-text-rebuild-change-grant.deprecated.md b/changelog.d/full-text-rebuild-change-grant.deprecated.md new file mode 100644 index 000000000..b17d94953 --- /dev/null +++ b/changelog.d/full-text-rebuild-change-grant.deprecated.md @@ -0,0 +1,6 @@ +- The full-text rebuild accepts `maintain` or `change` on its branch in this + release; a later release accepts `maintain` only. Grant `maintain` to the + accounts that rebuild full-text indexes. See + [rebuild full-text indexes][full-text-rebuild-change-grant]. + +[full-text-rebuild-change-grant]: ../docs/user/operations/maintenance.md#rebuild-full-text-indexes diff --git a/changelog.d/lease-beat-failures-logged.added.md b/changelog.d/lease-beat-failures-logged.added.md new file mode 100644 index 000000000..d58985546 --- /dev/null +++ b/changelog.d/lease-beat-failures-logged.added.md @@ -0,0 +1,7 @@ +- **Failed cluster lease heartbeats are logged.** A server logs every + heartbeat request that fails, every failed check for a successor's claim and + a heartbeat left unanswered at its deadline, each with the failure's class and + storage kind and before any fence. See the [storage failure + logs][lease-beat-logs]. + +[lease-beat-logs]: ../docs/user/operations/storage-failures.md#logs diff --git a/changelog.d/maintenance-budgets.changed.md b/changelog.d/maintenance-budgets.changed.md new file mode 100644 index 000000000..dc1707a7a --- /dev/null +++ b/changelog.d/maintenance-budgets.changed.md @@ -0,0 +1,10 @@ +- **Maintenance runs one table at a time under declared memory caps.** + Optimize, cleanup, index builds and the full-text rebuild now run one at a + time per graph in a process, and a run rewrites or indexes one table at a + time by default (`OMNIGRAPH_MAINTENANCE_CONCURRENCY`, which defaulted to 8, + is now capped at 2). Compaction scans queue at most 64 MiB in Lance's I/O + buffer and full-text builds use 512 MiB over two workers; each has its own + variable. Expect a direct `optimize` on a graph with many tables to take + longer. See [maintenance budgets][maintenance-budgets]. + +[maintenance-budgets]: ../docs/user/operations/maintenance.md#budgets diff --git a/changelog.d/maintenance-entry-points-take-an-actor.breaking.md b/changelog.d/maintenance-entry-points-take-an-actor.breaking.md new file mode 100644 index 000000000..d6f96b6fc --- /dev/null +++ b/changelog.d/maintenance-entry-points-take-an-actor.breaking.md @@ -0,0 +1,12 @@ +- Rust callers of the engine's maintenance methods move to their actor-taking + forms: `optimize_as(actor)`, `cleanup_as(options, actor)`, + `ensure_indices_as(actor)`, `ensure_indices_on_as(branch, actor)` and + `rebuild_full_text_indices_on_as(branch, actor)`; pass `None` where no policy + is installed. `repair` is now the read-only `repair_report_as(actor)`: + `RepairOptions`, the `healed` and `forced` actions and + `RepairStats::graph_manifest_version` are gone, and `repair --json` no longer + prints the always-null `graph_manifest_version`. An exhaustive match on + `PolicyAction` adds arms for `Maintain` and `Reclaim`. See + [repair][maintenance-entry-points-repair]. + +[maintenance-entry-points-repair]: ../docs/user/operations/maintenance.md#repair diff --git a/changelog.d/maintenance-failures-are-definite.changed.md b/changelog.d/maintenance-failures-are-definite.changed.md new file mode 100644 index 000000000..8ebdc85d6 --- /dev/null +++ b/changelog.d/maintenance-failures-are-definite.changed.md @@ -0,0 +1,8 @@ +- **A maintenance failure before its publication says the graph did not + move.** Optimize, cleanup, index builds, the full-text rebuild and the repair + report mark every such failure, so the direct CLI exits `1` (no effects) for + a storage or internal failure where it exited `2` (outcome unknown), and a + host serving the graph keeps serving through one. A publication in doubt is + still reported as one. See the [write path][maintenance-failure-evidence]. + +[maintenance-failure-evidence]: ../docs/dev/writes.md#policy-and-attribution diff --git a/changelog.d/maintenance-policy-actions.added.md b/changelog.d/maintenance-policy-actions.added.md new file mode 100644 index 000000000..d7698ffce --- /dev/null +++ b/changelog.d/maintenance-policy-actions.added.md @@ -0,0 +1,10 @@ +- **Maintenance has its own Cedar actions.** `maintain` covers optimize, index + builds, the full-text rebuild and the repair report on the branch they + maintain, and `reclaim` covers cleanup with no branch scope; `change` implies + neither, so automation can compact a graph without the right to write data or + delete history. An embedded host that installs a policy checker enforces them + on every maintenance entry point, optimize and the index writers record the + requesting actor on their commit, and existing policies grant neither action. + See [policy actions][maintenance-policy-actions]. + +[maintenance-policy-actions]: ../docs/user/operations/policy.md#actions diff --git a/crates/omnigraph-api-types/src/lib.rs b/crates/omnigraph-api-types/src/lib.rs index f07f23307..ebe8b3a1c 100644 --- a/crates/omnigraph-api-types/src/lib.rs +++ b/crates/omnigraph-api-types/src/lib.rs @@ -2013,6 +2013,9 @@ fn access_slot(action: omnigraph_policy::PolicyAction) -> Option { P::InvokeQuery => Some(AccessSlot::GraphWide), // Server- or operator-scoped: no graph's access report names them. P::Admin | P::GraphList | P::ConfigManage => None, + // Maintenance runs only through the direct CLI and embedded callers; + // no served operation performs it, so the report does not name it. + P::Maintain | P::Reclaim => None, } } diff --git a/crates/omnigraph-bench/src/dataset_worker.rs b/crates/omnigraph-bench/src/dataset_worker.rs index f66ae24af..3999d6e98 100644 --- a/crates/omnigraph-bench/src/dataset_worker.rs +++ b/crates/omnigraph-bench/src/dataset_worker.rs @@ -803,7 +803,10 @@ pub(crate) async fn build_dataset( Engine::V2, )?; if plan.needs_indices { - session.ensure_indices().await.map_err(|e| e.to_string())?; + session + .ensure_indices_as(None) + .await + .map_err(|e| e.to_string())?; } (session, case, Some(exact)) } diff --git a/crates/omnigraph-bench/src/legacy/branch_merge_oracle.rs b/crates/omnigraph-bench/src/legacy/branch_merge_oracle.rs index b5151f265..78466e2be 100644 --- a/crates/omnigraph-bench/src/legacy/branch_merge_oracle.rs +++ b/crates/omnigraph-bench/src/legacy/branch_merge_oracle.rs @@ -28,7 +28,7 @@ pub async fn initialize_local_fixture( let optimized_user_tables = match plan.compaction_recency { CompactionRecency::Optimized => { let outcomes = db - .optimize() + .optimize_as(None) .await .map_err(|error| fixture_error(format!("optimize fixture main branch: {error}")))?; let intended_keys = (0..plan.node_tables()) diff --git a/crates/omnigraph-cli/src/commands/maintenance/mod.rs b/crates/omnigraph-cli/src/commands/maintenance/mod.rs index a6557054c..f39beb150 100644 --- a/crates/omnigraph-cli/src/commands/maintenance/mod.rs +++ b/crates/omnigraph-cli/src/commands/maintenance/mod.rs @@ -3,7 +3,7 @@ use std::io::Write; use color_eyre::eyre::{Result, bail}; -use omnigraph::db::{CleanupPolicyOptions, Omnigraph, RepairAction, RepairOptions}; +use omnigraph::db::{CleanupPolicyOptions, Omnigraph, RepairAction}; use crate::app::Completion; use crate::helpers::{confirm_destructive, parse_duration_arg, resolve_actor}; @@ -42,13 +42,14 @@ pub(crate) async fn run_optimize( let OptimizeArgs { json } = args; let uri = crate::scope::direct_graph_uri(scope.target, scope.graph).await?; let binding = crate::scope::capture_storage_binding(scope.target)?.for_uri(&uri)?; + let actor = resolve_actor(scope.operator, scope.actor, stderr)?; crate::admission::ensure_graph_with_binding(&binding, stderr).await?; let db = Omnigraph::open_with_storage( binding.uri(), omnigraph::storage::storage_for_binding(&binding)?, ) .await?; - let stats = db.optimize().await?; + let stats = db.optimize_as(actor.as_deref()).await?; write_optimize(&uri, &stats, json, stdout)?; Ok(Completion::success()) } @@ -93,13 +94,14 @@ pub(crate) async fn run_repair( let RepairArgs { json } = args; let uri = crate::scope::direct_graph_uri(scope.target, scope.graph).await?; let binding = crate::scope::capture_storage_binding(scope.target)?.for_uri(&uri)?; + let actor = resolve_actor(scope.operator, scope.actor, stderr)?; crate::admission::ensure_graph_with_binding(&binding, stderr).await?; let db = Omnigraph::open_with_storage( binding.uri(), omnigraph::storage::storage_for_binding(&binding)?, ) .await?; - let stats = db.repair(RepairOptions::default()).await?; + let stats = db.repair_report_as(actor.as_deref()).await?; let refused_count = stats .datasets .iter() @@ -159,13 +161,14 @@ pub(crate) async fn run_cleanup( keep_versions: keep, older_than: older_than_dur, }; + let actor = resolve_actor(scope.operator, scope.actor, stderr)?; crate::admission::ensure_graph_with_binding(&binding, stderr).await?; let db = Omnigraph::open_with_storage( binding.uri(), omnigraph::storage::storage_for_binding(&binding)?, ) .await?; - let stats = db.cleanup(options.clone()).await?; + let stats = db.cleanup_as(options.clone(), actor.as_deref()).await?; write_cleanup(&uri, &options, &policy_desc, &stats, json, stdout)?; Ok(Completion::success()) } diff --git a/crates/omnigraph-cli/src/output.rs b/crates/omnigraph-cli/src/output.rs index aa8280127..6455bd7d5 100644 --- a/crates/omnigraph-cli/src/output.rs +++ b/crates/omnigraph-cli/src/output.rs @@ -133,7 +133,6 @@ pub(crate) fn write_repair( if json { let value = serde_json::json!({ "uri": uri, - "graph_manifest_version": stats.graph_manifest_version, "datasets": stats.datasets.iter().map(|s| serde_json::json!({ "type_key": s.type_key, "published_dataset_version": s.published_dataset_version, @@ -1867,7 +1866,7 @@ mod tests { #[tokio::test] async fn maintenance_optimize_and_repair_reports_preserve_all_result_fields() { use omnigraph::db::{ - Omnigraph, PendingIndex, RepairAction, RepairClassification, RepairOptions, SkipReason, + Omnigraph, PendingIndex, RepairAction, RepairClassification, SkipReason, }; // These engine result types are non_exhaustive. Obtain one valid value // from a tiny graph, then set its public presentation fields; no engine @@ -1881,7 +1880,7 @@ mod tests { let db = Omnigraph::init(&uri, "node Person {\n slug: String @key\n}\n") .await .unwrap(); - let mut compacted = db.optimize().await.unwrap().pop().unwrap(); + let mut compacted = db.optimize_as(None).await.unwrap().pop().unwrap(); compacted.type_key = "node:Person".into(); compacted.fragments_removed = 3; compacted.fragments_added = 1; @@ -1933,7 +1932,7 @@ mod tests { |json, writer| super::write_optimize("graph", &[], json, writer), ); - let mut repair = db.repair(RepairOptions::default()).await.unwrap(); + let mut repair = db.repair_report_as(None).await.unwrap(); let mut clean = repair.datasets.pop().unwrap(); clean.type_key = "node:Person".into(); clean.published_dataset_version = 4; @@ -1950,7 +1949,6 @@ mod tests { refused.operations = vec!["Append".into(), "Delete".into()]; refused.error = Some("foreign write".into()); repair.datasets = vec![clean, refused]; - repair.graph_manifest_version = Some(9); assert_maintenance_report( concat!( "repair graph — 2 datasets\n", @@ -1958,7 +1956,7 @@ mod tests { " edge type 'Knows' refused foreign_drift 4 → 6 [Append, Delete] (foreign write)\n", ), serde_json::json!({ - "uri":"graph", "graph_manifest_version":9, + "uri":"graph", "datasets":[ {"type_key":"node:Person","published_dataset_version":4,"lance_head_version":4,"classification":"no_drift","action":"no_op","operations":[],"error":null}, {"type_key":"edge:Knows","published_dataset_version":4,"lance_head_version":6,"classification":"foreign_drift","action":"refused","operations":["Append","Delete"],"error":"foreign write"}, @@ -1967,10 +1965,9 @@ mod tests { |json, writer| super::write_repair("graph", &repair, json, writer), ); repair.datasets.clear(); - repair.graph_manifest_version = None; assert_maintenance_report( "repair graph — 0 datasets\n", - serde_json::json!({"uri":"graph","graph_manifest_version":null,"datasets":[]}), + serde_json::json!({"uri":"graph","datasets":[]}), |json, writer| super::write_repair("graph", &repair, json, writer), ); } diff --git a/crates/omnigraph-cli/tests/integration/cli_data.rs b/crates/omnigraph-cli/tests/integration/cli_data.rs index 939fe3de9..9d110089a 100644 --- a/crates/omnigraph-cli/tests/integration/cli_data.rs +++ b/crates/omnigraph-cli/tests/integration/cli_data.rs @@ -1211,7 +1211,8 @@ fn repair_json_reports_noop_on_clean_graph() { assert!(payload.get("confirm").is_none()); assert!(payload.get("force").is_none()); - assert_eq!(payload["graph_manifest_version"], Value::Null); + // The report publishes nothing, so it names no graph manifest version. + assert!(payload.get("graph_manifest_version").is_none()); assert!(payload.get("manifest_version").is_none()); assert!(payload.get("tables").is_none()); let datasets = payload["datasets"].as_array().unwrap(); @@ -1282,7 +1283,7 @@ fn repair_reports_foreign_drift_and_refuses_removed_flags() { .arg(&graph), ); let payload: Value = serde_json::from_slice(&output.stdout).unwrap(); - assert_eq!(payload["graph_manifest_version"], Value::Null); + assert!(payload.get("graph_manifest_version").is_none()); let person = payload["datasets"] .as_array() .unwrap() diff --git a/crates/omnigraph-cluster/Cargo.toml b/crates/omnigraph-cluster/Cargo.toml index 43fffaedc..53f8469f3 100644 --- a/crates/omnigraph-cluster/Cargo.toml +++ b/crates/omnigraph-cluster/Cargo.toml @@ -42,6 +42,7 @@ serial_test = "3" tempfile = { workspace = true } # Paused time for the lease tests, so the crate's own test build runs them. tokio = { workspace = true, features = ["test-util"] } +tracing-subscriber = { workspace = true } [lints] workspace = true diff --git a/crates/omnigraph-cluster/src/lease.rs b/crates/omnigraph-cluster/src/lease.rs index e21b61d25..9cd217b91 100644 --- a/crates/omnigraph-cluster/src/lease.rs +++ b/crates/omnigraph-cluster/src/lease.rs @@ -56,7 +56,7 @@ use std::sync::Arc; use std::time::Duration; -use omnigraph_storage::{StorageAdapter, StorageIoScope}; +use omnigraph_storage::{StorageAdapter, StorageError, StorageFailureKind, StorageIoScope}; use tokio::sync::{Notify, watch}; use tokio::time::Instant; @@ -217,6 +217,7 @@ impl LeaseKeeper { biased; fence = beat.first(seconds) => fence, () = tokio::time::sleep_until(lock_sent + owner_deadline(seconds)) => { + log_unanswered_beat(0); Some(LeaseFenced::Lapsed) } }; @@ -232,6 +233,7 @@ impl LeaseKeeper { // hangs must not postpone the owner's fence. A beat that // completes at the same moment is counted first. let deadline = beat.acknowledged + owner_deadline(seconds); + let seq = beat.seq; let pause = if beat.answered { beat_interval(seconds) } else { @@ -247,7 +249,10 @@ impl LeaseKeeper { fence = next => if let Some(fence) = fence { break fence; }, - () = tokio::time::sleep_until(deadline) => break LeaseFenced::Lapsed, + () = tokio::time::sleep_until(deadline) => { + log_unanswered_beat(seq); + break LeaseFenced::Lapsed; + } } }; fence_holder(scope.as_ref(), fence); @@ -302,6 +307,28 @@ struct Beat { answered: bool, } +/// The class of a failed lease request, which a log may carry: its error's +/// variant and storage failure kind, never its message, which can name the +/// root's object URIs. +fn failure_class(error: &StorageError) -> (&'static str, Option) { + match error { + StorageError::Internal(_) => ("internal", None), + StorageError::Backend(failure) => ("backend", Some(failure.kind)), + StorageError::Io { failure, .. } => ("io", Some(failure.kind)), + StorageError::ResourceLimit { .. } => ("resource_limit", None), + } +} + +/// Log a beat that got no answer before the owner's deadline, before the +/// fence that follows it. +fn log_unanswered_beat(seq: u64) { + tracing::warn!( + seq, + class = "unanswered", + "cluster lease beat got no answer before the owner's deadline" + ); +} + /// Stop the holder's storage sends, then report the fence. A successor may /// claim the lock five sixths of a lease after the owner's deadline, which /// can be sooner than the holder's process exits. Fencing the scope here @@ -341,7 +368,17 @@ impl Beat { .await { Ok(Some(_)) => Some(LeaseFenced::Superseded), - _ => None, + Ok(None) => None, + Err(error) => { + let (class, kind) = failure_class(&error); + tracing::warn!( + seq = self.seq, + class, + ?kind, + "cluster lease claim read failed; the beat is sent anyway" + ); + None + } } } @@ -371,7 +408,16 @@ impl Beat { self.seq += 1; self.first_sent = None; } - Err(_) => self.answered = false, + Err(error) => { + let (class, kind) = failure_class(&error); + tracing::warn!( + seq = self.seq, + class, + ?kind, + "cluster lease beat failed; it is sent again" + ); + self.answered = false; + } } None } @@ -599,6 +645,35 @@ mod tests { } } + /// The text a test's log lines were written as. + #[derive(Clone, Default)] + struct Logged(Arc>>); + + impl Logged { + fn text(&self) -> String { + String::from_utf8(self.0.lock().unwrap().clone()).unwrap() + } + } + + impl std::io::Write for Logged { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + impl<'a> tracing_subscriber::fmt::MakeWriter<'a> for Logged { + type Writer = Logged; + + fn make_writer(&'a self) -> Self::Writer { + self.clone() + } + } + async fn fenced_within(keeper: &LeaseKeeper, limit: Duration) -> Option { tokio::time::timeout(limit, LeaseFence(Some(keeper.fenced())).fenced()) .await @@ -687,6 +762,54 @@ mod tests { assert_eq!(store.failures.load(Ordering::SeqCst), 0); } + /// Every failed beat logs its class, and a beat left unanswered at the + /// owner's deadline logs before the fence it causes. No line carries a + /// failure's message, which can name the root's object URIs. + #[tokio::test(start_paused = true)] + async fn failed_and_unanswered_beats_log_their_class_before_the_fence() { + let logged = Logged::default(); + let subscriber = tracing_subscriber::fmt() + .with_writer(logged.clone()) + .with_ansi(false) + .finish(); + let _default = tracing::subscriber::set_default(subscriber); + let store = Beats::new(); + let keeper = start(&store, "owner").await; + + store.failures.store(1, Ordering::SeqCst); + assert_eq!(fenced_within(&keeper, lease(SECONDS) * 2).await, None); + let text = logged.text(); + let failed = text + .lines() + .find(|line| line.contains("cluster lease beat failed; it is sent again")) + .unwrap_or_else(|| panic!("a failed beat is logged: {text}")); + assert!( + failed.contains("backend") && failed.contains("Transient"), + "the line names the failure's class and kind: {failed}" + ); + assert!( + !text.contains("beat request unanswered"), + "no line carries the failure's message: {text}" + ); + + store.hang.store(true, Ordering::SeqCst); + assert_eq!( + fenced_within(&keeper, lease(SECONDS) * 2).await, + Some(LeaseFenced::Lapsed) + ); + let text = logged.text(); + let unanswered = text + .find("cluster lease beat got no answer before the owner's deadline") + .unwrap_or_else(|| panic!("an unanswered beat is logged: {text}")); + let fenced = text + .find("cluster admission lease fenced") + .unwrap_or_else(|| panic!("the fence is logged: {text}")); + assert!( + unanswered < fenced, + "the beat is logged before the fence: {text}" + ); + } + /// A beat the store applied but whose answer was lost is the owner's own: /// the owner sends it again soon, finds it present, and counts it from /// its first send. Lost answers never fence a live owner, and a diff --git a/crates/omnigraph-core/src/dst_clock.rs b/crates/omnigraph-core/src/dst_clock.rs index 7372b18c8..5c77bad17 100644 --- a/crates/omnigraph-core/src/dst_clock.rs +++ b/crates/omnigraph-core/src/dst_clock.rs @@ -62,6 +62,21 @@ fn next_logical_ms() -> Option { CLOCK.with(|clock| clock.now_ms()) } +/// Whether this thread reads an installed logical clock, as a harness +/// actor thread does. Without the `dst` feature: never. +#[cfg(not(feature = "dst"))] +#[inline(always)] +pub fn installed() -> bool { + false +} + +/// Whether this thread reads an installed logical clock, as a harness +/// actor thread does. +#[cfg(feature = "dst")] +pub fn installed() -> bool { + CLOCK.with(|_| ()).is_some() +} + /// Seam for `chrono::Utc::now()` call sites. Without the `dst` feature: /// the real clock, directly. #[cfg(not(feature = "dst"))] diff --git a/crates/omnigraph-core/src/dst_ids.rs b/crates/omnigraph-core/src/dst_ids.rs index 3c9dfe233..ec31070fc 100644 --- a/crates/omnigraph-core/src/dst_ids.rs +++ b/crates/omnigraph-core/src/dst_ids.rs @@ -75,6 +75,21 @@ omnigraph_seams::thread_local_seam! { pub static IDS: dyn IdSource = ("ids", Op::Unreachable); } +/// Whether this thread mints from installed seeded identities, as a +/// harness actor thread does. Without the `dst` feature: never. +#[cfg(not(feature = "dst"))] +#[inline(always)] +pub fn installed() -> bool { + false +} + +/// Whether this thread mints from installed seeded identities, as a +/// harness actor thread does. +#[cfg(feature = "dst")] +pub fn installed() -> bool { + IDS.with(|_| ()).is_some() +} + /// Every production identity mint in this crate comes through here. /// Without the `dst` feature this is a direct `Ulid::new()`: no /// thread-local probe, no override authority anywhere in the build. diff --git a/crates/omnigraph-core/src/error.rs b/crates/omnigraph-core/src/error.rs index 55b0e861c..5aa11332f 100644 --- a/crates/omnigraph-core/src/error.rs +++ b/crates/omnigraph-core/src/error.rs @@ -510,6 +510,14 @@ pub enum OmniError { "graph initialization at '{uri}' is claimed by '__init_claim.json'; another initializer may still be running or a prior initializer may have stopped; quiesce all initializers before manually removing the claim, then retry init (use --force only when orphan schema files remain)" )] InitializationClaimed { uri: String }, + /// A maintenance run (optimize, cleanup, an index build or the full-text + /// rebuild) stopped at a cancel point because its graph's maintenance + /// lane was cancelled, as a server does on shutdown or a deployment + /// drain. The run carries completion evidence for where it stopped: a run + /// cancelled before its publication left only unreachable staging, which + /// a later cleanup reclaims. + #[error("maintenance {operation} was cancelled")] + MaintenanceCancelled { operation: &'static str }, } /// Cloneable, closed payload for engine-domain failures that may cross a @@ -677,6 +685,18 @@ impl OmniError { self.with_completion_evidence(CompletionEvidence::BeforeEffect) } + /// Mark a failure before the owning operation's first durable effect + /// when, as for [`Self::unpublished`], its type alone leaves open + /// whether the graph moved. A typed outcome stays as it is, so callers + /// matching on it see the same shape, and an error that already carries + /// evidence keeps it. + pub fn before_effect_if_opaque(self) -> Self { + if !self.outcome_opaque() || self.completion_evidence().is_some() { + return self; + } + self.with_completion_evidence(CompletionEvidence::BeforeEffect) + } + /// Mark a failure between the first durable effect and the publisher /// call: nothing is graph-visible. Only an opaque failure needs the /// mark, one whose type alone leaves open whether the graph moved @@ -1585,6 +1605,60 @@ mod tests { ); } + /// The pre-effect mark a maintenance run puts on its failures before its + /// first durable effect: opaque failures get `BeforeEffect`, a typed + /// outcome keeps its shape, and existing evidence, uncertainty included, + /// is never replaced. + #[test] + fn before_effect_if_opaque_marks_only_opaque_failures() { + for opaque in [ + OmniError::Storage(StorageFailure::new( + StorageFailureKind::Transient, + "storage: manifest read failed", + )), + OmniError::Io(std::io::Error::other("listing failed")), + OmniError::DataFusion("execution failure".into()), + OmniError::manifest_internal("coordinator view is stale"), + ] { + let message = opaque.to_string(); + let marked = opaque.before_effect_if_opaque(); + assert_eq!( + marked.completion_evidence(), + Some(CompletionEvidence::BeforeEffect), + "{message}" + ); + assert_eq!(marked.to_string(), message, "the mark is transparent"); + } + for typed in [ + OmniError::manifest("bad request"), + OmniError::manifest_conflict("authority conflict"), + OmniError::Policy("denied".into()), + OmniError::MaintenanceCancelled { + operation: "optimize", + }, + ] { + let message = typed.to_string(); + let marked = typed.before_effect_if_opaque(); + assert_eq!(marked.completion_evidence(), None, "{message}"); + assert!(!matches!(marked, OmniError::Completion { .. }), "{message}"); + } + assert_eq!( + OmniError::Io(std::io::Error::other("staged")) + .unpublished() + .before_effect_if_opaque() + .completion_evidence(), + Some(CompletionEvidence::Unpublished), + "an inner mark stays" + ); + assert_eq!( + OmniError::manifest_publish_in_doubt("acknowledgement lost") + .before_effect_if_opaque() + .completion_evidence(), + Some(CompletionEvidence::Uncertain), + "a publication in doubt stays uncertain" + ); + } + /// A step that fails after an earlier step of the same operation /// published says so: the mark outlives the step's own definite proofs /// and keeps them beside it, keeps the typed cause, and never hides diff --git a/crates/omnigraph-dst/src/concurrent.rs b/crates/omnigraph-dst/src/concurrent.rs index a05a5274a..98fc5f188 100644 --- a/crates/omnigraph-dst/src/concurrent.rs +++ b/crates/omnigraph-dst/src/concurrent.rs @@ -1753,12 +1753,15 @@ fn maintenance_life( let mut occ_retries = 0usize; loop { let result = match kind { - 0 => Box::pin(db.ensure_indices()).await.map(|_| ()), - 1 => Box::pin(db.optimize()).await.map(|_| ()), - _ => Box::pin(db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - })) + 0 => Box::pin(db.ensure_indices_as(None)).await.map(|_| ()), + 1 => Box::pin(db.optimize_as(None)).await.map(|_| ()), + _ => Box::pin(db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + )) .await .map(|_| ()), }; diff --git a/crates/omnigraph-dst/src/harness.rs b/crates/omnigraph-dst/src/harness.rs index 109c1df1c..9f4e65730 100644 --- a/crates/omnigraph-dst/src/harness.rs +++ b/crates/omnigraph-dst/src/harness.rs @@ -1634,12 +1634,15 @@ async fn exec_op(db: &Session, branch: &str, op: &Op) -> OmniResult<()> { ) .await .map(|_| ()), - Op::Optimize => Box::pin(db.optimize()).await.map(|_| ()), + Op::Optimize => Box::pin(db.optimize_as(None)).await.map(|_| ()), Op::Cleanup => { - let stats = Box::pin(db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - })) + let stats = Box::pin(db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + )) .await?; let deferred: BTreeMap = stats .iter() @@ -1655,7 +1658,7 @@ async fn exec_op(db: &Session, branch: &str, op: &Op) -> OmniResult<()> { .insert(db.uri().to_string(), deferred); Ok(()) } - Op::EnsureIndices => Box::pin(db.ensure_indices()).await.map(|_| ()), + Op::EnsureIndices => Box::pin(db.ensure_indices_as(None)).await.map(|_| ()), Op::SchemaAddProperty { count } => Box::pin(db.apply_schema(&schema_with_extras(*count))) .await .map(|_| ()), @@ -6432,7 +6435,7 @@ impl UniverseScenario for Scenario { tokio::time::resume(); let lively = tokio::time::timeout( std::time::Duration::from_secs(120), - Box::pin(db.ensure_indices()), + Box::pin(db.ensure_indices_as(None)), ) .await; tokio::time::pause(); diff --git a/crates/omnigraph-dst/tests/scenarios.rs b/crates/omnigraph-dst/tests/scenarios.rs index 8266b52b3..c0cc84306 100644 --- a/crates/omnigraph-dst/tests/scenarios.rs +++ b/crates/omnigraph-dst/tests/scenarios.rs @@ -1045,7 +1045,7 @@ async fn dst_memory_graph_end_to_end() { db.load_jsonl(TEST_DATA, LoadMode::Overwrite) .await .expect("load test data"); - db.ensure_indices().await.expect("ensure indices"); + db.ensure_indices_as(None).await.expect("ensure indices"); let persons = count_rows(&db, "node:Person").await; assert!(persons > 0, "loaded graph must have Person rows"); @@ -3668,7 +3668,7 @@ fn reborn_branch_cache_poison_body() { .expect("op1"); db.branch_create("b0").await.expect("op2"); world_read(&db).await; // ARMING READ 1 (the universe's op-2 verify) - db.optimize().await.expect("op3"); + db.optimize_as(None).await.expect("op3"); m!( db, "b0", @@ -3682,7 +3682,7 @@ fn reborn_branch_cache_poison_body() { Box::pin(db.branch_merge("b0", "main")) .await .expect("op6 merge"); - db.ensure_indices().await.expect("op7"); + db.ensure_indices_as(None).await.expect("op7"); db.branch_delete("b0").await.expect("op8"); m!( db, @@ -3716,7 +3716,7 @@ fn reborn_branch_cache_poison_body() { &[] ) .expect("op12"); - db.ensure_indices().await.expect("op13"); + db.ensure_indices_as(None).await.expect("op13"); m!( db, "main", @@ -3757,10 +3757,13 @@ fn reborn_branch_cache_poison_body() { &[("$age", 20)] ) .expect("op20"); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .expect("op21"); m!( @@ -3779,10 +3782,13 @@ fn reborn_branch_cache_poison_body() { &[("$age", 14), ("$ver", 8)] ) .expect("op23"); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .expect("op24"); db.branch_create("b0").await.expect("op25"); @@ -5197,7 +5203,7 @@ fn dst_classa_ablation_matrix_body() { db.branch_create("b0").await.expect("bc b0 life1"); } if o { - db.optimize().await.expect("optimize"); + db.optimize_as(None).await.expect("optimize"); } if r { upsert(&db, "b0", "w6", 45, 2).await; @@ -5210,20 +5216,20 @@ fn dst_classa_ablation_matrix_body() { upsert(&db, "main", "w1", 19, 3).await; upsert(&db, "main", "w7", 7, 6).await; if c { - db.cleanup(omnigraph::db::CleanupPolicyOptions { + db.cleanup_as(omnigraph::db::CleanupPolicyOptions { keep_versions: Some(1), older_than: None, - }) + }, None) .await .expect("cleanup 1"); } upsert(&db, "main", "w7", 17, 7).await; upsert(&db, "main", "w5", 14, 8).await; if c { - db.cleanup(omnigraph::db::CleanupPolicyOptions { + db.cleanup_as(omnigraph::db::CleanupPolicyOptions { keep_versions: Some(1), older_than: None, - }) + }, None) .await .expect("cleanup 2"); } @@ -5284,26 +5290,26 @@ fn dst_classa_ablation_matrix_body() { m!(db, "main", "set_age_v", &[("$name", "w5")], &[("$age", 70), ("$ver", 1)]).expect("op0"); m!(db, "main", "insert_person", &[("$name", "w6")], &[("$age", 7)]).expect("op1"); db.branch_create("b0").await.expect("op2"); - db.optimize().await.expect("op3"); + db.optimize_as(None).await.expect("op3"); m!(db, "b0", "set_age_v", &[("$name", "w6")], &[("$age", 45), ("$ver", 2)]).expect("op4"); m!(db, "b0", "remove_friendships_from", &[("$from", "w6")], &[]).expect("op5"); Box::pin(db.branch_merge("b0", "main")).await.expect("op6 merge"); - db.ensure_indices().await.expect("op7"); + db.ensure_indices_as(None).await.expect("op7"); db.branch_delete("b0").await.expect("op8"); m!(db, "main", "remove_friendships_from", &[("$from", "w2")], &[]).expect("op9"); m!(db, "main", "insert_person_v", &[("$name", "w1")], &[("$age", 19), ("$ver", 3)]).expect("op10"); m!(db, "main", "set_age_v", &[("$name", "w2")], &[("$age", 3), ("$ver", 4)]).expect("op11"); m!(db, "main", "add_friend", &[("$from", "Charlie"), ("$to", "w6")], &[]).expect("op12"); - db.ensure_indices().await.expect("op13"); + db.ensure_indices_as(None).await.expect("op13"); m!(db, "main", "insert_person", &[("$name", "w1")], &[("$age", 52)]).expect("op15"); m!(db, "main", "remove_friendships_from", &[("$from", "w3")], &[]).expect("op16"); m!(db, "main", "set_age_v", &[("$name", "w3")], &[("$age", 24), ("$ver", 5)]).expect("op18"); m!(db, "main", "set_age_v", &[("$name", "w7")], &[("$age", 7), ("$ver", 6)]).expect("op19"); m!(db, "main", "insert_person", &[("$name", "w7")], &[("$age", 20)]).expect("op20"); - db.cleanup(omnigraph::db::CleanupPolicyOptions { keep_versions: Some(1), older_than: None }).await.expect("op21"); + db.cleanup_as(omnigraph::db::CleanupPolicyOptions { keep_versions: Some(1), older_than: None }, None).await.expect("op21"); m!(db, "main", "set_age_v", &[("$name", "w7")], &[("$age", 17), ("$ver", 7)]).expect("op22"); m!(db, "main", "set_age_v", &[("$name", "w5")], &[("$age", 14), ("$ver", 8)]).expect("op23"); - db.cleanup(omnigraph::db::CleanupPolicyOptions { keep_versions: Some(1), older_than: None }).await.expect("op24"); + db.cleanup_as(omnigraph::db::CleanupPolicyOptions { keep_versions: Some(1), older_than: None }, None).await.expect("op24"); db.branch_create("b0").await.expect("op25"); let empty_merge = Box::pin(db.branch_merge("b0", "main")).await; println!("faithful op26 empty merge: {:?}", empty_merge.map(|_| "ok")); @@ -5373,7 +5379,7 @@ fn dst_liveness_oracle_survives_cross_thread_work() { tokio::time::resume(); let lively = tokio::time::timeout( std::time::Duration::from_secs(120), - Box::pin(db.ensure_indices()), + Box::pin(db.ensure_indices_as(None)), ) .await; tokio::time::pause(); @@ -6338,7 +6344,7 @@ fn dst_optimize_races_branch_delete_minimal_two_thread_negative() { let _ = mutate_main(&db, MUTATION_QUERIES, "set_age", ¶ms).await; } - match Box::pin(db.optimize()).await { + match Box::pin(db.optimize_as(None)).await { Ok(_) => {} Err(err) => { let rendered = format!("{err:?}"); diff --git a/crates/omnigraph-gqt-core/src/lib.rs b/crates/omnigraph-gqt-core/src/lib.rs index 9f5d9f91f..1157ebf41 100644 --- a/crates/omnigraph-gqt-core/src/lib.rs +++ b/crates/omnigraph-gqt-core/src/lib.rs @@ -4322,7 +4322,7 @@ pub async fn seed_case(session: &Session, seed: &Seed, needs_indices: bool) -> R seed.load(session).await?; if needs_indices { session - .ensure_indices() + .ensure_indices_as(None) .await .map_err(|e| format!("ensure_indices failed: {e}"))?; } diff --git a/crates/omnigraph-policy/src/lib.rs b/crates/omnigraph-policy/src/lib.rs index 3e1c18422..c0ea6200a 100644 --- a/crates/omnigraph-policy/src/lib.rs +++ b/crates/omnigraph-policy/src/lib.rs @@ -30,6 +30,16 @@ pub enum PolicyAction { /// Drop an unpublished graph commit (`commit drop ""`). Target-branch /// scoped: the branch the proposal would have landed on. CommitDrop, + /// Content-preserving maintenance: optimize, index builds, the full-text + /// rebuild and the repair report. Branch scoped like `change`: the scope + /// is the branch whose tables the operation maintains, `main` for + /// optimize and repair. It grants no right to change data or to delete + /// history. + Maintain, + /// Reclaim storage: cleanup, which permanently deletes table versions and + /// files no retained commit needs. A run covers every branch's history, + /// so the action is graph scoped and takes no branch scope. + Reclaim, /// Reserved action; no current engine or HTTP operation uses this grant. Admin, /// List the server's graph registry. This is server-scoped, with no branch @@ -57,6 +67,8 @@ impl PolicyAction { Self::BranchMerge => "branch_merge", Self::CommitPublish => "commit_publish", Self::CommitDrop => "commit_drop", + Self::Maintain => "maintain", + Self::Reclaim => "reclaim", Self::Admin => "admin", Self::GraphList => "graph_list", Self::InvokeQuery => "invoke_query", @@ -65,7 +77,10 @@ impl PolicyAction { } fn uses_branch_scope(self) -> bool { - matches!(self, Self::Read | Self::Export | Self::Change) + matches!( + self, + Self::Read | Self::Export | Self::Change | Self::Maintain + ) } fn uses_target_branch_scope(self) -> bool { @@ -98,6 +113,8 @@ impl PolicyAction { | Self::BranchMerge | Self::CommitPublish | Self::CommitDrop + | Self::Maintain + | Self::Reclaim | Self::Admin | Self::InvokeQuery => PolicyResourceKind::Graph, } @@ -160,6 +177,8 @@ impl FromStr for PolicyAction { "branch_merge" => Ok(Self::BranchMerge), "commit_publish" => Ok(Self::CommitPublish), "commit_drop" => Ok(Self::CommitDrop), + "maintain" => Ok(Self::Maintain), + "reclaim" => Ok(Self::Reclaim), "admin" => Ok(Self::Admin), "graph_list" => Ok(Self::GraphList), "invoke_query" => Ok(Self::InvokeQuery), @@ -942,6 +961,8 @@ namespace Omnigraph { action "branch_merge" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; action "commit_publish" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; action "commit_drop" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; + action "maintain" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; + action "reclaim" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; action "admin" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; action "invoke_query" appliesTo { principal: Actor, resource: Graph, context: RequestContext }; @@ -1095,6 +1116,35 @@ pub trait PolicyChecker: Send + Sync { scope: &ResourceScope, actor: &str, ) -> Result<(), PolicyError>; + + /// Gate for an operation that any one of `actions` admits on `scope`, + /// such as the full-text rebuild, which `maintain` and its older grant + /// `change` both admit. Each action is decided as [`Self::check`] decides + /// it, in order, and the first allowed one admits the operation. An + /// evaluation failure is returned at once rather than passed over for + /// the next action, so a broken rule never widens access. When every + /// action is denied, the denial names each one. + fn check_any( + &self, + actions: &[PolicyAction], + scope: &ResourceScope, + actor: &str, + ) -> Result<(), PolicyError> { + if actions.is_empty() { + return Err(PolicyError::Internal( + "no policy action to check".to_string(), + )); + } + let mut denials = Vec::with_capacity(actions.len()); + for &action in actions { + match self.check(action, scope, actor) { + Ok(()) => return Ok(()), + Err(PolicyError::Denied(message)) => denials.push(message), + Err(internal) => return Err(internal), + } + } + Err(PolicyError::Denied(denials.join("; "))) + } } impl PolicyChecker for PolicyEngine { @@ -1951,4 +2001,225 @@ rules: assert!(PolicyEngine::load_cluster_from_source(&mixed).is_err()); } } + + #[test] + fn every_action_round_trips_through_its_name() { + use clap::ValueEnum; + for &action in PolicyAction::value_variants() { + let name = action.as_str(); + assert_eq!(action.to_string(), name); + assert_eq!(name.parse::().unwrap(), action, "{name}"); + assert_eq!( + serde_json::to_value(action).unwrap(), + serde_json::Value::String(name.to_string()) + ); + assert_eq!( + serde_json::from_value::(serde_json::json!(name)).unwrap(), + action + ); + } + assert_eq!( + "maintain".parse::().unwrap(), + PolicyAction::Maintain + ); + assert_eq!( + "reclaim".parse::().unwrap(), + PolicyAction::Reclaim + ); + assert!("maintenance".parse::().is_err()); + } + + /// A rule granting any one action compiles under the Cedar schema and + /// allows that action, so no action can be added to the enum without its + /// declaration. + #[test] + fn every_action_is_declared_in_the_cedar_schema() { + use clap::ValueEnum; + for &action in PolicyAction::value_variants() { + let source = format!( + "version: 1\ngroups:\n g: [act]\nrules:\n - id: r\n allow:\n actors: {{ group: g }}\n actions: [{}]\n", + action.as_str() + ); + let engine = match action.resource_kind() { + PolicyResourceKind::Graph => PolicyEngine::load_graph_from_source(&source, "graph"), + PolicyResourceKind::Server => PolicyEngine::load_server_from_source(&source), + PolicyResourceKind::Cluster => PolicyEngine::load_cluster_from_source(&source), + } + .unwrap_or_else(|error| panic!("{action}: {error}")); + let decision = engine + .authorize( + "act", + &PolicyRequest { + action, + branch: None, + target_branch: None, + }, + ) + .unwrap(); + assert!(decision.allowed, "{action}: {}", decision.message); + } + } + + #[test] + fn maintain_is_branch_scoped_and_reclaim_takes_no_branch_scope() { + let policy: PolicyConfig = yaml_serde::from_str( + r#" +version: 1 +groups: + automation: [act-bot] + writers: [act-writer] +protected_branches: [main] +rules: + - id: automation-maintains-unprotected + allow: + actors: { group: automation } + actions: [maintain] + branch_scope: unprotected + - id: writers-change + allow: + actors: { group: writers } + actions: [change] + branch_scope: any +"#, + ) + .unwrap(); + let engine = PolicyCompiler::compile(&policy, "graph").unwrap(); + let decide = |actor: &str, action, branch: Option<&str>| { + engine + .authorize( + actor, + &PolicyRequest { + action, + branch: branch.map(str::to_string), + target_branch: None, + }, + ) + .unwrap() + .allowed + }; + assert!(decide("act-bot", PolicyAction::Maintain, Some("feature"))); + assert!(!decide("act-bot", PolicyAction::Maintain, Some("main"))); + // Maintenance grants neither data writes nor reclamation. + assert!(!decide("act-bot", PolicyAction::Change, Some("feature"))); + assert!(!decide("act-bot", PolicyAction::Reclaim, None)); + // And `change` grants no maintenance. + assert!(!decide( + "act-writer", + PolicyAction::Maintain, + Some("feature") + )); + assert!(!decide("act-writer", PolicyAction::Reclaim, None)); + + let reclaim = r#" +version: 1 +groups: + automation: [act-bot] +rules: + - id: automation-reclaims + allow: + actors: { group: automation } + actions: [reclaim] +"#; + let engine = PolicyEngine::load_graph_from_source(reclaim, "graph").unwrap(); + let allowed = engine + .authorize( + "act-bot", + &PolicyRequest { + action: PolicyAction::Reclaim, + branch: None, + target_branch: None, + }, + ) + .unwrap(); + assert!(allowed.allowed); + assert_eq!( + PolicyAction::Reclaim.resource_kind(), + PolicyResourceKind::Graph + ); + for scope in ["branch_scope", "target_branch_scope"] { + let scoped = reclaim.replace( + "actions: [reclaim]", + &format!("actions: [reclaim]\n {scope}: any"), + ); + let Err(error) = PolicyEngine::load_graph_from_source(&scoped, "graph") else { + panic!("{scope} on reclaim must be refused"); + }; + assert!( + error.to_string().contains("unsupported action 'reclaim'"), + "{scope}: {error}" + ); + } + let target_scoped_maintain = reclaim.replace("[reclaim]", "[maintain]").replace( + "actions: [maintain]", + "actions: [maintain]\n target_branch_scope: any", + ); + assert!(PolicyEngine::load_graph_from_source(&target_scoped_maintain, "graph").is_err()); + } + + #[test] + fn check_any_admits_on_any_allowed_action_and_names_every_denial() { + use super::{PolicyChecker, PolicyError, ResourceScope}; + + let policy = PolicyEngine::load_graph_from_source( + r#" +version: 1 +groups: + maintainers: [act-maintainer] + writers: [act-writer] + readers: [act-reader] +rules: + - id: maintainers + allow: + actors: { group: maintainers } + actions: [maintain] + - id: writers + allow: + actors: { group: writers } + actions: [change] +"#, + "graph", + ) + .unwrap(); + let either = [PolicyAction::Maintain, PolicyAction::Change]; + let scope = ResourceScope::Branch("main".to_string()); + for actor in ["act-maintainer", "act-writer"] { + policy.check_any(&either, &scope, actor).unwrap(); + } + match policy.check_any(&either, &scope, "act-reader") { + Err(PolicyError::Denied(message)) => { + assert!( + message.contains("'maintain'") && message.contains("'change'"), + "{message}" + ); + } + other => panic!("expected a denial naming both actions, got {other:?}"), + } + assert!(matches!( + policy.check_any(&[], &scope, "act-maintainer"), + Err(PolicyError::Internal(_)) + )); + + /// Fails to evaluate `maintain` and allows everything else. + struct BrokenMaintain; + impl PolicyChecker for BrokenMaintain { + fn check( + &self, + action: PolicyAction, + _scope: &ResourceScope, + _actor: &str, + ) -> Result<(), PolicyError> { + match action { + PolicyAction::Maintain => Err(PolicyError::Internal("broken".to_string())), + _ => Ok(()), + } + } + } + assert!( + matches!( + BrokenMaintain.check_any(&either, &scope, "act"), + Err(PolicyError::Internal(message)) if message == "broken" + ), + "an evaluation failure must not fall through to the next action" + ); + } } diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index a0829806b..1f947bc9a 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -1788,6 +1788,12 @@ impl ApiError { err @ OmniError::InitializationClaimed { .. } => Self::conflict(err.to_string()), err @ (OmniError::InitializationCommitted { .. } | OmniError::InitializationIndeterminate { .. }) => Self::internal(err.to_string()), + // A maintenance run stopped at a cancel point because its lane was + // cancelled for shutdown or a drain; the run's completion evidence + // says where it stopped. + err @ OmniError::MaintenanceCancelled { .. } => { + Self::new(ErrorCode::ServiceUnavailable, err.to_string()) + } OmniError::Completion { .. } => unreachable!("completion evidence was unwrapped"), }; response.completion_uncertain |= uncertain; diff --git a/crates/omnigraph-server/src/redacted_cause.rs b/crates/omnigraph-server/src/redacted_cause.rs index 17a2d92fc..f4ec26307 100644 --- a/crates/omnigraph-server/src/redacted_cause.rs +++ b/crates/omnigraph-server/src/redacted_cause.rs @@ -58,6 +58,7 @@ impl RedactedCause { OmniError::InitializationCommitted { .. } => "InitializationCommitted", OmniError::InitializationIndeterminate { .. } => "InitializationIndeterminate", OmniError::InitializationClaimed { .. } => "InitializationClaimed", + OmniError::MaintenanceCancelled { .. } => "MaintenanceCancelled", }; let manifest_kind = match error { OmniError::Manifest(manifest) => Some(manifest.kind), diff --git a/crates/omnigraph/src/core_tests.rs b/crates/omnigraph/src/core_tests.rs index 65bcf88fd..749d15e72 100644 --- a/crates/omnigraph/src/core_tests.rs +++ b/crates/omnigraph/src/core_tests.rs @@ -66,6 +66,7 @@ mod fts_compat_tests { &[IndexBuildSpec::FullText { column: "body".into(), }], + &crate::db::MaintenanceBudget::DEFAULT, ) .await .unwrap(); diff --git a/crates/omnigraph/src/db/catalog_tests.rs b/crates/omnigraph/src/db/catalog_tests.rs index e31c51d6b..6513e0ed3 100644 --- a/crates/omnigraph/src/db/catalog_tests.rs +++ b/crates/omnigraph/src/db/catalog_tests.rs @@ -624,6 +624,7 @@ async fn snapshot_dataset_proves_index_inventory_from_raw_manifest_section() { column: "id".to_string(), name: None, }], + &crate::db::MaintenanceBudget::DEFAULT, ) .await .unwrap(); diff --git a/crates/omnigraph/src/db/graph_coordinator.rs b/crates/omnigraph/src/db/graph_coordinator.rs index f2d9d1cfe..e5a0b74ca 100644 --- a/crates/omnigraph/src/db/graph_coordinator.rs +++ b/crates/omnigraph/src/db/graph_coordinator.rs @@ -12,8 +12,10 @@ use super::commit_graph::{ CommitGraph, FirstParentPage, GraphCommit, HistoryCache, Lineage, graph_commit_from_manifest_row, }; +#[cfg(any(test, feature = "failpoints"))] +use super::manifest::DatasetUpdate; use super::manifest::{ - BranchRecords, CapturedManifestProbe, CommitBuffer, DatasetUpdate, ExpectedTableVersions, + BranchRecords, CapturedManifestProbe, CommitBuffer, ExpectedTableVersions, GenesisManifestAttempt, GraphHeadExpectation, HistoryRecord, HistoryReleaseBytes, LineageIntent, ManifestChange, ManifestCoordinator, ManifestIncarnation, ManifestInitError, PublishPrecondition, SchemaContractRow, UnattachedCommit, UnattachedIntent, @@ -896,7 +898,10 @@ impl GraphCoordinator { /// each immutable table identity matches what the caller observed before /// writing; the diagnostic alias is checked as part of the expectation. /// Mismatches surface as `OmniError::Manifest` with - /// `ManifestConflictDetails::PublishedDatasetVersionMismatch`. + /// `ManifestConflictDetails::PublishedDatasetVersionMismatch`. It publishes + /// over whatever head the branch has (`PublishPrecondition::Any`), so no + /// production writer uses it; tests and failpoint fixtures do. + #[cfg(any(test, feature = "failpoints"))] pub(crate) async fn commit_updates_with_actor_with_expected( &mut self, updates: &[DatasetUpdate], @@ -915,6 +920,7 @@ impl GraphCoordinator { /// no separate write, no `commit_graph.refresh()` to pick a parent (the /// publisher resolves it under the CAS). The in-memory commit cache is then /// updated from the intent + the resolved parent without a re-read. + #[cfg(any(test, feature = "failpoints"))] async fn commit_changes_with_actor_with_expected( &mut self, changes: &[ManifestChange], @@ -1104,6 +1110,7 @@ pub(crate) fn commit_not_found(snapshot_id: &SnapshotId) -> OmniError { } /// Wrap each `DatasetUpdate` as a `ManifestChange::Update` for the publisher. +#[cfg(any(test, feature = "failpoints"))] fn updates_to_changes(updates: &[DatasetUpdate]) -> Vec { updates .iter() diff --git a/crates/omnigraph/src/db/maintenance.rs b/crates/omnigraph/src/db/maintenance.rs new file mode 100644 index 000000000..30a04940b --- /dev/null +++ b/crates/omnigraph/src/db/maintenance.rs @@ -0,0 +1,722 @@ +//! The failure model every maintenance run shares (RFC "Served graph +//! maintenance", Phase A): its budgets, the per-graph maintenance lane that +//! runs one maintenance operation at a time, the cancel points that stop a +//! run before its publication, the completion evidence its failures carry, +//! and the runtime that keeps Lance's CPU-heavy maintenance work off the +//! threads that serve requests. +//! +//! The lane is process-local derived state: it orders maintenance runs on +//! one graph root inside one process, as the write queue orders writers, and +//! it is not writer fencing. Fencing remains the publication's exact +//! compare-and-swap; a maintenance run in another process loses that +//! compare-and-swap as it does today. + +use std::future::Future; +use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::{Arc, Mutex, OnceLock}; + +use lance::dataset::optimize::CompactionOptions; +use lance_index::scalar::InvertedIndexParams; +use tokio::sync::{OwnedMutexGuard, watch}; + +use crate::error::{CompletionEvidence, OmniError, Result}; + +/// Maintenance units (one table's compaction or index work) a run executes at +/// once. Read by [`MaintenanceBudget::from_env`]. +pub const MAINTENANCE_CONCURRENCY_ENV: &str = "OMNIGRAPH_MAINTENANCE_CONCURRENCY"; +/// Bytes a compaction scan may queue in Lance's I/O buffer. +pub const MAINTENANCE_SCAN_IO_BUFFER_ENV: &str = "OMNIGRAPH_MAINTENANCE_SCAN_IO_BUFFER_BYTES"; +/// Memory, in MiB, one full-text index build splits across its workers. +pub const MAINTENANCE_FULL_TEXT_MEMORY_ENV: &str = "OMNIGRAPH_MAINTENANCE_FULL_TEXT_MEMORY_MB"; +/// Workers one full-text index build tokenizes with. +pub const MAINTENANCE_FULL_TEXT_WORKERS_ENV: &str = "OMNIGRAPH_MAINTENANCE_FULL_TEXT_WORKERS"; + +/// Rows per compaction scan batch where no Blob sizing derives one. It is the +/// value Lance falls back to (`BATCH_SIZE_FALLBACK`), set explicitly so that +/// `LANCE_DEFAULT_BATCH_SIZE`, a process-wide variable that also sizes every +/// read, does not size maintenance. +pub(crate) const COMPACTION_BATCH_ROWS: usize = 8192; + +/// The resource budget of one maintenance run: how many units it executes at +/// once and the caps it sets on Lance's buffers. Every value has a default +/// and an environment variable, read by [`Self::from_env`]; a served graph +/// can set it on its handle with [`crate::db::Omnigraph::with_maintenance_budget`]. +/// +/// The caps bound Lance's own buffers; they are declared bounds, not measured +/// accounting of the process heap. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MaintenanceBudget { + unit_concurrency: usize, + scan_io_buffer_bytes: u64, + full_text_memory_mb: u64, + full_text_workers: usize, +} + +impl Default for MaintenanceBudget { + fn default() -> Self { + Self::DEFAULT + } +} + +impl MaintenanceBudget { + /// The most units one run may execute at once. + pub const MAX_UNIT_CONCURRENCY: usize = 2; + + /// One unit at a time, a 64 MiB scan I/O buffer, and full-text builds of + /// 512 MiB over two workers. + pub const DEFAULT: Self = Self { + unit_concurrency: 1, + scan_io_buffer_bytes: 64 * 1024 * 1024, + full_text_memory_mb: 512, + full_text_workers: 2, + }; + + /// The defaults, overridden by the `OMNIGRAPH_MAINTENANCE_*` variables. + /// A value that does not parse, or is zero, keeps its default; a unit + /// concurrency above [`Self::MAX_UNIT_CONCURRENCY`] is lowered to it. + /// Each such correction is logged. + pub fn from_env() -> Self { + Self::from_lookup(|name| std::env::var(name).ok()) + } + + pub(crate) fn from_lookup(lookup: impl Fn(&str) -> Option) -> Self { + let mut budget = Self::DEFAULT; + if let Some(concurrency) = positive::(&lookup, MAINTENANCE_CONCURRENCY_ENV) { + if concurrency > Self::MAX_UNIT_CONCURRENCY { + tracing::warn!( + target: "omnigraph::maintenance", + variable = MAINTENANCE_CONCURRENCY_ENV, + requested = concurrency, + applied = Self::MAX_UNIT_CONCURRENCY, + "maintenance unit concurrency is at most {}", + Self::MAX_UNIT_CONCURRENCY + ); + } + budget.unit_concurrency = concurrency.min(Self::MAX_UNIT_CONCURRENCY); + } + if let Some(bytes) = positive(&lookup, MAINTENANCE_SCAN_IO_BUFFER_ENV) { + budget.scan_io_buffer_bytes = bytes; + } + if let Some(megabytes) = positive(&lookup, MAINTENANCE_FULL_TEXT_MEMORY_ENV) { + budget.full_text_memory_mb = megabytes; + } + if let Some(workers) = positive(&lookup, MAINTENANCE_FULL_TEXT_WORKERS_ENV) { + budget.full_text_workers = workers; + } + budget + } + + /// This budget with `units` executed at once, `1..=MAX_UNIT_CONCURRENCY`. + pub fn with_unit_concurrency(mut self, units: usize) -> Result { + if !(1..=Self::MAX_UNIT_CONCURRENCY).contains(&units) { + return Err(OmniError::manifest(format!( + "maintenance unit concurrency must be from 1 to {}, got {units}", + Self::MAX_UNIT_CONCURRENCY + ))); + } + self.unit_concurrency = units; + Ok(self) + } + + /// This budget with a compaction scan I/O buffer of `bytes`, at least 1. + pub fn with_scan_io_buffer_bytes(mut self, bytes: u64) -> Result { + if bytes == 0 { + // Lance reads a zero buffer as no backpressure at all. + return Err(OmniError::manifest( + "the maintenance scan I/O buffer must be at least one byte", + )); + } + self.scan_io_buffer_bytes = bytes; + Ok(self) + } + + /// This budget with full-text builds of `memory_mb` MiB over `workers` + /// workers, both at least 1. + pub fn with_full_text_build(mut self, memory_mb: u64, workers: usize) -> Result { + if memory_mb == 0 || workers == 0 { + return Err(OmniError::manifest( + "a full-text build needs at least 1 MiB and one worker", + )); + } + self.full_text_memory_mb = memory_mb; + self.full_text_workers = workers; + Ok(self) + } + + pub fn unit_concurrency(&self) -> usize { + self.unit_concurrency + } + + pub fn scan_io_buffer_bytes(&self) -> u64 { + self.scan_io_buffer_bytes + } + + pub fn full_text_memory_mb(&self) -> u64 { + self.full_text_memory_mb + } + + pub fn full_text_workers(&self) -> usize { + self.full_text_workers + } + + /// The compaction options a run plans and executes with: Lance's + /// defaults with this budget's scan I/O buffer and an explicit batch + /// size. `TableStore::stage_compaction` lowers the batch size of a Blob + /// table's task to the size its rows derive. + pub(crate) fn compaction_options(&self) -> CompactionOptions { + CompactionOptions { + batch_size: Some(COMPACTION_BATCH_ROWS), + io_buffer_size: Some(self.scan_io_buffer_bytes), + ..CompactionOptions::default() + } + } + + /// The parameters of a full-text index build: the engine's analyzer + /// defaults with this budget's memory and workers. Lance splits the + /// memory evenly across the workers and persists neither value with the + /// index, so they change how a build runs, not what it writes. + pub(crate) fn full_text_params(&self) -> InvertedIndexParams { + InvertedIndexParams::default() + .memory_limit_mb(self.full_text_memory_mb) + .num_workers(self.full_text_workers) + } +} + +/// A positive integer from `name`, or `None` (with a warning when set but +/// unusable). +fn positive(lookup: &impl Fn(&str) -> Option, name: &str) -> Option +where + T: std::str::FromStr + PartialEq + Default + Copy, +{ + let raw = lookup(name)?; + match raw.trim().parse::() { + Ok(value) if value != T::default() => Some(value), + _ => { + tracing::warn!( + target: "omnigraph::maintenance", + variable = name, + "ignoring a maintenance budget variable that is not a positive integer" + ); + None + } + } +} + +/// The kind of maintenance run a lane holds. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum MaintenanceOperation { + Optimize, + Cleanup, + IndexBuild, + FullTextRebuild, +} + +impl MaintenanceOperation { + pub fn as_str(self) -> &'static str { + match self { + Self::Optimize => "optimize", + Self::Cleanup => "cleanup", + Self::IndexBuild => "index build", + Self::FullTextRebuild => "full-text rebuild", + } + } +} + +/// The run a graph's maintenance lane holds: its operation and the branch it +/// maintains (`None` for cleanup, which covers every branch). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MaintenanceInProgress { + pub operation: MaintenanceOperation, + pub branch: Option, +} + +/// One graph root's maintenance lane in this process: at most one +/// maintenance run at a time, the others waiting in arrival order in memory. +/// Nothing about it is stored. Shared by every handle on the root through +/// the root's write queue manager. +pub(crate) struct MaintenanceLane { + turn: Arc>, + /// Bumped by every cancel; a run entered at generation `g` is cancelled + /// once the generation passes `g`, so a cancel stops the run that holds + /// the lane and every run waiting for it, and none that arrives later. + cancels: watch::Sender, + active: Mutex>, +} + +impl Default for MaintenanceLane { + fn default() -> Self { + Self { + turn: Arc::new(tokio::sync::Mutex::new(())), + cancels: watch::Sender::new(0), + active: Mutex::new(None), + } + } +} + +impl MaintenanceLane { + /// Wait for the lane and hold it for one run. A cancel while waiting + /// refuses the run before any effect. + pub(crate) async fn enter( + self: &Arc, + operation: MaintenanceOperation, + branch: Option<&str>, + ) -> Result { + let mut cancel = self.cancels.subscribe(); + let entered = *cancel.borrow_and_update(); + let turn = tokio::select! { + biased; + () = cancelled_after(&mut cancel, entered) => { + return Err(OmniError::MaintenanceCancelled { + operation: operation.as_str(), + } + .with_completion_evidence(CompletionEvidence::BeforeEffect)); + } + turn = Arc::clone(&self.turn).lock_owned() => turn, + }; + *self.active.lock().expect("maintenance lane poisoned") = Some(MaintenanceInProgress { + operation, + branch: branch.map(str::to_string), + }); + Ok(MaintenanceRun { + lane: Arc::clone(self), + _turn: turn, + cancel, + entered, + operation, + phase: AtomicU8::new(BEFORE_EFFECT), + }) + } + + /// Cancel the run holding the lane and every run waiting for it. Each + /// stops at its next cancel point; a publication in progress finishes. + pub(crate) fn cancel(&self) { + self.cancels.send_modify(|generation| *generation += 1); + } + + pub(crate) fn in_progress(&self) -> Option { + self.active + .lock() + .expect("maintenance lane poisoned") + .clone() + } +} + +/// Resolves once the lane's cancel generation passes `entered`. +async fn cancelled_after(cancel: &mut watch::Receiver, entered: u64) { + if cancel + .wait_for(|generation| *generation > entered) + .await + .is_err() + { + // The lane outlives every run it holds; a closed channel cancels + // nothing. + std::future::pending::<()>().await; + } +} + +const BEFORE_EFFECT: u8 = 0; +const STAGED: u8 = 1; +const PUBLISHING: u8 = 2; +const PUBLISHED: u8 = 3; + +/// One maintenance run holding its graph's lane. It records how far the run +/// has gone, so a failure carries the completion evidence of where it +/// stopped ([`Self::classify`]), and it answers the run's cancel points. +/// Dropping it releases the lane. +pub(crate) struct MaintenanceRun { + lane: Arc, + _turn: OwnedMutexGuard<()>, + cancel: watch::Receiver, + entered: u64, + operation: MaintenanceOperation, + phase: AtomicU8, +} + +impl Drop for MaintenanceRun { + fn drop(&mut self) { + *self.lane.active.lock().expect("maintenance lane poisoned") = None; + } +} + +impl MaintenanceRun { + /// The run may leave durable effects from here on: detached commits, + /// staged files or deletions that no `__manifest` version names. Nothing + /// is published. + pub(crate) fn staged(&self) { + self.phase.fetch_max(STAGED, Ordering::SeqCst); + } + + /// The run hands its batch to the publisher, whose own evidence (or a + /// publication in doubt) its failure carries. + pub(crate) fn publishing(&self) { + self.phase.fetch_max(PUBLISHING, Ordering::SeqCst); + } + + /// The run's publication succeeded; a later failure follows it. + pub(crate) fn published(&self) { + self.phase.fetch_max(PUBLISHED, Ordering::SeqCst); + } + + /// Give `error` the completion evidence of where the run stopped. Before + /// any effect an opaque failure is `BeforeEffect`; after staged effects + /// it is `Unpublished`; the publisher's own failure keeps what the + /// publisher said, so a publication in doubt stays uncertain; after the + /// publication every failure is `PartlyPublished`. A typed outcome + /// before the publication keeps its shape. + pub(crate) fn classify(&self, error: OmniError) -> OmniError { + match self.phase.load(Ordering::SeqCst) { + BEFORE_EFFECT => error.before_effect_if_opaque(), + STAGED => error.unpublished(), + PUBLISHING => error, + _ => error.after_earlier_publication(), + } + } + + pub(crate) fn is_cancelled(&self) -> bool { + *self.cancel.borrow() > self.entered + } + + /// A cancel point: refuse when the lane was cancelled since the run + /// entered it. + pub(crate) fn check(&self) -> Result<()> { + if self.is_cancelled() { + return Err(self.cancelled()); + } + Ok(()) + } + + /// Run `work` until it finishes or the lane is cancelled, in which case + /// `work` is dropped where it stands. Only work whose durable effects + /// stay unreachable may run here: reads, detached commits and staged + /// files, never a publication or a deletion. + pub(crate) async fn unless_cancelled(&self, work: F) -> Result { + let mut cancel = self.cancel.clone(); + tokio::select! { + biased; + () = cancelled_after(&mut cancel, self.entered) => Err(self.cancelled()), + output = work => Ok(output), + } + } + + fn cancelled(&self) -> OmniError { + let evidence = match self.phase.load(Ordering::SeqCst) { + BEFORE_EFFECT => CompletionEvidence::BeforeEffect, + STAGED | PUBLISHING => CompletionEvidence::Unpublished, + _ => CompletionEvidence::PartlyPublished { + step_unpublished: true, + }, + }; + OmniError::MaintenanceCancelled { + operation: self.operation.as_str(), + } + .with_completion_evidence(evidence) + } +} + +/// Run `work`, a Lance call that does CPU-heavy maintenance work (a +/// compaction task, an index build), on the maintenance runtime, a small +/// runtime of its own, rather than on the caller's. +/// +/// Lance decodes the batches a compaction reads in tasks it spawns with +/// `tokio::spawn` (`lance-encoding` `decoder.rs`), tokenizes a full-text +/// build's documents in tasks of a `JoinSet` (`lance-index` +/// `scalar/inverted/builder.rs`), and sorts a BTREE's training data in the +/// DataFusion stream it polls, all on whichever runtime polls the call. +/// Neither `spawn_blocking` nor Lance's CPU pool can hold that work: tasks +/// spawned from a blocking thread still land on the runtime's workers, and +/// Lance's pool takes only synchronous closures. Polling the call here puts +/// those tasks on this runtime's workers, so a server's request handlers and +/// its lease keeper keep theirs. Dropping the returned future aborts the +/// call, as dropping it in place would. +/// +/// A thread running under the deterministic simulation harness polls `work` +/// in place: the harness schedules one thread per actor. +pub(crate) async fn off_serving_runtime(work: F) -> F::Output +where + F: Future + Send + 'static, + F::Output: Send + 'static, +{ + if harness_thread() { + return work.await; + } + let task = maintenance_runtime().spawn(tracing::Instrument::in_current_span(work)); + let _abort = AbortOnDrop(task.abort_handle()); + match task.await { + Ok(output) => output, + Err(error) => match error.try_into_panic() { + Ok(panic) => std::panic::resume_unwind(panic), + // Only this future's own drop aborts the task, and then nothing + // awaits it. + Err(error) => panic!("maintenance runtime task failed: {error}"), + }, + } +} + +struct AbortOnDrop(tokio::task::AbortHandle); + +impl Drop for AbortOnDrop { + fn drop(&mut self) { + self.0.abort(); + } +} + +/// Whether this thread runs under the deterministic simulation harness. +fn harness_thread() -> bool { + crate::dst_clock::installed() || crate::dst_ids::installed() +} + +/// The maintenance runtime: one worker per unit a run may execute at once, +/// built on first use and kept for the life of the process. +fn maintenance_runtime() -> &'static tokio::runtime::Runtime { + static RUNTIME: OnceLock = OnceLock::new(); + RUNTIME.get_or_init(|| { + tokio::runtime::Builder::new_multi_thread() + .worker_threads(MaintenanceBudget::MAX_UNIT_CONCURRENCY) + .thread_name("omnigraph-maintenance") + .enable_all() + .build() + .expect("the maintenance runtime starts") + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn lookup<'a>(vars: &'a [(&'a str, &'a str)]) -> impl Fn(&str) -> Option + 'a { + move |name| { + vars.iter() + .find(|(key, _)| *key == name) + .map(|(_, value)| value.to_string()) + } + } + + #[test] + fn budget_defaults_and_overrides() { + let defaults = MaintenanceBudget::from_lookup(lookup(&[])); + assert_eq!(defaults, MaintenanceBudget::DEFAULT); + assert_eq!(defaults.unit_concurrency(), 1); + assert_eq!(defaults.scan_io_buffer_bytes(), 64 * 1024 * 1024); + assert_eq!(defaults.full_text_memory_mb(), 512); + assert_eq!(defaults.full_text_workers(), 2); + + let set = MaintenanceBudget::from_lookup(lookup(&[ + (MAINTENANCE_CONCURRENCY_ENV, "2"), + (MAINTENANCE_SCAN_IO_BUFFER_ENV, "1048576"), + (MAINTENANCE_FULL_TEXT_MEMORY_ENV, " 256 "), + (MAINTENANCE_FULL_TEXT_WORKERS_ENV, "1"), + ])); + assert_eq!(set.unit_concurrency(), 2); + assert_eq!(set.scan_io_buffer_bytes(), 1_048_576); + assert_eq!(set.full_text_memory_mb(), 256); + assert_eq!(set.full_text_workers(), 1); + + // The old default of eight units is lowered to the most a run takes. + let clamped = MaintenanceBudget::from_lookup(lookup(&[(MAINTENANCE_CONCURRENCY_ENV, "8")])); + assert_eq!(clamped.unit_concurrency(), 2); + + for unusable in ["0", "-1", "two", ""] { + let kept = MaintenanceBudget::from_lookup(lookup(&[ + (MAINTENANCE_CONCURRENCY_ENV, unusable), + (MAINTENANCE_SCAN_IO_BUFFER_ENV, unusable), + (MAINTENANCE_FULL_TEXT_MEMORY_ENV, unusable), + (MAINTENANCE_FULL_TEXT_WORKERS_ENV, unusable), + ])); + assert_eq!(kept, MaintenanceBudget::DEFAULT, "{unusable:?}"); + } + + assert!(MaintenanceBudget::DEFAULT.with_unit_concurrency(0).is_err()); + assert!(MaintenanceBudget::DEFAULT.with_unit_concurrency(3).is_err()); + assert!( + MaintenanceBudget::DEFAULT + .with_scan_io_buffer_bytes(0) + .is_err() + ); + assert!( + MaintenanceBudget::DEFAULT + .with_full_text_build(0, 2) + .is_err() + ); + assert!( + MaintenanceBudget::DEFAULT + .with_full_text_build(512, 0) + .is_err() + ); + } + + #[test] + fn compaction_options_carry_the_budget() { + let budget = MaintenanceBudget::DEFAULT + .with_scan_io_buffer_bytes(4096) + .unwrap(); + let options = budget.compaction_options(); + assert_eq!(options.io_buffer_size, Some(4096)); + assert_eq!(options.batch_size, Some(COMPACTION_BATCH_ROWS)); + let defaults = CompactionOptions::default(); + assert_eq!( + options.target_rows_per_fragment, + defaults.target_rows_per_fragment + ); + assert_eq!(options.max_source_fragments, None); + assert_eq!(options.max_source_rows, None); + assert_eq!(options.max_source_bytes, None); + } + + fn storage_error() -> OmniError { + OmniError::Io(std::io::Error::other("storage failed")) + } + + #[tokio::test] + async fn a_run_classifies_failures_by_how_far_it_went() { + let lane = Arc::new(MaintenanceLane::default()); + let run = lane + .enter(MaintenanceOperation::Optimize, Some("main")) + .await + .unwrap(); + assert_eq!( + run.classify(storage_error()).completion_evidence(), + Some(CompletionEvidence::BeforeEffect) + ); + let typed = run.classify(OmniError::manifest_conflict("pin moved")); + assert!( + matches!(typed, OmniError::Manifest(_)), + "a typed outcome keeps its shape" + ); + + run.staged(); + assert_eq!( + run.classify(storage_error()).completion_evidence(), + Some(CompletionEvidence::Unpublished) + ); + + run.publishing(); + assert_eq!(run.classify(storage_error()).completion_evidence(), None); + assert_eq!( + run.classify(OmniError::manifest_publish_in_doubt("lost")) + .completion_evidence(), + Some(CompletionEvidence::Uncertain), + "a publication in doubt stays uncertain" + ); + + run.published(); + assert_eq!( + run.classify(storage_error()).completion_evidence(), + Some(CompletionEvidence::PartlyPublished { + step_unpublished: false + }) + ); + // The phase only moves forward. + run.staged(); + assert!(matches!( + run.classify(storage_error()).completion_evidence(), + Some(CompletionEvidence::PartlyPublished { .. }) + )); + } + + #[tokio::test] + async fn the_lane_holds_one_run_and_a_cancel_stops_it_and_its_waiters() { + let lane = Arc::new(MaintenanceLane::default()); + let first = lane + .enter(MaintenanceOperation::Optimize, Some("main")) + .await + .unwrap(); + assert_eq!( + lane.in_progress(), + Some(MaintenanceInProgress { + operation: MaintenanceOperation::Optimize, + branch: Some("main".to_string()), + }) + ); + + let waiting_lane = Arc::clone(&lane); + let mut waiter = tokio::spawn(async move { + waiting_lane + .enter(MaintenanceOperation::Cleanup, None) + .await + .map(|_| ()) + }); + assert!( + tokio::time::timeout(std::time::Duration::from_millis(50), &mut waiter) + .await + .is_err(), + "a second run waits for the lane" + ); + + first.staged(); + assert!(!first.is_cancelled()); + lane.cancel(); + let refused = first.check().unwrap_err(); + assert_eq!( + refused.completion_evidence(), + Some(CompletionEvidence::Unpublished) + ); + assert!(matches!( + refused.into_completion_evidence().0, + OmniError::MaintenanceCancelled { + operation: "optimize" + } + )); + let waited = waiter.await.unwrap().unwrap_err(); + assert_eq!( + waited.completion_evidence(), + Some(CompletionEvidence::BeforeEffect), + "a waiting run is refused before any effect" + ); + let pending = first + .unless_cancelled(std::future::pending::<()>()) + .await + .unwrap_err(); + assert!(matches!( + pending.into_completion_evidence().0, + OmniError::MaintenanceCancelled { .. } + )); + + drop(first); + assert_eq!(lane.in_progress(), None); + let later = lane + .enter(MaintenanceOperation::IndexBuild, Some("feature")) + .await + .expect("a run that arrives after the cancel is not cancelled"); + assert!(later.check().is_ok()); + assert_eq!( + later.unless_cancelled(async { 7 }).await.unwrap(), + 7, + "work finishes when nothing cancels it" + ); + } + + #[tokio::test] + async fn work_off_the_serving_runtime_returns_its_output_and_aborts_on_drop() { + let caller = std::thread::current().id(); + let (ran_on, value) = + off_serving_runtime(async { (std::thread::current().id(), 41 + 1) }).await; + assert_eq!(value, 42); + assert_ne!(ran_on, caller, "the work ran on the maintenance runtime"); + let name = + off_serving_runtime(async { std::thread::current().name().map(str::to_string) }).await; + assert_eq!(name.as_deref(), Some("omnigraph-maintenance")); + + let (started_tx, started_rx) = tokio::sync::oneshot::channel::<()>(); + let (dropped_tx, dropped_rx) = tokio::sync::oneshot::channel::<()>(); + struct SignalOnDrop(Option>); + impl Drop for SignalOnDrop { + fn drop(&mut self) { + if let Some(sender) = self.0.take() { + let _ = sender.send(()); + } + } + } + let work = tokio::spawn(off_serving_runtime(async move { + let _signal = SignalOnDrop(Some(dropped_tx)); + let _ = started_tx.send(()); + std::future::pending::<()>().await; + })); + started_rx.await.unwrap(); + work.abort(); + tokio::time::timeout(std::time::Duration::from_secs(5), dropped_rx) + .await + .expect("dropping the caller drops the work") + .unwrap(); + } +} diff --git a/crates/omnigraph/src/db/mod.rs b/crates/omnigraph/src/db/mod.rs index 85f3d3415..fa558d47d 100644 --- a/crates/omnigraph/src/db/mod.rs +++ b/crates/omnigraph/src/db/mod.rs @@ -3,6 +3,7 @@ mod catalog_tests; pub mod commit_graph; pub(crate) mod graph_coordinator; pub(crate) mod legacy_sidecars; +pub(crate) mod maintenance; pub mod manifest; pub(crate) mod omnigraph; mod schema_state; @@ -14,6 +15,11 @@ pub(crate) mod write_queue; pub use commit_graph::{GraphCommit, GraphCommitKind}; pub use graph_coordinator::{ReadTarget, ResolvedTarget, SnapshotId}; +pub use maintenance::{ + MAINTENANCE_CONCURRENCY_ENV, MAINTENANCE_FULL_TEXT_MEMORY_ENV, + MAINTENANCE_FULL_TEXT_WORKERS_ENV, MAINTENANCE_SCAN_IO_BUFFER_ENV, MaintenanceBudget, + MaintenanceInProgress, MaintenanceOperation, +}; pub use manifest::{DatasetEntry, DatasetUpdate}; pub use omnigraph::{ BranchCreated, COMMIT_LIST_DEFAULT_PAGE_COMMITS, COMMIT_LIST_MAX_PAGE_COMMITS, @@ -22,7 +28,7 @@ pub use omnigraph::{ DatasetRepairStats, EXPORT_CHUNK_MAX_BYTES, ExportCut, FullTextIndexRebuildResult, GraphCreateReconciliation, InitOptions, MergeOutcome, MergeResult, Omnigraph, OpenMode, PendingIndex, PreparedGraphCreate, PreparedSchemaApply, PreparedSchemaSettlement, ReadCheck, - RebuiltFullTextIndex, RepairAction, RepairClassification, RepairOptions, RepairStats, + RebuiltFullTextIndex, RepairAction, RepairClassification, RepairStats, RetainedManifestVersions, SYSTEM_COLUMNS_PREFLIGHT, SchemaApplyReconciliation, SchemaApplyResult, SchemaApplySettlement, SchemaContractDigest, SchemaNonPublicationProof, SkipReason, StagingVerdict, SystemColumnUpgradeFinding, SystemColumnUpgradeOptions, diff --git a/crates/omnigraph/src/db/omnigraph.rs b/crates/omnigraph/src/db/omnigraph.rs index 2a5b62ff7..bc004fdd4 100644 --- a/crates/omnigraph/src/db/omnigraph.rs +++ b/crates/omnigraph/src/db/omnigraph.rs @@ -53,9 +53,7 @@ pub(crate) use export::{ pub use optimize::{CleanupPolicyOptions, DatasetCleanupStats, DatasetOptimizeStats, SkipReason}; use prepared_create::initial_schema_ir; pub use prepared_create::{GraphCreateReconciliation, PreparedGraphCreate}; -pub use repair::{ - DatasetRepairStats, RepairAction, RepairClassification, RepairOptions, RepairStats, -}; +pub use repair::{DatasetRepairStats, RepairAction, RepairClassification, RepairStats}; pub use schema_apply::{ PreparedSchemaApply, PreparedSchemaSettlement, SchemaApplyReconciliation, SchemaApplySettlement, SchemaContractDigest, SchemaNonPublicationProof, @@ -471,6 +469,9 @@ pub struct Omnigraph { /// When set, the embedding cell builds its client from this instead of /// `EmbeddingClient::from_env()`; `None` keeps the env fallback. embedding_config: Option>, + /// The maintenance budget installed with [`Omnigraph::with_maintenance_budget`]; + /// `None` reads `MaintenanceBudget::from_env` at the start of each run. + maintenance_budget: Option, } /// Whether open checks write capability and refuses legacy recovery sidecars. @@ -745,6 +746,7 @@ impl Omnigraph { policy: None, embedding: Arc::new(tokio::sync::OnceCell::new()), embedding_config: None, + maintenance_budget: None, }) } @@ -938,6 +940,7 @@ impl Omnigraph { policy: None, embedding: Arc::new(tokio::sync::OnceCell::new()), embedding_config: None, + maintenance_budget: None, }; // The returned handle now owns one coherent schema source/catalog view. // Release only after both have been installed in the new object. @@ -1062,6 +1065,53 @@ impl Omnigraph { self } + /// Install the maintenance budget this handle's maintenance runs use, in + /// place of the one [`MaintenanceBudget::from_env`] reads at the start of + /// each run. Builder-style, like [`Omnigraph::with_embedding_config`]. + /// + /// [`MaintenanceBudget::from_env`]: crate::db::MaintenanceBudget::from_env + pub fn with_maintenance_budget( + mut self, + budget: crate::db::maintenance::MaintenanceBudget, + ) -> Self { + self.maintenance_budget = Some(budget); + self + } + + /// The budget a maintenance run starting now uses. + pub(crate) fn maintenance_budget(&self) -> crate::db::maintenance::MaintenanceBudget { + self.maintenance_budget + .unwrap_or_else(crate::db::maintenance::MaintenanceBudget::from_env) + } + + /// Wait for this graph's maintenance lane and hold it for one run of + /// `operation` on `branch` (`None` for a run that covers every branch). + pub(crate) async fn enter_maintenance( + &self, + operation: crate::db::maintenance::MaintenanceOperation, + branch: Option<&str>, + ) -> Result { + self.write_queue + .maintenance_lane() + .enter(operation, branch) + .await + } + + /// Cancel the maintenance run this graph's lane holds in this process, + /// and every run waiting for it. Each stops at its next cancel point + /// with [`OmniError::MaintenanceCancelled`] and the completion evidence + /// of where it stopped; a publication already in progress finishes. + /// Runs that start after the call are not affected. Every handle on the + /// same root shares the lane. + pub fn cancel_maintenance(&self) { + self.write_queue.maintenance_lane().cancel(); + } + + /// The maintenance run this graph's lane holds in this process, if any. + pub fn maintenance_in_progress(&self) -> Option { + self.write_queue.maintenance_lane().in_progress() + } + /// Prepare an immutable runtime view over this handle's existing owner. /// /// No storage is opened or written. The coordinator, schema authority, @@ -1095,6 +1145,7 @@ impl Omnigraph { policy, embedding: Arc::new(tokio::sync::OnceCell::new()), embedding_config, + maintenance_budget: self.maintenance_budget, }) } @@ -1130,6 +1181,19 @@ impl Omnigraph { action: omnigraph_policy::PolicyAction, scope: &omnigraph_policy::ResourceScope, actor: Option<&str>, + ) -> Result<()> { + self.enforce_any(&[action], scope, actor) + } + + /// [`Self::enforce`] for an operation that any one of `actions` admits on + /// `scope`, decided by [`omnigraph_policy::PolicyChecker::check_any`]: the + /// first allowed action admits it, an evaluation failure refuses it, and a + /// refusal names every denied action. + pub(crate) fn enforce_any( + &self, + actions: &[omnigraph_policy::PolicyAction], + scope: &omnigraph_policy::ResourceScope, + actor: Option<&str>, ) -> Result<()> { let (branch, target_branch) = scope.to_branch_pair(); for name in [branch, target_branch].into_iter().flatten() { @@ -1147,7 +1211,7 @@ impl Omnigraph { )); }; checker - .check(action, scope, actor) + .check_any(actions, scope, actor) .map_err(|err| OmniError::Policy(err.to_string())) } @@ -3528,14 +3592,23 @@ impl Omnigraph { /// under the final gates. /// Returns the declared indexes that could not be materialized on this /// pass (today: vector properties with no trainable vectors yet). They are - /// deferred, not errors; a later `ensure_indices`/`optimize` builds them - /// once the property is trainable. Reads stay correct (brute-force) meanwhile. - pub async fn ensure_indices(&self) -> Result> { - table_ops::ensure_indices(self).await + /// deferred, not errors; a later `ensure_indices_as`/`optimize_as` builds + /// them once the property is trainable. Reads stay correct (brute-force) + /// meanwhile. + /// + /// Works on the handle's current branch. Requires `maintain` on that + /// branch before any effect and records `actor` in the publication. + pub async fn ensure_indices_as(&self, actor: Option<&str>) -> Result> { + table_ops::ensure_indices_as(self, actor).await } - pub async fn ensure_indices_on(&self, branch: &str) -> Result> { - table_ops::ensure_indices_on(self, branch).await + /// [`Self::ensure_indices_as`] on `branch`, under `maintain` on `branch`. + pub async fn ensure_indices_on_as( + &self, + branch: &str, + actor: Option<&str>, + ) -> Result> { + table_ops::ensure_indices_on_as(self, branch, actor).await } /// Fully rebuild all declared or existing supported full-text indexes from @@ -3545,19 +3618,13 @@ impl Omnigraph { /// Rebuilt full-text indexes use the engine's default analyzer, including /// indexes originally created externally with custom tokenizer settings. /// - /// Unlike [`Self::ensure_indices_on`], this replaces existing FTS indexes - /// even when their row coverage is complete. It establishes the supported - /// analyzer generation after an upgrade; it does not rewrite old history. - /// With a policy installed, use the actor-aware counterpart. - pub async fn rebuild_full_text_indices_on( - &self, - branch: &str, - ) -> Result { - self.rebuild_full_text_indices_on_as(branch, None).await - } - - /// Actor-aware full-text rebuilding. Requires `Change` on the selected - /// branch before any effects and records the actor in the graph publication. + /// Unlike [`Self::ensure_indices_on_as`], this replaces existing FTS + /// indexes even when their row coverage is complete. It establishes the + /// supported analyzer generation after an upgrade; it does not rewrite old + /// history. + /// + /// Requires `maintain` or `change` on the selected branch before any + /// effect and records the actor in the graph publication. pub async fn rebuild_full_text_indices_on_as( &self, branch: &str, @@ -3584,26 +3651,34 @@ impl Omnigraph { } /// Compact small Lance fragments into fewer larger ones across every - /// backing dataset for a node or edge type on `main`. See [`optimize`] for details. - pub async fn optimize(&self) -> Result> { - optimize::optimize_all_datasets(self).await + /// backing dataset for a node or edge type on `main`. See [`optimize`] for + /// details. Requires `maintain` on `main` before any effect and records + /// `actor` in the publication. + pub async fn optimize_as( + &self, + actor: Option<&str>, + ) -> Result> { + optimize::optimize_all_datasets(self, actor).await } - /// Classify and explicitly repair uncovered graph-manifest/Lance-HEAD drift. See - /// [`repair`] for the distinction between safe maintenance drift and - /// suspicious/unverifiable drift. - pub async fn repair(&self, options: repair::RepairOptions) -> Result { - repair::repair_all_datasets(self, options).await + /// Report, per table on `main`, how its Lance linear history stands + /// against its registration. See [`repair`] for the classifications. The + /// report publishes nothing and adopts no drift. Requires `maintain` on + /// `main`. + pub async fn repair_report_as(&self, actor: Option<&str>) -> Result { + repair::repair_report(self, actor).await } /// Remove Lance manifests (and the fragments they uniquely own) per the /// given [`optimize::CleanupPolicyOptions`]. Destructive to version - /// history. See [`optimize`] for details. - pub async fn cleanup( + /// history. See [`optimize`] for details. Requires `reclaim` before any + /// effect; a run covers every branch, so the check names none. + pub async fn cleanup_as( &self, options: optimize::CleanupPolicyOptions, + actor: Option<&str>, ) -> Result> { - optimize::cleanup_all_datasets(self, options).await + optimize::cleanup_all_datasets(self, options, actor).await } /// The tracing collector's plan for `options`, deleting nothing and taking @@ -6185,7 +6260,7 @@ edge WorksAt: Person -> Company ); // The reconciler materializes it (Person has a row). - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let snapshot = db.snapshot().await; let ds = db .storage() @@ -6217,7 +6292,7 @@ edge WorksAt: Person -> Company db.apply_schema(&desired).await.unwrap(); // After the rewrite the reconciler restores index coverage. - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let snapshot = db.snapshot().await; let ds = db .storage() diff --git a/crates/omnigraph/src/db/omnigraph/collector.rs b/crates/omnigraph/src/db/omnigraph/collector.rs index f803fd81d..24c168196 100644 --- a/crates/omnigraph/src/db/omnigraph/collector.rs +++ b/crates/omnigraph/src/db/omnigraph/collector.rs @@ -2262,7 +2262,7 @@ mod borrowed_origin_tests { load_company(&db, "initial-company").await; db.branch_create("legacy").await.unwrap(); load_person(&db, "borrowed", 41).await; - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let source_snapshot = db.snapshot_of(ReadTarget::branch("main")).await.unwrap(); let entry = source_snapshot.dataset("node:Person").unwrap().clone(); let mut source = source_snapshot @@ -2376,7 +2376,7 @@ mod borrowed_origin_tests { assert!(!company_old.is_empty()); load_company(&db, "unrelated-current").await; load_person(&db, "main-current", 99).await; - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let options = super::super::optimize::CleanupPolicyOptions { keep_versions: Some(1), older_than: None, @@ -2391,7 +2391,7 @@ mod borrowed_origin_tests { source_plan.would_remove().contains(&source_version), "borrowed source manifest is reclaimable" ); - let rows = db.cleanup(options).await.unwrap(); + let rows = db.cleanup_as(options, None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); let source_store = source.object_store(None).await.unwrap(); for path in source_data.iter().chain(&source_indices) { @@ -2504,7 +2504,7 @@ mod borrowed_origin_tests { .any(|table| table.would_remove().contains(&version)), "old source pin must be reclaimable without the native tag's borrowed files" ); - let rows = db.cleanup(options).await.unwrap(); + let rows = db.cleanup_as(options, None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); drop(db); // forbidden-api-allow: test-only cold native read of a tagged fork absent from graph registrations @@ -2745,7 +2745,7 @@ mod overlay_retention_tests { .any(|object| object.path == overlay_path) ); assert!(expected.missing_paths().await.unwrap().is_empty()); - let rows = db.cleanup(options).await.unwrap(); + let rows = db.cleanup_as(options, None).await.unwrap(); let row = rows .iter() .find(|row| row.type_key == entry.type_key) @@ -2793,7 +2793,7 @@ mod overlay_retention_tests { .any(|object| object.path == overlay_path), "the independent pre-cleanup probe must include overlays" ); - let rows = db.cleanup(options.clone()).await.unwrap(); + let rows = db.cleanup_as(options.clone(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); let store = overlay.object_store(None).await.unwrap(); assert!(store.exists(&overlay_path).await.unwrap()); @@ -2842,7 +2842,7 @@ mod overlay_retention_tests { } else { load_person(&db, 99).await; } - let rows = db.cleanup(options).await.unwrap(); + let rows = db.cleanup_as(options, None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!( !store.exists(&overlay_path).await.unwrap(), @@ -3020,7 +3020,7 @@ mod native_table_tag_tests { !plan.would_remove().contains(&version), "an exact native table tag must exclude its version from cleanup: {plan:?}", ); - let rows = db.cleanup(options.clone()).await.unwrap(); + let rows = db.cleanup_as(options.clone(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!(store.exists(&manifest_path).await.unwrap()); // forbidden-api-allow: test-only cold native tag read after the graph collector ran @@ -3056,7 +3056,7 @@ mod native_table_tag_tests { .any(|plan| plan.location == location && plan.would_remove().contains(&version)), "removing the tag must make the old version reclaimable" ); - let rows = db.cleanup(options).await.unwrap(); + let rows = db.cleanup_as(options, None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!( !store.exists(&manifest_path).await.unwrap(), diff --git a/crates/omnigraph/src/db/omnigraph/optimize.rs b/crates/omnigraph/src/db/omnigraph/optimize.rs index dd5517308..235a2c043 100644 --- a/crates/omnigraph/src/db/omnigraph/optimize.rs +++ b/crates/omnigraph/src/db/omnigraph/optimize.rs @@ -31,20 +31,15 @@ use lance::index::{DatasetIndexExt, DatasetIndexInternalExt}; use super::*; use crate::dataset_index::is_full_text_index; +use crate::db::maintenance::{MaintenanceBudget, MaintenanceOperation, MaintenanceRun}; use crate::seams::{decide_seam, fail}; -/// How many datasets to optimize/cleanup concurrently. Each has separate -/// Lance dataset so there is no shared state; the bound is there to avoid -/// flooding the runtime and the S3 connection pool. -const DEFAULT_MAINT_CONCURRENCY: usize = 8; - -fn maint_concurrency() -> usize { - std::env::var("OMNIGRAPH_MAINTENANCE_CONCURRENCY") - .ok() - .and_then(|v| v.parse::().ok()) - .filter(|v| *v > 0) - .unwrap_or(DEFAULT_MAINT_CONCURRENCY) -} +/// How many tables optimize plans, and cleanup sweeps, at once. Planning +/// reads manifests and index metadata and a sweep lists and deletes objects, +/// so the bound is on concurrent requests, not on Lance's buffers; the work +/// that rewrites a table or builds its indexes runs at the budget's unit +/// concurrency instead ([`MaintenanceBudget::unit_concurrency`]). +const MAINTENANCE_IO_FANOUT: usize = 8; /// Retention knobs for [`cleanup_all_datasets`]. At least one must be set or /// nothing is cleaned. If both are set, Lance applies them as AND (a manifest @@ -216,7 +211,34 @@ decide_seam! { /// with an exact pin CAS, so one public Optimize produces at most one graph /// commit. The publisher owns main's `__manifest` fragment layout; maintenance /// only removes obsolete automatic-cleanup configuration from that dataset. -pub async fn optimize_all_datasets(db: &Omnigraph) -> Result> { +pub async fn optimize_all_datasets( + db: &Omnigraph, + actor: Option<&str>, +) -> Result> { + db.enforce( + omnigraph_policy::PolicyAction::Maintain, + &omnigraph_policy::ResourceScope::Branch("main".to_string()), + actor, + )?; + let run = db + .enter_maintenance(MaintenanceOperation::Optimize, Some("main")) + .await?; + let budget = db.maintenance_budget(); + optimize_in_lane(db, actor, &run, &budget) + .await + .map_err(|error| run.classify(error)) +} + +/// [`optimize_all_datasets`] holding the graph's maintenance lane. Its cancel +/// points come before the run's publication and between its tables; a cancel +/// drops the table work in flight, which leaves only detached commits no +/// `__manifest` version names. +async fn optimize_in_lane( + db: &Omnigraph, + actor: Option<&str>, + run: &MaintenanceRun, + budget: &MaintenanceBudget, +) -> Result> { let _export_exclusion = db.reserve_export_destructive_control()?; db.ensure_schema_state_valid().await?; @@ -267,16 +289,21 @@ pub async fn optimize_all_datasets(db: &Omnigraph) -> Result>(); - let concurrency = maint_concurrency().min(table_tasks.len()).max(1); + let concurrency = MAINTENANCE_IO_FANOUT.min(table_tasks.len()).max(1); + let options = budget.compaction_options(); - let preparations: Vec> = futures::stream::iter(table_tasks) - .map(|task| { - let catalog = std::sync::Arc::clone(&catalog); - async move { prepare_optimize_table(db, catalog.as_ref(), task).await } - }) - .buffer_unordered(concurrency) - .collect() - .await; + let preparations: Vec> = run + .unless_cancelled( + futures::stream::iter(table_tasks) + .map(|task| { + let catalog = std::sync::Arc::clone(&catalog); + let options = &options; + async move { prepare_optimize_table(db, catalog.as_ref(), task, options).await } + }) + .buffer_unordered(concurrency) + .collect(), + ) + .await?; let mut prepared = Vec::new(); let mut stats = Vec::new(); @@ -289,20 +316,30 @@ pub async fn optimize_all_datasets(db: &Omnigraph) -> Result> = futures::stream::iter(prepared) - .map(|work| { - let catalog = std::sync::Arc::clone(&catalog); - async move { apply_optimize_table_effects(db, catalog.as_ref(), work).await } - }) - .buffer_unordered(effect_concurrency) - .collect() - .await; + run.check()?; + run.staged(); + let effect_concurrency = budget.unit_concurrency().min(prepared.len()).max(1); + let effect_results: Vec> = run + .unless_cancelled( + futures::stream::iter(prepared) + .map(|work| { + let catalog = std::sync::Arc::clone(&catalog); + async move { + apply_optimize_table_effects(db, catalog.as_ref(), work, run, budget) + .await + } + }) + .buffer_unordered(effect_concurrency) + .collect(), + ) + .await?; let mut outcomes = Vec::new(); for result in effect_results { outcomes.push(result?); } fail(&OPTIMIZE_POST_PHASE_B_PRE_MANIFEST_COMMIT)?; + // The last cancel point: past it the batch publishes or fails. + run.check()?; let mut updates = Vec::new(); let mut expected_versions = crate::db::manifest::ExpectedTableVersions::new(); for outcome in &mut outcomes { @@ -324,19 +361,21 @@ pub async fn optimize_all_datasets(db: &Omnigraph) -> Result Result>>(); all.push(clear_manifest_auto_cleanup(db).await); - if let Err(error) = persist_graph_index_artifact(db, &catalog, edge_tables_committed).await { - tracing::warn!( + // The artifact is derived and regenerable, so a cancel skips it and the + // run keeps what it published. + match run + .unless_cancelled(persist_graph_index_artifact( + db, + &catalog, + edge_tables_committed, + )) + .await + { + Ok(Ok(())) => {} + Ok(Err(error)) => tracing::warn!( target: "omnigraph::optimize", error = %error, "graph index artifact persist failed; traversals keep building in memory" - ); + ), + Err(_) => tracing::info!( + target: "omnigraph::optimize", + "graph index artifact persist skipped: the maintenance run was cancelled" + ), } all.into_iter().collect() @@ -443,6 +500,7 @@ async fn prepare_optimize_table( db: &Omnigraph, catalog: &omnigraph_compiler::catalog::Catalog, task: OptimizeTableTask, + options: &CompactionOptions, ) -> Result { let snapshot = db .open_pinned_for_write(&task.full_path, &task.entry) @@ -451,8 +509,7 @@ async fn prepare_optimize_table( // The same plan `stage_compaction` executes: a fragment still holding a // dropped property's values is compaction work even when Lance's planner // would leave the table alone. - let options = CompactionOptions::default(); - let will_compact = TableStore::plan_table_compaction(snapshot.dataset(), &options) + let will_compact = TableStore::plan_table_compaction(snapshot.dataset(), options) .await? .num_tasks() > 0; @@ -550,14 +607,20 @@ async fn apply_optimize_table_effects( db: &Omnigraph, catalog: &omnigraph_compiler::catalog::Catalog, work: PreparedOptimizeTable, + run: &MaintenanceRun, + budget: &MaintenanceBudget, ) -> Result { let table_key = work.table_key; let full_path = work.full_path; let base = work.initial_snapshot; let witness = work.witness; let declared_full_text = super::table_ops::declared_full_text_columns(catalog, &table_key); + // A cancel point between tables, and below one between the links of a + // table's chain: every link is a detached commit no `__manifest` version + // names until the batch publishes. + run.check()?; fail(&OPTIMIZE_BEFORE_COMPACT)?; - let options = CompactionOptions::default(); + let options = budget.compaction_options(); let mut chain = Vec::new(); let mut tip = base.clone(); let mut tip_identity = None; @@ -573,6 +636,7 @@ async fn apply_optimize_table_effects( tip = rewrite; tip_identity = Some(identity); fail(&OPTIMIZE_POST_TABLE_EFFECT)?; + run.check()?; } // Fold every index whose coverage lags (appended fragments, or a vector // index that keeps row addresses and dropped the compacted fragments) @@ -589,6 +653,7 @@ async fn apply_optimize_table_effects( tip = folded; tip_identity = Some(identity); fail(&OPTIMIZE_POST_TABLE_EFFECT)?; + run.check()?; } let mut index_work = super::table_ops::plan_index_work_on_dataset_for_catalog(db, catalog, &table_key, &tip) @@ -605,7 +670,7 @@ async fn apply_optimize_table_effects( if !index_work.specs.is_empty() { let staged = db .storage() - .stage_create_indices(&tip, &index_work.specs) + .stage_create_indices(&tip, &index_work.specs, budget) .await .map_err(|error| { error.with_context(format!( @@ -623,6 +688,7 @@ async fn apply_optimize_table_effects( tip = indexed; tip_identity = Some(identity); fail(&OPTIMIZE_POST_TABLE_EFFECT)?; + run.check()?; } let Some(identity) = tip_identity else { let mut stat = DatasetOptimizeStats::compacted(table_key, &metrics, false); @@ -745,13 +811,34 @@ decide_seam! { pub async fn cleanup_all_datasets( db: &Omnigraph, options: CleanupPolicyOptions, + actor: Option<&str>, ) -> Result> { + db.enforce( + omnigraph_policy::PolicyAction::Reclaim, + &omnigraph_policy::ResourceScope::Graph, + actor, + )?; if options.keep_versions.is_none() && options.older_than.is_none() { return Err(OmniError::manifest( "cleanup requires at least one of keep_versions or older_than", )); } + let run = db + .enter_maintenance(MaintenanceOperation::Cleanup, None) + .await?; + cleanup_in_lane(db, options, &run) + .await + .map_err(|error| run.classify(error)) +} +/// [`cleanup_all_datasets`] holding the graph's maintenance lane. Cleanup +/// publishes nothing; a cancel stops it before its first deletion, between +/// tables and before each later step, never inside a deletion. +async fn cleanup_in_lane( + db: &Omnigraph, + options: CleanupPolicyOptions, + run: &MaintenanceRun, +) -> Result> { let _export_exclusion = db.reserve_export_destructive_control()?; db.ensure_schema_state_valid().await?; fail(&CLEANUP_PRE_GATES)?; @@ -782,7 +869,7 @@ pub async fn cleanup_all_datasets( db.revalidate_write_txn(&authority_txn).await?; - cleanup_detached_only(db, &options, &graph_branches, &table_tasks).await + cleanup_detached_only(db, &options, &graph_branches, &table_tasks, run).await } /// The detached-only `cleanup` (RFC "Detached-only tables"): plan the collector @@ -793,12 +880,24 @@ async fn cleanup_detached_only( options: &CleanupPolicyOptions, graph_branches: &[Option], table_tasks: &[(String, String)], + run: &MaintenanceRun, ) -> Result> { let before_timestamp = options .older_than .map(|duration| crate::dst_clock::now_utc() - duration); - let native_plan = prepare_native_fork_reconciliation(db, before_timestamp).await?; - let plan = super::collector::plan_collection(db, options, graph_branches).await?; + // Planning reads and deletes nothing, so a cancel drops it where it stands. + let native_plan = run + .unless_cancelled(prepare_native_fork_reconciliation(db, before_timestamp)) + .await??; + let plan = run + .unless_cancelled(super::collector::plan_collection( + db, + options, + graph_branches, + )) + .await??; + run.check()?; + run.staged(); let mut table_tasks = table_tasks.to_vec(); let mut scheduled_paths = table_tasks .iter() @@ -809,10 +908,15 @@ async fn cleanup_detached_only( table_tasks.push((planned.table_key.clone(), planned.full_path.clone())); } } - let concurrency = maint_concurrency().min(table_tasks.len()).max(1); + let concurrency = MAINTENANCE_IO_FANOUT.min(table_tasks.len()).max(1); let plan = &plan; let results: Vec = futures::stream::iter(table_tasks) .map(|(table_key, full_path)| async move { + // A cancel point between tables: a sweep that started finishes, + // so no deletion is abandoned mid-request. + if run.is_cancelled() { + return None; + } let summary = plan.row_summary(&full_path); let mut swept = super::collector::SweepStats::default(); let outcome: Result<()> = async { @@ -848,7 +952,7 @@ async fn cleanup_detached_only( ); err.to_string() }); - DatasetCleanupStats { + Some(DatasetCleanupStats { type_key: table_key, bytes_removed: swept.bytes_removed, old_versions_removed: swept.manifests_removed, @@ -857,11 +961,13 @@ async fn cleanup_detached_only( unpublished_manifests: summary.unpublished_manifests, unpublished_bytes: summary.unpublished_bytes, foreign_versions: summary.foreign_versions, - } + }) }) .buffer_unordered(concurrency) + .filter_map(std::future::ready) .collect() .await; + run.check()?; let unattached_release = super::collector::unattached_release_order( &plan.expired_unattached_tags, &plan.retained_unattached_tags, @@ -883,11 +989,13 @@ async fn cleanup_detached_only( ) .await?; for name in &plan.expired_unpublished_tags { + run.check()?; crate::db::manifest::unpublished::release_unpublished_tag(manifest.dataset(), name) .await?; } let mut unattached_tags_released: u64 = 0; for record in &unattached_release { + run.check()?; crate::db::manifest::unattached::release_unattached_tag_confirmed( manifest.dataset(), &record.name, @@ -904,6 +1012,7 @@ async fn cleanup_detached_only( "cleanup released the expired unattached branch tags" ); } + run.check()?; let reconciled = reconcile_frozen_native_forks(db, native_plan, plan).await?; if !reconciled.reclaimed.is_empty() { tracing::info!( @@ -1677,10 +1786,13 @@ mod tests { let bytes = store.inner.get(&path).await.unwrap().bytes().await.unwrap(); store.delete(&path).await.unwrap(); let rows = db - .cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert!( @@ -1694,10 +1806,13 @@ mod tests { ); store.inner.put(&path, bytes.into()).await.unwrap(); let rows = db - .cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); diff --git a/crates/omnigraph/src/db/omnigraph/repair.rs b/crates/omnigraph/src/db/omnigraph/repair.rs index ff0ac8ee8..3dbafa0bf 100644 --- a/crates/omnigraph/src/db/omnigraph/repair.rs +++ b/crates/omnigraph/src/db/omnigraph/repair.rs @@ -1,27 +1,17 @@ -//! Explicit repair for uncovered published-dataset/Lance-HEAD drift. +//! The repair report: uncovered published-dataset/Lance-HEAD drift. //! //! Pending pins converge automatically: the next writer or `cleanup` promotes //! them. This module is for the different case: a dataset's Lance HEAD is //! ahead of the version recorded in `__manifest` and no published pin explains -//! the movement. `repair` classifies that uncovered drift from Lance -//! transactions and only auto-publishes maintenance-only drift when the -//! operator confirms. +//! the movement. The report classifies that drift from Lance transactions. It +//! publishes nothing: no read or write resolves a table's linear HEAD, so +//! drift never needs adopting. use lance::Dataset; use lance::dataset::transaction::Operation; use super::*; -/// Options for [`Omnigraph::repair`]. -#[derive(Debug, Clone, Copy, Default)] -pub struct RepairOptions { - /// Preview by default. With `confirm`, verified maintenance drift is - /// published to `__manifest`. - pub confirm: bool, - /// Also publish suspicious/unverifiable drift. Requires `confirm`. - pub force: bool, -} - /// Classification of a dataset's graph-manifest/Lance-HEAD state. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[non_exhaustive] @@ -65,20 +55,15 @@ impl std::fmt::Display for RepairClassification { } } -/// What repair did for a backing dataset. +/// What the repair report says about a backing dataset. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[non_exhaustive] pub enum RepairAction { - /// Nothing to do. + /// Nothing to report: no drift, or foreign drift no read or write resolves. NoOp, - /// Drift was reported but not published because this was a preview. + /// Drift the report classified and left in place. Preview, - /// Verified maintenance drift was published to `__manifest`. - Healed, - /// Suspicious/unverifiable drift was published because `force` was set. - Forced, - /// Drift was left untouched because it was not safe to publish without - /// `force`. + /// The table could not be judged; its `error` says why. Refused, } @@ -88,8 +73,6 @@ impl RepairAction { match self { Self::NoOp => "no_op", Self::Preview => "preview", - Self::Healed => "healed", - Self::Forced => "forced", Self::Refused => "refused", } } @@ -101,13 +84,13 @@ impl std::fmt::Display for RepairAction { } } -/// Per-dataset repair outcome. +/// One dataset's row of the repair report. #[derive(Debug, Clone)] #[non_exhaustive] pub struct DatasetRepairStats { /// Qualified graph type key, or `__manifest` for the system dataset. pub type_key: String, - /// Published dataset version before repair. + /// The published dataset version the registration pins. pub published_dataset_version: u64, pub lance_head_version: u64, pub classification: RepairClassification, @@ -116,13 +99,11 @@ pub struct DatasetRepairStats { pub error: Option, } -/// Whole-graph repair outcome. +/// The repair report for the whole graph. #[derive(Debug, Clone)] #[non_exhaustive] pub struct RepairStats { pub datasets: Vec, - /// New graph-manifest version if repair published any dataset pins. - pub graph_manifest_version: Option, } struct ClassificationResult { @@ -132,31 +113,34 @@ struct ClassificationResult { } struct RepairTableTask { - identity: crate::db::manifest::TableIdentity, table_key: String, full_path: String, pinned_data_version: u64, - pinned_native_ref: Option, entry: crate::db::manifest::DatasetEntry, } -pub async fn repair_all_datasets(db: &Omnigraph, options: RepairOptions) -> Result { - if options.force && !options.confirm { - return Err(OmniError::manifest("repair --force requires --confirm")); - } +/// The repair report on `main`, under `maintain` on `main`. +pub(super) async fn repair_report(db: &Omnigraph, actor: Option<&str>) -> Result { + db.enforce( + omnigraph_policy::PolicyAction::Maintain, + &omnigraph_policy::ResourceScope::Branch("main".to_string()), + actor, + )?; + // The report only reads, so every failure is one before any effect. + report_under_gates(db) + .await + .map_err(OmniError::before_effect_if_opaque) +} +async fn report_under_gates(db: &Omnigraph) -> Result { db.ensure_schema_state_valid().await?; - // Repair may adopt physical HEADs into graph authority. Bind the entire - // attempt to one accepted view and revalidate after schema -> main -> table - // gates so concurrent graph movement refuses before publication. + // Bind the whole report to one accepted view, revalidated after the + // schema -> main -> table gates. The accepted catalog, the identity/path + // pairs and every raw Lance read stay under those gates, so a concurrent + // schema apply cannot pair an old dataset path with a replacement's alias, + // and a cleanup cannot remove a version the report is reading. let authority_txn = db.open_write_txn(None).await?; - - // Repair publishes manifest authority, so it joins the canonical writer - // envelope. The accepted catalog, identity/path pairs, raw Lance reads, and - // final publish all remain under schema -> main -> sorted-table gates. This - // prevents a concurrent drop/re-add from pairing the old dataset path with - // the replacement's same public alias and new identity. let _schema_permit = db.write_queue().acquire_schema_shared().await; db.refresh_coordinator_only().await?; let catalog = db.load_accepted_catalog_with_schema_gate_held().await?; @@ -175,41 +159,26 @@ pub async fn repair_all_datasets(db: &Omnigraph, options: RepairOptions) -> Resu .filter_map(|table_key| { let entry = snapshot.dataset(&table_key)?; Some(RepairTableTask { - identity: entry.identity, table_key, full_path: format!("{}/{}", db.root_uri, entry.dataset_path), pinned_data_version: entry.published_dataset_version, - pinned_native_ref: entry.native_dataset_branch.clone(), entry: entry.clone(), }) }) .collect::>(); - if table_tasks.is_empty() { - return Ok(RepairStats { - datasets: Vec::new(), - graph_manifest_version: None, - }); - } - let mut tables = Vec::with_capacity(table_tasks.len()); - let mut updates = Vec::new(); - let mut expected = crate::db::manifest::ExpectedTableVersions::new(); - let mut any_forced = false; - for task in table_tasks { let RepairTableTask { - identity, table_key, full_path, pinned_data_version, - pinned_native_ref, entry, } = task; // `classify_drift` inspects raw Lance transaction history // (`read_transaction_by_version`), a Lance-only maintenance read the // staged-write trait does not surface. The raw borrow is an enumerated, - // read-only escape; repair never takes ownership or moves Lance HEAD. + // read-only escape; the report never takes ownership or moves Lance HEAD. let handle = db.storage().open_dataset_head(&full_path, None).await?; if let Some(row) = judge_against_last_linear_version(&table_key, &entry, &handle)? { tables.push(row); @@ -262,90 +231,18 @@ pub async fn repair_all_datasets(db: &Omnigraph, options: RepairOptions) -> Resu } let classification = classify_drift(ds, pinned_data_version, lance_head_version).await; - let action = match ( - options.confirm, - options.force, - classification.classification, - ) { - (false, _, _) => RepairAction::Preview, - (true, _, RepairClassification::VerifiedMaintenance) => RepairAction::Healed, - (true, true, RepairClassification::Suspicious | RepairClassification::Unverifiable) => { - any_forced = true; - RepairAction::Forced - } - (true, _, RepairClassification::Suspicious | RepairClassification::Unverifiable) => { - RepairAction::Refused - } - (true, _, RepairClassification::NoDrift) => RepairAction::NoOp, - // Reported above, before drift is classified; never adopted. - (true, _, RepairClassification::BlockedPromotion) => RepairAction::Refused, - (true, _, RepairClassification::ForeignDrift) => RepairAction::NoOp, - }; - - if matches!(action, RepairAction::Healed | RepairAction::Forced) { - let state = db.storage().table_state(&full_path, &handle).await?; - updates.push(crate::db::DatasetUpdate { - identity, - type_key: table_key.clone(), - published_dataset_version: state.version, - native_dataset_branch: None, - entity_count: state.row_count, - version_metadata: state - .version_metadata - .with_last_linear_version(entry.version_metadata.last_linear_version()), - }); - expected.insert( - identity, - crate::db::manifest::TableVersionExpectation { - table_key: table_key.clone(), - table_version: pinned_data_version, - native_ref: crate::db::manifest::NativeRefPin::Exact(pinned_native_ref), - }, - ); - } - tables.push(DatasetRepairStats { type_key: table_key, published_dataset_version: pinned_data_version, lance_head_version, classification: classification.classification, - action, + action: RepairAction::Preview, operations: classification.operations, error: classification.error, }); } - let manifest_version = if updates.is_empty() { - None - } else { - let actor = if any_forced { - Some("omnigraph:repair:force") - } else { - Some("omnigraph:repair") - }; - let PublishedSnapshot { - graph_manifest_version: manifest_version, - .. - } = db - .coordinator - .write() - .await - .commit_updates_with_actor_with_expected(&updates, &expected, actor) - .await?; - db.runtime_cache.invalidate_all().await; - if updates - .iter() - .any(|update| update.type_key.starts_with("edge:")) - { - db.invalidate_graph_index().await; - } - Some(manifest_version) - }; - - Ok(RepairStats { - datasets: tables, - graph_manifest_version: manifest_version, - }) + Ok(RepairStats { datasets: tables }) } /// The detached-only rule: the recorded last linear version is the baseline diff --git a/crates/omnigraph/src/db/omnigraph/table_ops.rs b/crates/omnigraph/src/db/omnigraph/table_ops.rs index 68875383a..65b9aa202 100644 --- a/crates/omnigraph/src/db/omnigraph/table_ops.rs +++ b/crates/omnigraph/src/db/omnigraph/table_ops.rs @@ -1,5 +1,6 @@ use super::*; use crate::dataset_index::is_full_text_index; +use crate::db::maintenance::{MaintenanceOperation, MaintenanceRun}; use crate::error::missing_graph_type_at_snapshot; use crate::seams::{decide_seam, fail}; use lance::index::DatasetIndexExt; @@ -35,19 +36,26 @@ pub(super) async fn graph_index_for_resolved( .await } -pub(super) async fn ensure_indices(db: &Omnigraph) -> Result> { +pub(super) async fn ensure_indices_as( + db: &Omnigraph, + actor: Option<&str>, +) -> Result> { let current_branch = db .coordinator .read() .await .current_branch() .map(str::to_string); - ensure_indices_for_branch(db, current_branch.as_deref()).await + ensure_indices_for_branch(db, current_branch.as_deref(), actor).await } -pub(super) async fn ensure_indices_on(db: &Omnigraph, branch: &str) -> Result> { +pub(super) async fn ensure_indices_on_as( + db: &Omnigraph, + branch: &str, + actor: Option<&str>, +) -> Result> { let branch = normalize_branch_name(branch)?; - ensure_indices_for_branch(db, branch.as_deref()).await + ensure_indices_for_branch(db, branch.as_deref(), actor).await } /// The indexes replaced by one successfully published full-text rebuild. @@ -86,18 +94,28 @@ pub(super) async fn rebuild_full_text_indices_on_as( ) -> Result { let branch = normalize_branch_name(branch)?; let public_branch = branch.as_deref().unwrap_or("main"); - db.enforce( - omnigraph_policy::PolicyAction::Change, + // `maintain` is the rebuild's grant; `change`, its grant before + // `maintain` existed, still admits it so existing policies keep working. + db.enforce_any( + &[ + omnigraph_policy::PolicyAction::Maintain, + omnigraph_policy::PolicyAction::Change, + ], &omnigraph_policy::ResourceScope::Branch(public_branch.to_string()), actor, )?; + let run = db + .enter_maintenance(MaintenanceOperation::FullTextRebuild, Some(public_branch)) + .await?; let outcome = maintain_indices_for_branch( db, branch.as_deref(), IndexMaintenanceMode::RebuildFullText, actor, + &run, ) - .await?; + .await + .map_err(|error| run.classify(error))?; Ok(FullTextIndexRebuildResult { branch: public_branch.to_string(), graph_commit_id: outcome.graph_commit_id, @@ -186,13 +204,25 @@ pub(super) async fn failpoint_publish_table_head_without_index_rebuild_for_test( .await } -pub(super) async fn ensure_indices_for_branch( +/// The index writer on `branch` (`None` is main), under `maintain` on it. +async fn ensure_indices_for_branch( db: &Omnigraph, branch: Option<&str>, + actor: Option<&str>, ) -> Result> { + let public_branch = branch.unwrap_or("main"); + db.enforce( + omnigraph_policy::PolicyAction::Maintain, + &omnigraph_policy::ResourceScope::Branch(public_branch.to_string()), + actor, + )?; + let run = db + .enter_maintenance(MaintenanceOperation::IndexBuild, Some(public_branch)) + .await?; Ok( - maintain_indices_for_branch(db, branch, IndexMaintenanceMode::Ensure, None) - .await? + maintain_indices_for_branch(db, branch, IndexMaintenanceMode::Ensure, actor, &run) + .await + .map_err(|error| run.classify(error))? .pending, ) } @@ -224,12 +254,18 @@ struct IndexTarget { table_branch: Option, } +/// The index writer holding the graph's maintenance lane. Its cancel points +/// come between tables and before the publication; a cancel drops an index +/// build in flight, whose segment files no manifest names, so the collector +/// reclaims them as orphans under its age rule. async fn maintain_indices_for_branch( db: &Omnigraph, branch: Option<&str>, mode: IndexMaintenanceMode, actor: Option<&str>, + run: &MaintenanceRun, ) -> Result { + let budget = db.maintenance_budget(); // Install this handle's pending schema contract, if any, before capturing // the index plan's base. The under-gate revalidation below closes the race // between this capture and the first table effect. @@ -264,6 +300,7 @@ async fn maintain_indices_for_branch( let mut __dst_nk1: Vec<_> = catalog.node_types.keys().collect(); __dst_nk1.sort(); for type_name in __dst_nk1 { + run.check()?; let table_key = format!("node:{}", type_name); let Some(entry) = snapshot.dataset(&table_key) else { continue; @@ -298,6 +335,7 @@ async fn maintain_indices_for_branch( let mut __dst_ek1: Vec<_> = catalog.edge_types.keys().collect(); __dst_ek1.sort(); for edge_name in __dst_ek1 { + run.check()?; let table_key = format!("edge:{}", edge_name); let Some(entry) = snapshot.dataset(&table_key) else { continue; @@ -336,11 +374,14 @@ async fn maintain_indices_for_branch( // for the fork. for target in &targets { if let Some(ds) = existing_targets.get(&target.table_key) { + run.check()?; + // Index builds write segment files: durable, and named by no + // manifest until the batch publishes. + run.staged(); let work = &work_by_table[&target.table_key]; - let staged = db - .storage() - .stage_create_indices(ds, &work.specs) - .await + let staged = run + .unless_cancelled(db.storage().stage_create_indices(ds, &work.specs, &budget)) + .await? .map_err(|error| { error.with_context(format!( "stage index batch on {} ({:?})", @@ -439,6 +480,7 @@ async fn maintain_indices_for_branch( let mut updates = Vec::with_capacity(targets.len()); for target in &targets { + run.check()?; let ds = existing_targets.remove(&target.table_key).ok_or_else(|| { OmniError::manifest_internal(format!( "missing index target for table '{}'", @@ -486,6 +528,9 @@ async fn maintain_indices_for_branch( } fail(&ENSURE_INDICES_POST_PHASE_B_PRE_MANIFEST_COMMIT)?; + // The last cancel point: past it the batch publishes or fails. + run.check()?; + run.publishing(); let published = commit_updates_on_branch_with_expected( db, active_branch.as_deref(), @@ -497,6 +542,7 @@ async fn maintain_indices_for_branch( Publication::Publish, ) .await?; + run.published(); Some(published.graph_commit_id) }; @@ -1063,7 +1109,7 @@ pub(super) async fn build_indices_on_dataset_for_catalog( let staged = db .storage() - .stage_create_indices(ds, &work.specs) + .stage_create_indices(ds, &work.specs, &db.maintenance_budget()) .await .map_err(|error| { error.with_context(format!( diff --git a/crates/omnigraph/src/db/upgrade/tests.rs b/crates/omnigraph/src/db/upgrade/tests.rs index 39ab4cb9d..30fa83bb8 100644 --- a/crates/omnigraph/src/db/upgrade/tests.rs +++ b/crates/omnigraph/src/db/upgrade/tests.rs @@ -2752,7 +2752,7 @@ async fn cleanup_after_upgrade_with_keep_four_and_older_than() { .collect(); assert_eq!(links.len(), 1, "{links:?}"); for (round, options) in [week, keep(4)].into_iter().enumerate() { - let stats = graph.cleanup(options).await.unwrap(); + let stats = graph.cleanup_as(options, None).await.unwrap(); let reclaimed = |table_key: &str| { if round == 0 && table_key == PERSON { links.len() as u64 @@ -2800,7 +2800,7 @@ async fn merge_with_legacy_base_pins_and_collector_keeps_it() { "the merge base of feature and main pins Person version {}: {roots:?}", source.a_root ); - let stats = graph.cleanup(keep(1)).await.unwrap(); + let stats = graph.cleanup_as(keep(1), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); session(&graph) @@ -2897,7 +2897,7 @@ async fn leftover_merge_input_tag_on_bookkeeping_version_resolves() { roots.is_superset(&[source.b_root, source.c_root].into()), "{roots:?}" ); - let stats = graph.cleanup(keep(1)).await.unwrap(); + let stats = graph.cleanup_as(keep(1), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); assert_eq!( open(root, None) diff --git a/crates/omnigraph/src/db/write_queue.rs b/crates/omnigraph/src/db/write_queue.rs index b506074e6..beb34c4ee 100644 --- a/crates/omnigraph/src/db/write_queue.rs +++ b/crates/omnigraph/src/db/write_queue.rs @@ -393,6 +393,9 @@ pub(crate) struct WriteQueueManager { /// The graph-global shared/exclusive schema gate; see /// [`SchemaGateSlot`]. schema_gate: Arc, + /// The root's maintenance lane: one maintenance run at a time in this + /// process. Taken before any gate here and released after them. + maintenance_lane: Arc, } impl WriteQueueManager { @@ -418,6 +421,10 @@ impl WriteQueueManager { manager } + pub(crate) fn maintenance_lane(&self) -> &Arc { + &self.maintenance_lane + } + /// Get-or-create the per-key queue and clone its Arc. fn slot(&self, key: &TableQueueKey) -> Arc { let mut map = self.queues.lock().expect("write queue map poisoned"); diff --git a/crates/omnigraph/src/engine/plan_source.rs b/crates/omnigraph/src/engine/plan_source.rs index fcd88b8a7..f4bb53522 100644 --- a/crates/omnigraph/src/engine/plan_source.rs +++ b/crates/omnigraph/src/engine/plan_source.rs @@ -926,7 +926,7 @@ query people() { match { $p: Person } return { count($p) as n } } ) .await .unwrap(); - session.ensure_indices().await.unwrap(); + session.ensure_indices_as(None).await.unwrap(); let settings = SessionSettings::default(); let (view, catalog) = db .capture_read_view(ReadTarget::branch("main")) diff --git a/crates/omnigraph/src/runtime_cache.rs b/crates/omnigraph/src/runtime_cache.rs index be6267f34..3e3c00b6e 100644 --- a/crates/omnigraph/src/runtime_cache.rs +++ b/crates/omnigraph/src/runtime_cache.rs @@ -648,7 +648,7 @@ edge Likes: Person -> Person {} db.load_jsonl(DATA, crate::loader::LoadMode::Overwrite) .await .unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); // Fresh handle: cold in-memory caches, the artifact stays on the store. let db = crate::db::Omnigraph::open(dir.path().to_str().unwrap()) .await diff --git a/crates/omnigraph/src/storage_layer.rs b/crates/omnigraph/src/storage_layer.rs index 640fb98cc..736ee8ed5 100644 --- a/crates/omnigraph/src/storage_layer.rs +++ b/crates/omnigraph/src/storage_layer.rs @@ -886,6 +886,7 @@ pub trait TableStorage: sealed::Sealed + Send + Sync + Debug { &self, snapshot: &SnapshotHandle, specs: &[IndexBuildSpec], + budget: &crate::db::maintenance::MaintenanceBudget, ) -> Result; // ── Index presence (reads, no HEAD advance) ────────────────────── @@ -1493,8 +1494,9 @@ impl TableStorage for TableStore { &self, snapshot: &SnapshotHandle, specs: &[IndexBuildSpec], + budget: &crate::db::maintenance::MaintenanceBudget, ) -> Result { - TableStore::stage_create_indices(self, snapshot.dataset(), specs) + TableStore::stage_create_indices(self, snapshot.dataset(), specs, budget) .await .map(StagedHandle::new) } diff --git a/crates/omnigraph/src/table_store.rs b/crates/omnigraph/src/table_store.rs index a36b5da14..889dae784 100644 --- a/crates/omnigraph/src/table_store.rs +++ b/crates/omnigraph/src/table_store.rs @@ -2918,7 +2918,6 @@ impl TableStore { continue; }; let mut ds_clone = ds.clone(); - let columns = [column.as_str()]; let built = if item.vector { // Keep the index's partition count: the engine builds one // partition, but a partitioned index (Lance's split, or an @@ -2926,20 +2925,28 @@ impl TableStore { let partitions = Self::vector_partition_count(ds, &item.name).await; let params = lance::index::vector::VectorIndexParams::ivf_flat(partitions, MetricType::L2); - ds_clone - .create_index_builder(&columns, IndexType::Vector, ¶ms) - .name(item.name.clone()) - .replace(true) - .execute_uncommitted() - .await + let (name, column) = (item.name.clone(), column.clone()); + crate::db::maintenance::off_serving_runtime(async move { + ds_clone + .create_index_builder(&[column.as_str()], IndexType::Vector, ¶ms) + .name(name) + .replace(true) + .execute_uncommitted() + .await + }) + .await } else { let params = ScalarIndexParams::default(); - ds_clone - .create_index_builder(&columns, IndexType::BTree, ¶ms) - .name(item.name.clone()) - .replace(true) - .execute_uncommitted() - .await + let (name, column) = (item.name.clone(), column.clone()); + crate::db::maintenance::off_serving_runtime(async move { + ds_clone + .create_index_builder(&[column.as_str()], IndexType::BTree, ¶ms) + .name(name) + .replace(true) + .execute_uncommitted() + .await + }) + .await }; let new_idx = match built { Ok(index) => index, @@ -4348,7 +4355,15 @@ impl TableStore { if let (true, Some(rows)) = (has_blob_columns, task.options.batch_size) { crate::instrumentation::record_compaction_blob_batch(rows); } - results.push(task.execute(ds).await.map_err(OmniError::storage)?); + // Lance decodes and re-encodes the task's rows in tasks it spawns + // on the runtime that polls it. + let dataset = ds.clone(); + let result = + crate::db::maintenance::off_serving_runtime( + async move { task.execute(&dataset).await }, + ) + .await; + results.push(result.map_err(OmniError::storage)?); } if results.iter().any(|result| result.row_addrs.is_some()) { return Err(OmniError::manifest_internal(format!( @@ -5025,10 +5040,14 @@ impl TableStore { /// This intentionally covers OmniGraph's current one-segment full-table /// vector shape. Lance's generic multi-segment commit helper remains an /// inline-commit API and is not used here. + /// Each build runs on the maintenance runtime + /// ([`crate::db::maintenance::off_serving_runtime`]), and a full-text + /// build takes its memory and workers from `budget`. pub async fn stage_create_indices( &self, ds: &Dataset, specs: &[IndexBuildSpec], + budget: &crate::db::maintenance::MaintenanceBudget, ) -> Result { if specs.is_empty() { return Err(OmniError::manifest_internal( @@ -5062,33 +5081,46 @@ impl TableStore { let mut new_idx = match spec { IndexBuildSpec::BTree { column, name } => { let params = ScalarIndexParams::default(); - let columns = [column.as_str()]; - let mut builder = - ds_clone.create_index_builder(&columns, IndexType::BTree, ¶ms); - // Same-snapshot builders cannot see each other's new - // default names; companions need explicit distinct names - // even with Lance 11's sequential collision avoidance. - if let Some(name) = name { - builder = builder.name(name.clone()); - } - builder.replace(true).execute_uncommitted().await + let (column, name) = (column.clone(), name.clone()); + crate::db::maintenance::off_serving_runtime(async move { + let columns = [column.as_str()]; + let mut builder = + ds_clone.create_index_builder(&columns, IndexType::BTree, ¶ms); + // Same-snapshot builders cannot see each other's new + // default names; companions need explicit distinct + // names even with Lance 11's sequential collision + // avoidance. + if let Some(name) = name { + builder = builder.name(name); + } + builder.replace(true).execute_uncommitted().await + }) + .await } IndexBuildSpec::FullText { column } => { - let params = InvertedIndexParams::default(); - ds_clone - .create_index_builder(&[column.as_str()], IndexType::Inverted, ¶ms) - .replace(true) - .execute_uncommitted() - .await + let params = budget.full_text_params(); + let column = column.clone(); + crate::db::maintenance::off_serving_runtime(async move { + ds_clone + .create_index_builder(&[column.as_str()], IndexType::Inverted, ¶ms) + .replace(true) + .execute_uncommitted() + .await + }) + .await } IndexBuildSpec::Vector { column } => { let params = lance::index::vector::VectorIndexParams::ivf_flat(1, MetricType::L2); - let new_idx = ds_clone - .create_index_builder(&[column.as_str()], IndexType::Vector, ¶ms) - .replace(true) - .execute_uncommitted() - .await; + let column = column.clone(); + let new_idx = crate::db::maintenance::off_serving_runtime(async move { + ds_clone + .create_index_builder(&[column.as_str()], IndexType::Vector, ¶ms) + .replace(true) + .execute_uncommitted() + .await + }) + .await; if new_idx.is_ok() { vector_builds += 1; } diff --git a/crates/omnigraph/src/table_store/staged_tests.rs b/crates/omnigraph/src/table_store/staged_tests.rs index 8ebcff7d5..2a4b74d1e 100644 --- a/crates/omnigraph/src/table_store/staged_tests.rs +++ b/crates/omnigraph/src/table_store/staged_tests.rs @@ -476,6 +476,7 @@ async fn keyed_upsert_forces_filter_route_and_preserves_conflict_metadata() { column: "id".to_string(), name: None, }], + &crate::db::MaintenanceBudget::DEFAULT, ) .await .unwrap(); @@ -536,6 +537,7 @@ async fn known_present_update_is_update_only_and_fails_closed_on_missing_ids() { column: "id".to_string(), name: None, }], + &crate::db::MaintenanceBudget::DEFAULT, ) .await .unwrap(); @@ -960,6 +962,7 @@ async fn proven_strict_insert_pins_update_shape_and_leaves_new_fragments_unindex column: "id".to_string(), name: None, }], + &crate::db::MaintenanceBudget::DEFAULT, ) .await .unwrap(); @@ -2971,6 +2974,7 @@ async fn stage_create_indices_batches_mixed_types_into_one_exact_commit() { column: "embedding".to_string(), }, ], + &crate::db::MaintenanceBudget::DEFAULT, ) .await .unwrap(); @@ -3130,6 +3134,7 @@ async fn saved_lance10_full_text_index_requires_rebuild_and_preserves_history() &[IndexBuildSpec::FullText { column: "text".into(), }], + &crate::db::MaintenanceBudget::DEFAULT, ) .await .unwrap(); diff --git a/crates/omnigraph/tests/bound_storage.rs b/crates/omnigraph/tests/bound_storage.rs index 4641a513d..a4f989498 100644 --- a/crates/omnigraph/tests/bound_storage.rs +++ b/crates/omnigraph/tests/bound_storage.rs @@ -303,11 +303,14 @@ async fn bound_handle_workload_child() { .contains("Feature") ); - db.optimize().await.unwrap(); - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(2), - older_than: None, - }) + db.optimize_as(None).await.unwrap(); + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(2), + older_than: None, + }, + None, + ) .await .unwrap(); let reopened = session(Omnigraph::open_with_storage(&uri, storage()).await.unwrap()); diff --git a/crates/omnigraph/tests/detached_commit_matrix.rs b/crates/omnigraph/tests/detached_commit_matrix.rs index d0a5c2792..b6bc2bd3d 100644 --- a/crates/omnigraph/tests/detached_commit_matrix.rs +++ b/crates/omnigraph/tests/detached_commit_matrix.rs @@ -414,15 +414,18 @@ fn rfc0067_matrix_child_process() { .block_on(async move { let db = helpers::session(Omnigraph::open(&uri).await.unwrap()); let outcome: omnigraph::error::Result<()> = match op.as_str() { - "cleanup" => db.cleanup(reclaim_everything()).await.map(|_| ()), + "cleanup" => db.cleanup_as(reclaim_everything(), None).await.map(|_| ()), "cleanup_keep" => db - .cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .map(|_| ()), - "ensure_indices" => db.ensure_indices().await.map(|_| ()), + "ensure_indices" => db.ensure_indices_as(None).await.map(|_| ()), "merge" => db .branch_merge(&format!("src_{name}"), "main") .await @@ -437,9 +440,12 @@ fn rfc0067_matrix_child_process() { .unwrap(); db.apply_prepared_schema_as(&intent, None).await.map(|_| ()) } - "optimize" => db.optimize().await.map(|_| ()), + "optimize" => db.optimize_as(None).await.map(|_| ()), "load" => load_two(&db, &name).await, - "fts_rebuild" => db.rebuild_full_text_indices_on("main").await.map(|_| ()), + "fts_rebuild" => db + .rebuild_full_text_indices_on_as("main", None) + .await + .map(|_| ()), "system_column_upgrade" => db .upgrade_system_columns(upgrade_execute()) .await @@ -696,7 +702,7 @@ async fn run_cell( ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); } // A clear cell clears a photo a put stored first. if writer == Writer::BlobClear { @@ -736,8 +742,10 @@ async fn run_cell( let outcome: omnigraph::error::Result<()> = match writer { Writer::Insert => insert(&db, &write_name).await, Writer::MultiTable => insert_and_friend(&db, &write_name).await, - Writer::Cleanup => Box::pin(db.cleanup(reclaim_everything())).await.map(|_| ()), - Writer::EnsureIndices => db.ensure_indices().await.map(|_| ()), + Writer::Cleanup => Box::pin(db.cleanup_as(reclaim_everything(), None)) + .await + .map(|_| ()), + Writer::EnsureIndices => db.ensure_indices_as(None).await.map(|_| ()), Writer::Merge => db .branch_merge(&format!("src_{write_name}"), "main") .await @@ -747,9 +755,12 @@ async fn run_cell( .apply_prepared_schema_as(prepared.as_ref().unwrap(), None) .await .map(|_| ()), - Writer::Optimize => db.optimize().await.map(|_| ()), + Writer::Optimize => db.optimize_as(None).await.map(|_| ()), Writer::Load => load_two(&db, &write_name).await, - Writer::FtsRebuild => db.rebuild_full_text_indices_on("main").await.map(|_| ()), + Writer::FtsRebuild => db + .rebuild_full_text_indices_on_as("main", None) + .await + .map(|_| ()), Writer::SystemColumnUpgrade => db .upgrade_system_columns(upgrade_execute()) .await @@ -759,11 +770,11 @@ async fn run_cell( }; acknowledged = outcome.is_ok(); if let Err(error) = outcome { - // A data writer that fails after a detached effect and before - // its publication never reports an outcome it cannot - // establish: the injected refusal is typed, and an opaque - // failure in the same window carries `Unpublished`. The - // maintenance and schema writers do not carry that mark yet. + // A data or maintenance writer that fails after a detached + // effect and before its publication never reports an outcome + // it cannot establish: the injected refusal is typed, and an + // opaque failure in the same window carries `Unpublished`. The + // schema writers do not carry that mark yet. if matches!( writer, Writer::Insert @@ -772,6 +783,10 @@ async fn run_cell( | Writer::Merge | Writer::BlobPut | Writer::BlobClear + | Writer::Cleanup + | Writer::EnsureIndices + | Writer::Optimize + | Writer::FtsRebuild ) { assert_ne!( error.completion_evidence(), @@ -879,7 +894,7 @@ async fn run_cell( } Recovery::Cleanup => { let fresh = helpers::session(Omnigraph::open(&root).await.unwrap()); - Box::pin(fresh.cleanup(reclaim_everything())) + Box::pin(fresh.cleanup_as(reclaim_everything(), None)) .await .unwrap_or_else(|error| panic!("{cell}: recovery cleanup failed: {error}")); } @@ -974,7 +989,7 @@ async fn run_cell( if writer == Writer::Optimize && recovery != Recovery::ReadOnly { let fresh = helpers::session(Omnigraph::open(&root).await.unwrap()); for run in 1..=2 { - fresh.optimize().await.unwrap_or_else(|error| { + fresh.optimize_as(None).await.unwrap_or_else(|error| { panic!("{cell}: optimize run {run} after recovery failed: {error}") }); assert_pin_detached_and_head_unmoved(&cell, &fresh, &person_uri, head_before).await; @@ -984,12 +999,12 @@ async fn run_cell( if writer == Writer::EnsureIndices && recovery != Recovery::ReadOnly { let fresh = helpers::session(Omnigraph::open(&root).await.unwrap()); fresh - .ensure_indices() + .ensure_indices_as(None) .await .unwrap_or_else(|error| panic!("{cell}: index pass after recovery failed: {error}")); assert_pin_detached_and_head_unmoved(&cell, &fresh, &person_uri, head_before).await; fresh - .ensure_indices() + .ensure_indices_as(None) .await .unwrap_or_else(|error| panic!("{cell}: second index pass failed: {error}")); drop(fresh); @@ -998,7 +1013,7 @@ async fn run_cell( let fresh = helpers::session(Omnigraph::open(&root).await.unwrap()); for run in 1..=2 { let rebuilt = fresh - .rebuild_full_text_indices_on("main") + .rebuild_full_text_indices_on_as("main", None) .await .unwrap_or_else(|error| { panic!("{cell}: full-text rebuild run {run} after recovery failed: {error}") diff --git a/crates/omnigraph/tests/failpoints.rs b/crates/omnigraph/tests/failpoints.rs index d0e1a6822..60d8e4cc1 100644 --- a/crates/omnigraph/tests/failpoints.rs +++ b/crates/omnigraph/tests/failpoints.rs @@ -322,7 +322,7 @@ fn rfc023_external_writer_process() { } "collector_advance_merge_source" => { db.load("source", &payload, mode).await.unwrap(); - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); } "collector_retired" => { @@ -514,7 +514,7 @@ async fn branch_start_refuses_a_commit_an_interrupted_cleanup_emptied() { { let _fp = catalog::CLEANUP_SWEEP_PRE_MANIFEST_DELETE.fire_always(); - let _ = db.cleanup(keep_one()).await; + let _ = db.cleanup_as(keep_one(), None).await; } assert!( detached_versions(&person_uri).await.contains(&eve_person), @@ -546,7 +546,7 @@ async fn branch_start_refuses_a_commit_an_interrupted_cleanup_emptied() { assert_eq!(db.branch_list().await.unwrap(), vec!["main".to_string()]); assert_eq!(version_main(&db).await.unwrap(), main_version); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); let person = stats .iter() .find(|row| row.type_key == "node:Person") @@ -623,7 +623,7 @@ async fn branch_delete_cleanup_failure_converges_on_retry() { ); { let _fp = catalog::CLEANUP_TABLE_GC.fire_always(); - main.cleanup(keep_one()).await.unwrap(); + main.cleanup_as(keep_one(), None).await.unwrap(); } assert!( detached_versions(&person_uri) @@ -631,7 +631,7 @@ async fn branch_delete_cleanup_failure_converges_on_retry() { .contains(&former_staging), "failed cleanup preserves the deleted branch's staging" ); - main.cleanup(keep_one()).await.unwrap(); + main.cleanup_as(keep_one(), None).await.unwrap(); assert!( !detached_versions(&person_uri) .await @@ -723,7 +723,7 @@ async fn recreate_over_a_deleted_branch_writes_without_cleanup() { "writes do not collect the former staging" ); assert!(remaining.contains(&second_staging)); - main.cleanup(keep_one()).await.unwrap(); + main.cleanup_as(keep_one(), None).await.unwrap(); let remaining = detached_versions(&person_uri).await; assert!(!remaining.contains(&first_staging)); assert!( @@ -780,7 +780,7 @@ async fn branch_delete_acknowledges_with_staging_awaiting_cleanup() { .contains(&former_staging), "delete and reopen defer collection of the branch's staging" ); - main.cleanup(keep_one()).await.unwrap(); + main.cleanup_as(keep_one(), None).await.unwrap(); assert!( !detached_versions(&person_uri) .await @@ -850,7 +850,7 @@ async fn branch_recreate_completes_while_old_staging_awaits_cleanup() { let remaining = detached_versions(&person_uri).await; assert!(remaining.contains(&first_staging)); assert!(remaining.contains(&second_staging)); - main.cleanup(keep_one()).await.unwrap(); + main.cleanup_as(keep_one(), None).await.unwrap(); let remaining = detached_versions(&person_uri).await; assert!(!remaining.contains(&first_staging)); assert!(remaining.contains(&second_staging)); @@ -923,10 +923,13 @@ async fn branch_write_ignores_unavailable_cleanup_classifier() { helpers::count_rows_branch(&db, "feature", "node:Person").await, 5 ); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); let branches = lance::Dataset::open(&person_uri) @@ -970,10 +973,13 @@ async fn cleanup_isolates_single_table_failure() { // One table's version GC fails once; the sweep must isolate it. let _fp = catalog::CLEANUP_TABLE_GC.fire_once_at(1); let stats = db - .cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .expect("a single table's GC failure must not abort cleanup"); @@ -1024,10 +1030,13 @@ async fn cleanup_isolates_reconcile_failure() { // not abort. { let _fp = catalog::CLEANUP_RECONCILE_FORK.fire_once_at(1); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .expect("a reconcile force-delete failure must not abort cleanup"); } @@ -1044,10 +1053,13 @@ async fn cleanup_isolates_reconcile_failure() { ); } // A second cleanup with no injected failure converges. - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); { @@ -1096,10 +1108,13 @@ async fn reconcile_skips_fork_when_fresh_recheck_is_unavailable_then_converges() // branch is live but unreadable) → cleanup must SKIP it, not delete. { let _fp = catalog::CLASSIFY_FRESH_READ.fire_always(); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); let ds = lance::Dataset::open(&person_uri).await.unwrap(); @@ -1114,10 +1129,13 @@ async fn reconcile_skips_fork_when_fresh_recheck_is_unavailable_then_converges() } // Read succeeds now → cleanup confirms the orphan and reclaims it (converges). - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); { @@ -2666,7 +2684,7 @@ async fn merge_target_delete_waits_and_source_delete_does_not_issue_643() { .unwrap() .unwrap_or_else(|error| panic!("deleting the target after the merge: {error}")); } else { - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert_eq!( helpers::count_rows_branch(&db, "t", "node:Person").await, @@ -3059,7 +3077,7 @@ async fn cancelled_merge_leaves_the_target_whole_issue_643() { ) .await .unwrap_or_else(|error| panic!("{seam}: a later write on the target: {error}")); - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!( rows.iter().all(|row| row.error.is_none()), "{seam}: {rows:?}" @@ -3705,7 +3723,7 @@ async fn ensure_indices_stage_btree_failure_leaves_existing_tables_writable() { &catalog::ENSURE_INDICES_POST_STAGE_PRE_COMMIT_BTREE, ); let writer_a_db = std::sync::Arc::clone(&db); - let writer_a = tokio::spawn(async move { writer_a_db.ensure_indices().await }); + let writer_a = tokio::spawn(async move { writer_a_db.ensure_indices_as(None).await }); rendezvous.wait_until_reached().await; db.load( @@ -4043,15 +4061,9 @@ async fn schema_apply_retry_preserves_the_original_empty_create() { assert_eq!(entry.published_dataset_version, if changed { 2 } else { 1 }); assert_eq!(entry.version_metadata.staged_version().is_some(), changed); assert_eq!(entry.version_metadata.last_linear_version(), Some(1)); - recovered - .repair(omnigraph::db::RepairOptions { - confirm: true, - force: false, - }) - .await - .unwrap(); - recovered.ensure_indices().await.unwrap(); - let cleaned = recovered.cleanup(keep_one()).await.unwrap(); + recovered.repair_report_as(None).await.unwrap(); + recovered.ensure_indices_as(None).await.unwrap(); + let cleaned = recovered.cleanup_as(keep_one(), None).await.unwrap(); assert!(cleaned.iter().all(|row| row.error.is_none()), "{cleaned:?}"); drop(recovered); let reopened = helpers::session(Omnigraph::open(&uri).await.unwrap()); @@ -4475,7 +4487,7 @@ async fn optimize_pre_publish_failure_leaves_no_residue() { .version; { let _failpoint = catalog::OPTIMIZE_POST_PHASE_B_PRE_MANIFEST_COMMIT.fire_always(); - let err = db.optimize().await.unwrap_err(); + let err = db.optimize_as(None).await.unwrap_err(); assert!( err.to_string().contains( "injected failpoint triggered: optimize.post_phase_b_pre_manifest_commit" @@ -4502,7 +4514,7 @@ async fn optimize_pre_publish_failure_leaves_no_residue() { assert_eq!(helpers::count_rows(&db, "node:Person").await, 4); assert_eq!(helpers::count_rows(&db, "node:Company").await, 4); let stats = db - .optimize() + .optimize_as(None) .await .expect("the retry re-plans from the pins"); assert!( @@ -4533,7 +4545,7 @@ async fn optimize_lost_publish_acknowledgement_leaves_published_detached_pins() let db = helpers::session(Omnigraph::open(&uri).await.unwrap()); let before = person_head_and_published(&db, "main").await; let _failpoint = catalog::GRAPH_PUBLISH_AFTER_MANIFEST_COMMIT.fire_once_at(1); - db.optimize() + db.optimize_as(None) .await .expect_err("a lost publish acknowledgement surfaces as an error"); before @@ -4549,7 +4561,7 @@ async fn optimize_lost_publish_acknowledgement_leaves_published_detached_pins() before.1 ); assert_person_pin_detached(&recovered, "main", before.0).await; - let stats = recovered.optimize().await.unwrap(); + let stats = recovered.optimize_as(None).await.unwrap(); assert!( stats .iter() @@ -4585,7 +4597,7 @@ node Embedding { ) .await .unwrap(); - let pending = db.ensure_indices().await.unwrap(); + let pending = db.ensure_indices_as(None).await.unwrap(); assert!( pending .iter() @@ -4598,7 +4610,7 @@ node Embedding { ) .await .unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let snapshot = db.snapshot_of(ReadTarget::branch("main")).await.unwrap(); let embedding = snapshot.open_dataset("node:Embedding").await.unwrap(); assert!(embedding.has_unindexed_fragments().await.unwrap()); @@ -4635,7 +4647,7 @@ node Embedding { .version; { let _failpoint = catalog::OPTIMIZE_POST_PHASE_B_PRE_MANIFEST_COMMIT.fire_once_at(1); - let error = db.optimize().await.unwrap_err(); + let error = db.optimize_as(None).await.unwrap_err(); assert!( !matches!(error, OmniError::RecoveryRequired { .. }), "{error}" @@ -4663,7 +4675,7 @@ node Embedding { ); drop(db); let db = helpers::session(Omnigraph::open(&uri).await.unwrap()); - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); let work = stats .iter() .find(|stat| stat.type_key == "node:Work") @@ -4724,7 +4736,7 @@ async fn optimize_partial_table_effect_leaves_no_residue() { .version() .version; let _failpoint = catalog::OPTIMIZE_POST_TABLE_EFFECT.fire_once_at(1); - let error = db.optimize().await.unwrap_err(); + let error = db.optimize_as(None).await.unwrap_err(); assert!( !matches!(error, OmniError::RecoveryRequired { .. }), "a partial batch before publication needs no recovery: {error}" @@ -4768,7 +4780,7 @@ async fn optimize_partial_table_effect_leaves_no_residue() { recovered.list_commits(Some("main")).await.unwrap().len(), commits_before ); - let stats = recovered.optimize().await.unwrap(); + let stats = recovered.optimize_as(None).await.unwrap(); assert_eq!( stats .iter() @@ -4796,6 +4808,206 @@ async fn seed_optimize_race_graph(dir: &tempfile::TempDir) { helpers::commit_many(&seed, 4).await; } +/// Detached manifests per table of the test schema on main. +async fn detached_versions_by_table( + db: &Omnigraph, +) -> std::collections::BTreeMap<&'static str, std::collections::BTreeSet> { + let snapshot = db.snapshot_of(ReadTarget::branch("main")).await.unwrap(); + let mut versions = std::collections::BTreeMap::new(); + for table_key in ["edge:Knows", "edge:WorksAt", "node:Company", "node:Person"] { + let Some(entry) = snapshot.dataset(table_key) else { + continue; + }; + let uri = format!( + "{}/{}", + db.uri().trim_end_matches('/'), + entry.dataset_path.trim_start_matches('/') + ); + versions.insert(table_key, detached_versions(&uri).await); + } + versions +} + +/// A cancel of the graph's maintenance lane stops optimize at its next cancel +/// point, here after a table's first detached commit and before the batch +/// publishes. The run fails with the typed cancellation carrying +/// `Unpublished`; the graph head and every pin are unchanged; what the run +/// staged is detached commits no `__manifest` version names, and once the +/// head moves a cleanup reclaims them. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +#[serial] +async fn optimize_cancelled_mid_table_leaves_only_staging_cleanup_reclaims() { + let _scenario = FailScenario::setup(); + let dir = tempfile::tempdir().unwrap(); + let uri = dir.path().to_str().unwrap().to_string(); + seed_optimize_race_graph(&dir).await; + let db = Arc::new(helpers::session(Omnigraph::open(&uri).await.unwrap())); + let head_before = branch_head_commit_id(dir.path(), "main").await.unwrap(); + let pins_before = pinned_version(&db, "main", "node:Person").await; + let staged_before = detached_versions_by_table(&db).await; + + let parked = helpers::failpoint::Rendezvous::park_first(&catalog::OPTIMIZE_POST_TABLE_EFFECT); + let optimizer = Arc::clone(&db); + let optimize = tokio::spawn(async move { optimizer.optimize_as(None).await }); + parked.wait_until_reached().await; + assert_eq!( + db.maintenance_in_progress(), + Some(omnigraph::db::MaintenanceInProgress { + operation: omnigraph::db::MaintenanceOperation::Optimize, + branch: Some("main".to_string()), + }), + "optimize holds the graph's maintenance lane" + ); + db.cancel_maintenance(); + parked.release(); + let error = tokio::time::timeout(std::time::Duration::from_secs(30), optimize) + .await + .expect("a cancelled optimize stops at its next cancel point") + .unwrap() + .expect_err("a cancelled optimize publishes nothing"); + drop(parked); + assert_eq!( + error.completion_evidence(), + Some(CompletionEvidence::Unpublished), + "{error}" + ); + assert!( + matches!( + error.into_completion_evidence().0, + OmniError::MaintenanceCancelled { + operation: "optimize" + } + ), + "the cancellation is typed" + ); + assert_eq!(db.maintenance_in_progress(), None, "the lane is released"); + assert_eq!( + branch_head_commit_id(dir.path(), "main").await.unwrap(), + head_before, + "the graph head did not move" + ); + assert_eq!( + pinned_version(&db, "main", "node:Person").await, + pins_before + ); + let staged_after = detached_versions_by_table(&db).await; + let stranded = staged_after + .iter() + .map(|(table, versions)| (*table, versions - &staged_before[table])) + .filter(|(_, versions)| !versions.is_empty()) + .collect::>(); + assert!( + !stranded.is_empty(), + "the cancelled table's detached commit is left as staging" + ); + + // The staging recorded the head it was planned on; once the head moves + // it is dead, and cleanup reclaims it by its publication authority. + insert_person(&db, "main", "after-cancel").await; + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); + assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); + let reclaimed = detached_versions_by_table(&db).await; + for (table, versions) in &stranded { + assert!( + versions.is_disjoint(&reclaimed[table]), + "{table}: the cancelled run's staging {versions:?} is reclaimed: {:?}", + reclaimed[table] + ); + } + db.optimize_as(None) + .await + .expect("a run after the cancel optimizes"); +} + +/// The graph's maintenance lane runs one maintenance operation at a time in +/// this process: a cleanup on another handle for the same root waits behind +/// an optimize instead of running beside it. A cancel stops both: the +/// waiting cleanup before any effect, the optimize at its next cancel point, +/// still before any effect. A run that starts after the cancel is not +/// affected. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +#[serial] +async fn maintenance_runs_on_one_root_take_turns_and_a_cancel_stops_the_waiter() { + let _scenario = FailScenario::setup(); + let dir = tempfile::tempdir().unwrap(); + let uri = dir.path().to_str().unwrap().to_string(); + seed_optimize_race_graph(&dir).await; + let optimizing = Arc::new(helpers::session(Omnigraph::open(&uri).await.unwrap())); + let cleaning = Arc::new(helpers::session(Omnigraph::open(&uri).await.unwrap())); + let head_before = branch_head_commit_id(dir.path(), "main").await.unwrap(); + + let parked = helpers::failpoint::Rendezvous::park_first( + &catalog::OPTIMIZE_POST_AUTHORITY_CAPTURE_PRE_GATES, + ); + let optimize = tokio::spawn({ + let db = Arc::clone(&optimizing); + async move { db.optimize_as(None).await.map(|_| ()) } + }); + parked.wait_until_reached().await; + let mut cleanup = tokio::spawn({ + let db = Arc::clone(&cleaning); + async move { db.cleanup_as(keep_one(), None).await.map(|_| ()) } + }); + assert!( + tokio::time::timeout(std::time::Duration::from_millis(300), &mut cleanup) + .await + .is_err(), + "the cleanup waits for the lane the optimize holds" + ); + assert_eq!( + cleaning.maintenance_in_progress(), + Some(omnigraph::db::MaintenanceInProgress { + operation: omnigraph::db::MaintenanceOperation::Optimize, + branch: Some("main".to_string()), + }), + "every handle on the root sees the one lane" + ); + + cleaning.cancel_maintenance(); + let waited = tokio::time::timeout(std::time::Duration::from_secs(30), cleanup) + .await + .expect("a cancel releases a waiting run") + .unwrap() + .expect_err("a cancelled waiter does not run"); + assert_eq!( + waited.completion_evidence(), + Some(CompletionEvidence::BeforeEffect), + "{waited}" + ); + assert!(matches!( + waited.into_completion_evidence().0, + OmniError::MaintenanceCancelled { + operation: "cleanup" + } + )); + + parked.release(); + let stopped = tokio::time::timeout(std::time::Duration::from_secs(30), optimize) + .await + .expect("the parked optimize stops at its next cancel point") + .unwrap() + .expect_err("a cancelled optimize publishes nothing"); + drop(parked); + assert_eq!( + stopped.completion_evidence(), + Some(CompletionEvidence::BeforeEffect), + "the optimize had staged nothing yet: {stopped}" + ); + assert_eq!( + branch_head_commit_id(dir.path(), "main").await.unwrap(), + head_before + ); + + cleaning + .cleanup_as(keep_one(), None) + .await + .expect("a cleanup that starts after the cancel runs"); + optimizing + .optimize_as(None) + .await + .expect("an optimize that starts after the cancel runs"); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] #[serial] #[serial(optimize)] @@ -4816,7 +5028,7 @@ async fn optimize_refuses_when_graph_authority_moves_before_its_gates() { &catalog::OPTIMIZE_POST_AUTHORITY_CAPTURE_PRE_GATES, ); let optimize_task_db = std::sync::Arc::clone(&optimize_db); - let optimize = tokio::spawn(async move { optimize_task_db.optimize().await }); + let optimize = tokio::spawn(async move { optimize_task_db.optimize_as(None).await }); rendezvous.wait_until_reached().await; // Advance the graph head underneath it with an ordinary committed write, so @@ -4912,7 +5124,7 @@ async fn optimize_holds_main_gate_through_disjoint_table_effects() { let uri_opt = uri.clone(); let optimize = tokio::spawn(async move { let db = helpers::session(Omnigraph::open(&uri_opt).await.unwrap()); - db.optimize().await + db.optimize_as(None).await }); rendezvous.wait_until_reached().await; @@ -4956,7 +5168,7 @@ async fn optimize_holds_main_gate_through_disjoint_table_effects() { 1, "queued table-disjoint Company insert must persist", ); - db.optimize() + db.optimize_as(None) .await .expect("graph must remain healthy / re-optimizable"); } @@ -4996,7 +5208,7 @@ async fn optimize_serializes_concurrent_delete_across_handles() { let uri_opt = uri.clone(); let optimize = tokio::spawn(async move { let db = helpers::session(Omnigraph::open(&uri_opt).await.unwrap()); - db.optimize().await + db.optimize_as(None).await }); rendezvous.wait_until_reached().await; @@ -5056,7 +5268,7 @@ async fn optimize_serializes_concurrent_delete_across_handles() { 3, "the concurrent delete must persist (alice removed)", ); - db.optimize() + db.optimize_as(None) .await .expect("graph must remain healthy / re-optimizable"); } @@ -5410,7 +5622,7 @@ async fn ensure_indices_phase_b_failure_does_not_leak_sidecar_when_no_work_neede ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); } // Setup: trigger the failpoint. Steady-state ensure_indices @@ -5420,7 +5632,7 @@ async fn ensure_indices_phase_b_failure_does_not_leak_sidecar_when_no_work_neede { let db = helpers::session(Omnigraph::open(&uri).await.unwrap()); let _failpoint = catalog::ENSURE_INDICES_POST_PHASE_B_PRE_MANIFEST_COMMIT.fire_always(); - let err = db.ensure_indices().await.unwrap_err(); + let err = db.ensure_indices_as(None).await.unwrap_err(); assert!( err.to_string().contains( "injected failpoint triggered: ensure_indices.post_phase_b_pre_manifest_commit" @@ -7337,7 +7549,7 @@ async fn ensure_indices_failure_before_publish_leaves_no_residue() { let version = helpers::version_main(&db).await.unwrap(); { let _failpoint = catalog::ENSURE_INDICES_POST_TABLE_EFFECT.fire_always(); - let err = db.ensure_indices().await.unwrap_err(); + let err = db.ensure_indices_as(None).await.unwrap_err(); assert!( err.to_string() .contains("injected failpoint triggered: ensure_indices.post_table_effect"), @@ -7348,7 +7560,7 @@ async fn ensure_indices_failure_before_publish_leaves_no_residue() { assert_eq!(person_head_and_published(&db, "main").await, before); assert_eq!(helpers::version_main(&db).await.unwrap(), version); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); assert_person_pin_detached(&db, "main", before.0).await; assert_eq!( person_head_and_published(&db, "main").await.1, @@ -7409,7 +7621,7 @@ async fn full_text_rebuild_on_a_branch_stages_on_the_inherited_table() { db.load_jsonl(SEARCH_DATA, LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db.branch_create("rebuild").await.unwrap(); let main_before = doc_head_and_published(&db, "main").await; let main_entry = helpers::snapshot_main(&db) @@ -7421,7 +7633,7 @@ async fn full_text_rebuild_on_a_branch_stages_on_the_inherited_table() { let main_pin = pinned_version(&db, "main", "node:Doc").await; let branch_version = helpers::version_branch(&db, "rebuild").await.unwrap(); - db.rebuild_full_text_indices_on("rebuild") + db.rebuild_full_text_indices_on_as("rebuild", None) .await .expect("a branch rebuild stages on the inherited table and never forks"); assert!(sidecar_operation_ids(dir.path()).is_empty()); @@ -7518,7 +7730,7 @@ async fn assert_optimize_detached_pin_survives_next_write(uri: &str) { { let _failpoint = catalog::GRAPH_PUBLISH_AFTER_MANIFEST_COMMIT.fire_once_at(1); let error = db - .optimize() + .optimize_as(None) .await .expect_err("compaction published before its acknowledgement was lost"); assert!( @@ -8604,10 +8816,13 @@ async fn rfc_0067_cleanup_reaps_aged_surplus_detached_manifests() { } } for _ in 0..2 { - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: Some(std::time::Duration::ZERO), - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: Some(std::time::Duration::ZERO), + }, + None, + ) .await .unwrap(); } @@ -8672,10 +8887,13 @@ async fn rfc_0067_zero_age_cleanup_keeps_a_fresh_unpublished_detached_manifest() "the interrupted write must strand at least one detached manifest" ); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: Some(std::time::Duration::ZERO), - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: Some(std::time::Duration::ZERO), + }, + None, + ) .await .unwrap(); let after = detached_files(); @@ -8691,10 +8909,13 @@ async fn rfc_0067_zero_age_cleanup_keeps_a_fresh_unpublished_detached_manifest() .set_times(std::fs::FileTimes::new().set_modified(std::time::SystemTime::UNIX_EPOCH)) .unwrap(); } - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: Some(std::time::Duration::ZERO), - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: Some(std::time::Duration::ZERO), + }, + None, + ) .await .unwrap(); let aged = detached_files(); @@ -8716,10 +8937,13 @@ fn rfc_0067_external_cleanup_process() { .unwrap() .block_on(async { let db = Omnigraph::open(&uri).await.unwrap(); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: Some(std::time::Duration::ZERO), - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: Some(std::time::Duration::ZERO), + }, + None, + ) .await .unwrap(); }); @@ -9330,20 +9554,20 @@ async fn live_handle_writes_after_every_write_family_seam_failure() { db.apply_schema(&render_schema(&schema_props, age_indexed)) .await .expect("ensure-row schema setup"); - db.ensure_indices() + db.ensure_indices_as(None) .await .map(|_| ()) .map_err(|error| error.to_string()) } Driver::Optimize => db - .optimize() + .optimize_as(None) .await .map(|_| ()) .map_err(|error| error.to_string()), - Driver::Cleanup => Box::pin(db.cleanup(omnigraph::db::CleanupPolicyOptions { + Driver::Cleanup => Box::pin(db.cleanup_as(omnigraph::db::CleanupPolicyOptions { keep_versions: Some(1), older_than: None, - })) + }, None)) .await .map(|_| ()) .map_err(|error| error.to_string()), @@ -9419,10 +9643,13 @@ async fn rfc_0067_merge_diff_keeps_every_chunk() { after_rows - before_rows ); db.branch_delete("feature").await.unwrap(); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: Some(std::time::Duration::ZERO), - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: Some(std::time::Duration::ZERO), + }, + None, + ) .await .unwrap(); let person_uri = node_table_uri(&db, "Person").await; @@ -9462,10 +9689,13 @@ async fn rfc_0067_cleanup_window_keeps_version_gc_on_a_written_table() { for pass in 0..2 { let stats = db - .cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: Some(std::time::Duration::from_secs(7 * 24 * 60 * 60)), - }) + .cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: Some(std::time::Duration::from_secs(7 * 24 * 60 * 60)), + }, + None, + ) .await .unwrap(); let person = stats @@ -9543,7 +9773,7 @@ async fn collector_keeps_live_staging_and_reclaims_it_once_the_head_moves() { ); assert!(staging.bytes > 0); } - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); let row = stats .iter() .find(|row| row.type_key == "node:Person") @@ -9570,7 +9800,7 @@ async fn collector_keeps_live_staging_and_reclaims_it_once_the_head_moves() { staging.verdict ); } - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); let row = stats .iter() .find(|row| row.type_key == "node:Person") @@ -9662,7 +9892,7 @@ async fn collector_keeps_pins_published_by_a_late_branch_in_another_process() { let expected_rows = count_rows(&db, "node:Person").await + 1; let cleaner = helpers::session(Omnigraph::open(db.uri()).await.unwrap()); let (guard, hold) = catalog::CLEANUP_COLLECTOR_POST_SNAPSHOT.hold(); - let cleanup = tokio::spawn(async move { cleaner.cleanup(keep_one()).await }); + let cleanup = tokio::spawn(async move { cleaner.cleanup_as(keep_one(), None).await }); let reached = hold.clone(); tokio::task::spawn_blocking(move || reached.wait_until_reached()) .await @@ -9705,7 +9935,7 @@ async fn collector_keeps_pins_published_by_a_late_branch_in_another_process() { path.display() ); } - let retry = reader.cleanup(keep_one()).await.unwrap(); + let retry = reader.cleanup_as(keep_one(), None).await.unwrap(); assert!(retry.iter().all(|row| row.error.is_none()), "{retry:?}"); assert_eq!(count_rows(&reader, "node:Person").await, expected_rows - 1); continue; @@ -9752,7 +9982,7 @@ async fn collector_refuses_mixed_branch_captures_before_deleting() { count_rows(&db, "node:Person").await + usize::from(action == "collector_mixed"); let cleaner = helpers::session(Omnigraph::open(db.uri()).await.unwrap()); let (guard, hold) = catalog::CLEANUP_COLLECTOR_MID_BRANCH_READ.hold(); - let cleanup = tokio::spawn(async move { cleaner.cleanup(keep_one()).await }); + let cleanup = tokio::spawn(async move { cleaner.cleanup_as(keep_one(), None).await }); let reached = hold.clone(); tokio::task::spawn_blocking(move || reached.wait_until_reached()) .await @@ -9790,7 +10020,7 @@ async fn collector_refuses_mixed_branch_captures_before_deleting() { let pin = pinned_version(&reader, "main", "node:Person").await; assert!(detached_versions(&person_uri).await.contains(&pin)); assert_eq!(count_rows(&reader, "node:Person").await, expected_rows); - let retry = reader.cleanup(keep_one()).await.unwrap(); + let retry = reader.cleanup_as(keep_one(), None).await.unwrap(); assert!(retry.iter().all(|row| row.error.is_none()), "{retry:?}"); assert!(detached_versions(&person_uri).await.contains(&pin)); } @@ -9808,7 +10038,7 @@ async fn collector_pass_interrupted_mid_sweep_counts_nothing_and_retries_whole() let stats = { let _fp = catalog::CLEANUP_SWEEP_BETWEEN_MANIFESTS.fail_once_at(1); - db.cleanup(keep_one()).await.unwrap() + db.cleanup_as(keep_one(), None).await.unwrap() }; let row = stats .iter() @@ -9824,7 +10054,7 @@ async fn collector_pass_interrupted_mid_sweep_counts_nothing_and_retries_whole() row.manifests_removed, 1, "the pass stopped after its first manifest: {row:?}" ); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); let row = stats .iter() .find(|row| row.type_key == "node:Person") @@ -9891,7 +10121,7 @@ async fn assert_collector_retry_treats_an_index_directory_an_interrupted_pass_re let stats = { let _fp = catalog::CLEANUP_SWEEP_PRE_FILE_DELETE.fire_always(); - db.cleanup(keep_one()).await.unwrap() + db.cleanup_as(keep_one(), None).await.unwrap() }; let row = stats .iter() @@ -9924,7 +10154,7 @@ async fn assert_collector_retry_treats_an_index_directory_an_interrupted_pass_re let stats = { let _fp = catalog::CLEANUP_SWEEP_PRE_MANIFEST_DELETE.fire_always(); - db.cleanup(keep_one()).await.unwrap() + db.cleanup_as(keep_one(), None).await.unwrap() }; let row = stats .iter() @@ -9945,7 +10175,7 @@ async fn assert_collector_retry_treats_an_index_directory_an_interrupted_pass_re ); } - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); let row = stats .iter() @@ -10031,7 +10261,7 @@ async fn assert_collector_retry_after_a_pass_stopped_between_manifests_finishes_ let stats = { let _fp = catalog::CLEANUP_SWEEP_BETWEEN_MANIFESTS.fire_always(); - db.cleanup(keep_one()).await.unwrap() + db.cleanup_as(keep_one(), None).await.unwrap() }; let row = stats .iter() @@ -10079,7 +10309,7 @@ async fn assert_collector_retry_after_a_pass_stopped_between_manifests_finishes_ plan.sweep_paths ); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); let row = stats .iter() @@ -10134,7 +10364,7 @@ async fn assert_collector_retry_after_a_pass_stopped_before_the_dead_manifests_r let before = detached_versions(&person_uri).await; { let _fp = catalog::ENSURE_INDICES_POST_PHASE_B_PRE_MANIFEST_COMMIT.fire_always(); - let outcome = db.ensure_indices().await; + let outcome = db.ensure_indices_as(None).await; assert!( outcome.is_err(), "the seam fails the index build before publish" @@ -10172,7 +10402,7 @@ async fn assert_collector_retry_after_a_pass_stopped_before_the_dead_manifests_r let stats = { let _fp = catalog::CLEANUP_SWEEP_PRE_DEAD_MANIFEST_DELETE.fire_always(); - db.cleanup(keep_one()).await.unwrap() + db.cleanup_as(keep_one(), None).await.unwrap() }; let row = stats .iter() @@ -10213,7 +10443,7 @@ async fn assert_collector_retry_after_a_pass_stopped_before_the_dead_manifests_r plan.dead_paths ); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); let row = stats .iter() @@ -10358,7 +10588,7 @@ async fn branch_merge_in_doubt_keeps_input_tags_until_cleanup() { "the acknowledged source and target guards must survive an in-doubt fast-forward" ); - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!( !native @@ -10892,7 +11122,7 @@ async fn unattached_live_chain_pins_are_rooted_and_survive_cleanup() { report.retained_unattached_tags ); - let rows = db.cleanup(keep_one_at_any_age()).await.unwrap(); + let rows = db.cleanup_as(keep_one_at_any_age(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); let remaining = detached_versions(&person_uri).await; assert!( @@ -10937,7 +11167,7 @@ async fn unattached_live_chain_pins_are_rooted_and_survive_cleanup() { let plan = main_plan(&report, "node:Person"); assert!(plan.roots.contains(&grandchild_pin)); assert!(plan.roots.contains(&root_pin)); - let rows = db.cleanup(keep_one_at_any_age()).await.unwrap(); + let rows = db.cleanup_as(keep_one_at_any_age(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); let remaining = detached_versions(&person_uri).await; assert!(remaining.contains(&grandchild_pin)); @@ -11013,7 +11243,9 @@ async fn cleanup_releases_expired_unattached_tags_descendants_first() { let interrupted = { let _fp = catalog::CLEANUP_POST_UNATTACHED_RELEASE.fail_once_at(1); - db.cleanup(keep_one_at_any_age()).await.unwrap_err() + db.cleanup_as(keep_one_at_any_age(), None) + .await + .unwrap_err() }; assert!( interrupted @@ -11027,7 +11259,7 @@ async fn cleanup_releases_expired_unattached_tags_descendants_first() { "the child's tag went first; the root's stays for the retry" ); - let rows = db.cleanup(keep_one_at_any_age()).await.unwrap(); + let rows = db.cleanup_as(keep_one_at_any_age(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!(unattached_tags(&uri).await.is_empty()); let at = |id: &str| ReadTarget::snapshot(SnapshotId::new(id.to_string())); @@ -11045,7 +11277,7 @@ async fn cleanup_releases_expired_unattached_tags_descendants_first() { person_count_at(&db, ReadTarget::branch("main")).await, main_before + 1 ); - let rows = db.cleanup(keep_one_at_any_age()).await.unwrap(); + let rows = db.cleanup_as(keep_one_at_any_age(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); } @@ -11109,7 +11341,7 @@ async fn abandoned_merge_input_tags_expire_after_target_advances() { ) .await .unwrap(); - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!( captured_path.is_file(), @@ -11122,7 +11354,7 @@ async fn abandoned_merge_input_tags_expire_after_target_advances() { ) .await .unwrap(); - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!( !captured_path.exists(), @@ -11174,7 +11406,7 @@ async fn cleanup_keeps_a_late_retired_merge_base_provider() { db.branch_create("target").await.unwrap(); let cleaner = helpers::session(Omnigraph::open(&uri).await.unwrap()); let parked = helpers::failpoint::Rendezvous::park_first(&catalog::CLEANUP_TABLE_GC); - let cleanup = tokio::spawn(async move { cleaner.cleanup(keep_one()).await }); + let cleanup = tokio::spawn(async move { cleaner.cleanup_as(keep_one(), None).await }); parked.wait_until_reached().await; let child_uri = uri.clone(); let child = tokio::task::spawn_blocking(move || { @@ -11238,7 +11470,7 @@ async fn planning_source_failures_keep_their_error_class() { ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let lookup = params(&[("$name", "Planned")]); for seam in [ &catalog::QUERY_INDEX_FACTS_PRE_LOAD, diff --git a/crates/omnigraph/tests/forbidden_apis.rs b/crates/omnigraph/tests/forbidden_apis.rs index 288590fd1..7ddff6755 100644 --- a/crates/omnigraph/tests/forbidden_apis.rs +++ b/crates/omnigraph/tests/forbidden_apis.rs @@ -190,7 +190,6 @@ const SENTINEL: &str = "// forbidden-api-allow:"; enum WriteProtocol { Exact(&'static str), Composed(&'static str), - ManifestAdoption, NativeRefControl, PhysicalOnly, EphemeralScratch, @@ -205,7 +204,6 @@ impl WriteProtocol { match self { Self::Exact(name) => format!("exact adapter ({name})"), Self::Composed(name) => format!("composed protocol ({name})"), - Self::ManifestAdoption => "manifest adoption".into(), Self::NativeRefControl => "native ref control".into(), Self::PhysicalOnly => "physical-only maintenance".into(), Self::EphemeralScratch => "ephemeral scratch".into(), @@ -264,13 +262,11 @@ write_surfaces! { "db/omnigraph.rs" => WriteProtocol::Exact("unpublished commit publication: the proposal's recorded changes replayed through the manifest publisher under its captured authority, required to allocate the proposal's own id") => ["publish_commit_as"], "db/omnigraph.rs" => WriteProtocol::Composed("proposal tag removal confirmed by a readback, then merge-input tag release") => ["drop_commit_as"], "db/omnigraph.rs" => INDICES_V9 => [ - "ensure_indices", "ensure_indices_on", - "rebuild_full_text_indices_on", "rebuild_full_text_indices_on_as", + "ensure_indices_as", "ensure_indices_on_as", "rebuild_full_text_indices_on_as", ], "db/omnigraph.rs" => WriteProtocol::TestOnly => ["failpoint_publish_table_head_without_index_rebuild_for_test", "init_with_legacy_system_columns_for_tests"], - "db/omnigraph.rs" => OPTIMIZE_V9 => ["optimize"], - "db/omnigraph.rs" => WriteProtocol::ManifestAdoption => ["repair"], - "db/omnigraph.rs" => WriteProtocol::PhysicalOnly => ["cleanup"], + "db/omnigraph.rs" => OPTIMIZE_V9 => ["optimize_as"], + "db/omnigraph.rs" => WriteProtocol::PhysicalOnly => ["cleanup_as"], "db/omnigraph.rs" => WriteProtocol::NativeRefControl => ["branch_create", "branch_create_as", "branch_create_from", "branch_create_from_as", "branch_delete", "branch_delete_as"], } @@ -296,6 +292,9 @@ const READ_ONLY_SURFACES: &[(&str, &str)] = &[ ("db/omnigraph.rs", "open_read_only"), ("db/omnigraph.rs", "refresh"), ("db/omnigraph.rs", "cleanup_plan"), + // The repair report: it publishes nothing, but `maintain` gates it like + // the maintenance writers (`maintenance_entry_points_take_an_actor`). + ("db/omnigraph.rs", "repair_report_as"), ("db/omnigraph.rs", "cleanup_plan_missing_paths"), ("db/omnigraph.rs", "cleanup_plan_path_snapshot"), ("db/omnigraph.rs", "open_read_only_with_storage"), @@ -1054,6 +1053,7 @@ durable_calls! { ("omnigraph-storage/scoped.rs", ".complete(", 1, WriteProtocol::Composed("owned multipart publication, never success after uncertain part completion")), ("omnigraph-storage/scoped.rs", ".abort(", 1, WriteProtocol::Composed("owned multipart cleanup retained across detached abort")), ("engine/operators/memory.rs", ".abort(", 1, WriteProtocol::ReadTaskCancellation), + ("db/maintenance.rs", ".abort(", 1, WriteProtocol::Composed("drop a cancelled maintenance task's Lance work, whose effects stay unreachable staging")), ("omnigraph-storage/lib.rs", ".rename(", 1, WriteProtocol::Composed("object storage rename primitive")), ("storage_layer.rs", ".force_delete_branch(", 1, WriteProtocol::NativeRefControl), ("storage_layer.rs", ".commit_staged_create_exact(", 1, WriteProtocol::Exact("sealed TableStorage create forwarding")), @@ -1092,7 +1092,6 @@ durable_calls! { ("db/omnigraph/table_ops.rs", ".commit_changes_with_intent_and_expected(", 1, WriteProtocol::Exact("shared publisher")), ("db/omnigraph/schema_apply.rs", ".commit_changes_with_intent_and_expected(", 1, SCHEMA_V9), ("db/omnigraph/schema_apply/settlement.rs", ".commit_changes_with_intent_and_expected(", 1, WriteProtocol::Exact("schema settlement: strict numeric neutral lineage publication")), - ("db/omnigraph/repair.rs", ".commit_updates_with_actor_with_expected(", 1, WriteProtocol::ManifestAdoption), ("db/graph_coordinator.rs", ".commit_changes_with_intent_and_expected(", 1, WriteProtocol::Exact("publisher gateway")), ("db/graph_coordinator.rs", ".commit_changes_with_lineage_and_precondition(", 1, WriteProtocol::Exact("lowest manifest publisher gateway")), ("omnigraph-catalog/lib.rs", ".publish_with_precondition(", 1, WriteProtocol::Exact("lowest manifest publisher gateway")), @@ -1142,7 +1141,8 @@ durable_calls! { ("db/omnigraph/collector.rs", ".dataset()", 17, WriteProtocol::ReadOnlyAccess), ("db/omnigraph/collector.rs", ".delete(", 1, WriteProtocol::Composed("detached-only sweep through the table's own Lance store: freed files, then the published manifests no retained `__manifest` version pins (links before tips) and the dead stagings; an interrupted pass is re-swept by the next")), ("db/omnigraph/schema_apply.rs", ".dataset()", 1, SCHEMA_V9), - ("db/omnigraph/repair.rs", ".dataset()", 1, WriteProtocol::ManifestAdoption), + // The repair report reads raw transaction history; it publishes nothing. + ("db/omnigraph/repair.rs", ".dataset()", 1, WriteProtocol::ReadOnlyAccess), // The sixth accessor reports deferred FTS coverage from an immutable // snapshot; it only reads index metadata and never stages or publishes. ("db/omnigraph/optimize.rs", ".dataset()", 10, WriteProtocol::Composed("Optimize planning + read-only coverage and native-fork inventory + physical cleanup, including the release of expired proposal tags and of expired unattached tags descendants first, each confirmed by a readback")), @@ -2137,6 +2137,107 @@ fn graph_write_surfaces_are_registered() { ); } +/// The public surfaces of maintenance: the graph writers registered under +/// these protocols, plus the read-only repair report the same gate covers. +fn is_maintenance_protocol(protocol: WriteProtocol) -> bool { + protocol == INDICES_V9 || protocol == OPTIMIZE_V9 || protocol == WriteProtocol::PhysicalOnly +} + +const READ_ONLY_MAINTENANCE_SURFACES: &[(&str, &str)] = &[("db/omnigraph.rs", "repair_report_as")]; + +/// Maintenance has no actor-less public form (invariant 10). Every public +/// optimize, cleanup, index or repair entry point is an `_as` method taking the +/// `actor: Option<&str>` its policy check judges, so neither an embedded nor a +/// direct caller reaches maintenance around the gate; with a policy installed, +/// a `None` actor is refused. `policy_engine_chassis.rs` owns which action and +/// scope each one checks. +#[test] +fn maintenance_entry_points_take_an_actor() { + let src = engine_src_root(); + let read_only = READ_ONLY_SURFACES.iter().copied().collect::>(); + for surface in READ_ONLY_MAINTENANCE_SURFACES { + assert!( + read_only.contains(surface), + "{surface:?} must stay a registered read-only surface" + ); + } + let surfaces = WRITE_SURFACES + .iter() + .filter(|surface| is_maintenance_protocol(surface.protocol)) + .map(|surface| (surface.file, surface.function)) + .chain(READ_ONLY_MAINTENANCE_SURFACES.iter().copied()) + .collect::>(); + assert_eq!( + surfaces, + BTreeSet::from([ + ("db/omnigraph.rs", "cleanup_as"), + ("db/omnigraph.rs", "ensure_indices_as"), + ("db/omnigraph.rs", "ensure_indices_on_as"), + ("db/omnigraph.rs", "optimize_as"), + ("db/omnigraph.rs", "rebuild_full_text_indices_on_as"), + ("db/omnigraph.rs", "repair_report_as"), + ]), + "a maintenance surface was added or removed; give it an actor and a policy case" + ); + for (file, function) in surfaces { + assert!( + function.ends_with("_as"), + "{file}::{function}: maintenance entry points are `_as` forms" + ); + let path = guarded_path(&src, file); + let contents = std::fs::read_to_string(&path) + .unwrap_or_else(|error| panic!("failed to read {}: {error}", path.display())); + let ast = parse_rust_source(&contents, file); + let signatures = function_signatures(&ast, function); + assert_eq!( + signatures.len(), + 1, + "{file}::{function} must resolve to exactly one function" + ); + let takes_actor = signatures[0].inputs.iter().any(|input| { + let syn::FnArg::Typed(argument) = input else { + return false; + }; + matches!(&*argument.pat, syn::Pat::Ident(name) if name.ident == "actor") + && type_final_ident(&argument.ty).is_some_and(|ident| ident == "Option") + }); + assert!( + takes_actor, + "{file}::{function} must take `actor: Option<&str>` for its policy check" + ); + } +} + +fn function_signatures(ast: &syn::File, function_name: &str) -> Vec { + struct Finder<'a> { + function_name: &'a str, + signatures: Vec, + } + + impl<'ast> Visit<'ast> for Finder<'_> { + fn visit_item_fn(&mut self, node: &'ast syn::ItemFn) { + if node.sig.ident == self.function_name { + self.signatures.push(node.sig.clone()); + } + visit::visit_item_fn(self, node); + } + + fn visit_impl_item_fn(&mut self, node: &'ast syn::ImplItemFn) { + if node.sig.ident == self.function_name { + self.signatures.push(node.sig.clone()); + } + visit::visit_impl_item_fn(self, node); + } + } + + let mut finder = Finder { + function_name, + signatures: Vec::new(), + }; + finder.visit_file(ast); + finder.signatures +} + #[test] fn low_level_coordinator_surfaces_are_registered() { let src = engine_src_root(); diff --git a/crates/omnigraph/tests/helpers/expand_projection.rs b/crates/omnigraph/tests/helpers/expand_projection.rs index 0fe1e877e..8f792dee2 100644 --- a/crates/omnigraph/tests/helpers/expand_projection.rs +++ b/crates/omnigraph/tests/helpers/expand_projection.rs @@ -63,7 +63,7 @@ pub async fn fixture(dir: &tempfile::TempDir, sources: usize, payload_bytes: usi .await .unwrap(); } - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); with_setting(&db, "engine", "v2") } diff --git a/crates/omnigraph/tests/helpers/mod.rs b/crates/omnigraph/tests/helpers/mod.rs index c39350972..af4c39238 100644 --- a/crates/omnigraph/tests/helpers/mod.rs +++ b/crates/omnigraph/tests/helpers/mod.rs @@ -347,7 +347,7 @@ pub async fn init_and_load_at(uri: &str, schema: &str) -> Session { db.load_jsonl(TEST_DATA, LoadMode::Overwrite).await.unwrap(); // Mutation/load publish only exact data effects; physical indexes are // reconciled separately as derived state. - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } diff --git a/crates/omnigraph/tests/integration/branching.rs b/crates/omnigraph/tests/integration/branching.rs index 388854f56..bb2379621 100644 --- a/crates/omnigraph/tests/integration/branching.rs +++ b/crates/omnigraph/tests/integration/branching.rs @@ -150,7 +150,7 @@ async fn init_search_db(dir: &tempfile::TempDir) -> Session { db.load_jsonl(SEARCH_DATA, LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } @@ -2261,10 +2261,13 @@ fn assert_native_version_case( ); let maintenance = Omnigraph::open(uri).await.unwrap(); maintenance - .cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(100), - older_than: None, - }) + .cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(100), + older_than: None, + }, + None, + ) .await .unwrap(); main.mutate( @@ -3623,7 +3626,10 @@ async fn ensure_indices_on_child_branch_keeps_inherited_table_when_no_work_is_ne "the child branch inherits feature's pin" ); - experiment.ensure_indices_on("experiment").await.unwrap(); + experiment + .ensure_indices_on_as("experiment", None) + .await + .unwrap(); let experiment_snap = snapshot_branch(&experiment, "experiment").await.unwrap(); assert_eq!( @@ -4210,10 +4216,13 @@ async fn branch_delete_defers_pin_collection_and_allows_recreate() { "recreated writes leave the former pin intact" ); assert!(detached.contains(&second_pin)); - main.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + main.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); let detached = helpers::collector::detached_versions(&person_uri).await; @@ -4409,10 +4418,13 @@ async fn branch_delete_retires_legacy_physical_path_parents() { "{refused}" ); let rows = db - .cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); @@ -4595,10 +4607,13 @@ async fn branch_delete_retires_native_parent_and_cleanup_preserves_live_child() "{invalid}" ); assert!( - main.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + main.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None + ) .await .is_err(), "{invalid}" @@ -4632,10 +4647,13 @@ async fn branch_delete_retires_native_parent_and_cleanup_preserves_live_child() .await .unwrap(); reopened - .cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert!(manifest.branches().get(&native).await.is_err()); @@ -4649,10 +4667,13 @@ async fn branch_delete_retires_native_parent_and_cleanup_preserves_live_child() ); reopened.branch_delete("experiment").await.unwrap(); reopened - .cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert!(manifest.branches().get(&native).await.is_err()); @@ -4681,7 +4702,7 @@ async fn merged_table_preserves_row_version_for_unchanged_rows() { let dir = tempfile::tempdir().unwrap(); let uri = dir.path().to_str().unwrap(); let main = init_and_load(&dir).await; - main.ensure_indices().await.unwrap(); + main.ensure_indices_as(None).await.unwrap(); main.branch_create("feature").await.unwrap(); let feature = helpers::session(Omnigraph::open(uri).await.unwrap()); @@ -4764,7 +4785,7 @@ async fn merged_table_preserves_row_version_for_unchanged_rows() { async fn edge_tables_have_id_btree_after_ensure_indices() { let dir = tempfile::tempdir().unwrap(); let db = init_and_load(&dir).await; - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let snap = snapshot_main(&db).await.unwrap(); let ds = snap.open_dataset("edge:Knows").await.unwrap(); @@ -4786,7 +4807,7 @@ async fn merge_delta_only_bumps_changed_rows() { let dir = tempfile::tempdir().unwrap(); let uri = dir.path().to_str().unwrap(); let main = init_and_load(&dir).await; - main.ensure_indices().await.unwrap(); + main.ensure_indices_as(None).await.unwrap(); main.branch_create("feature").await.unwrap(); let feature = helpers::session(Omnigraph::open(uri).await.unwrap()); diff --git a/crates/omnigraph/tests/integration/changes.rs b/crates/omnigraph/tests/integration/changes.rs index fc782044f..80e0815b6 100644 --- a/crates/omnigraph/tests/integration/changes.rs +++ b/crates/omnigraph/tests/integration/changes.rs @@ -17,10 +17,13 @@ use crate::helpers::*; /// name). async fn reclaim_person_history(db: &Session) { let stats = db - .cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); let person = stats @@ -550,7 +553,7 @@ async fn diff_after_merge_reports_actual_changes() { let dir = tempfile::tempdir().unwrap(); let uri = dir.path().to_str().unwrap(); let main = init_and_load(&dir).await; - main.ensure_indices().await.unwrap(); + main.ensure_indices_as(None).await.unwrap(); let v_before_branch = snapshot_id(&main, "main").await.unwrap(); main.branch_create("feature").await.unwrap(); @@ -2056,7 +2059,7 @@ node Document { // touching logical rows. The moved descriptors force the payload // tie-break, which must classify every row as unchanged: the maintenance // commit is an empty block, not a phantom full-table update. - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); assert!( stats .iter() @@ -2158,7 +2161,7 @@ async fn physical_only_commit_over_a_large_fragment_reads_as_empty_issue_705() { ) .await .unwrap(); - let stats = main.optimize().await.unwrap(); + let stats = main.optimize_as(None).await.unwrap(); assert!( stats .iter() @@ -3186,7 +3189,7 @@ async fn change_feed_merge_commit_is_first_parent_relative_with_merged_parent_on let dir = tempfile::tempdir().unwrap(); let uri = dir.path().to_str().unwrap(); let main = init_and_load(&dir).await; - main.ensure_indices().await.unwrap(); + main.ensure_indices_as(None).await.unwrap(); let pre_merge_head = snapshot_id(&main, "main").await.unwrap(); main.branch_create("side").await.unwrap(); diff --git a/crates/omnigraph/tests/integration/changes_cost.rs b/crates/omnigraph/tests/integration/changes_cost.rs index c35e9a104..2f82e1716 100644 --- a/crates/omnigraph/tests/integration/changes_cost.rs +++ b/crates/omnigraph/tests/integration/changes_cost.rs @@ -26,14 +26,10 @@ use crate::helpers; -use std::sync::Arc; - use crate::helpers::cost::{IoCounts, assert_flat, assert_grows, cost_harness, measure}; -use lance::Dataset; -use lance::dataset::UpdateBuilder; use omnigraph::IndexCoverage; use omnigraph::changes::ChangeFeedScope; -use omnigraph::db::{Omnigraph, ReadTarget, RepairOptions}; +use omnigraph::db::{Omnigraph, ReadTarget}; use omnigraph::loader::LoadMode; use omnigraph_compiler::ir::ParamMap; @@ -93,7 +89,7 @@ node Company { // Build full coverage, then append a new parent fragment. // The measured update therefore runs with a normal partial // / stale index state. The false arm never builds an index. - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db.load_with_receipt( "main", r#"{"type":"Person","data":{"name":"stale-index-tail","age":1}}"#, @@ -303,123 +299,6 @@ node Document { .await; } -/// Multi-version graph intervals are intentionally outside the candidate -/// contract. The fixture advances one physical dataset twice, then adopts both -/// logical updates in one forced repair graph commit. Two stateless -/// `max_changes=1` pages must each fall back before reading either transaction; -/// the page count cannot multiply a transaction-history walk. -#[tokio::test] -#[ignore = "the multi-version interval is built by `repair`, which promotes at head; phase 4 of RFC \"Detached-only tables\" restates it over a detached chain"] -async fn changes_page_size_one_skips_transaction_history_for_multi_version_intervals() { - cost_harness(async { - let dir = tempfile::tempdir().unwrap(); - let db = Omnigraph::init( - dir.path().to_str().unwrap(), - "node Person {\n name: String @key\n age: I32?\n}", - ) - .await - .unwrap(); - let db = helpers::session(db); - db.load_with_receipt( - "main", - concat!( - r#"{"type":"Person","data":{"name":"alice","age":1}}"#, - "\n", - r#"{"type":"Person","data":{"name":"bob","age":1}}"#, - ), - LoadMode::Merge, - ) - .await - .unwrap(); - let snapshot = db.snapshot_of(ReadTarget::branch("main")).await.unwrap(); - let entry = snapshot.dataset("node:Person").unwrap(); - let before = entry.published_dataset_version; - let dataset_uri = format!( - "{}/{}", - db.uri().trim_end_matches('/'), - entry.dataset_path.trim_start_matches('/') - ); - - let dataset = Dataset::open(&dataset_uri).await.unwrap(); - let dataset = UpdateBuilder::new(Arc::new(dataset)) - .update_where("name = 'alice'") - .unwrap() - .set("age", "2") - .unwrap() - .build() - .unwrap() - .execute() - .await - .unwrap() - .new_dataset; - let dataset = UpdateBuilder::new(dataset) - .update_where("name = 'bob'") - .unwrap() - .set("age", "3") - .unwrap() - .build() - .unwrap() - .execute() - .await - .unwrap() - .new_dataset; - assert_eq!(dataset.version().version, before + 2); - - let repair = db - .repair(RepairOptions { - confirm: true, - force: true, - }) - .await - .unwrap(); - assert!(repair.graph_manifest_version.is_some()); - let repair_commit = db - .list_commits(None) - .await - .unwrap() - .first() - .expect("repair publishes a graph lineage commit") - .graph_commit_id - .clone(); - - let (first, first_io) = measure(db.commit_changes_page( - &repair_commit, - &ChangeFeedScope::default(), - None, - Some(1), - None, - )) - .await; - let first = first.unwrap(); - assert_eq!(first.block.changes.len(), 1); - let token = first.next_page_token.expect("two updates need two pages"); - assert_eq!( - first_io.candidate_transaction_reads, 0, - "first multi-version page must fall back before transaction-history I/O" - ); - - let (second, second_io) = measure(db.commit_changes_page( - &repair_commit, - &ChangeFeedScope::default(), - Some(&token), - Some(1), - None, - )) - .await; - let second = second.unwrap(); - assert_eq!(second.block.changes.len(), 1); - assert!(second.next_page_token.is_none()); - assert_ne!(first.block.changes[0].id, second.block.changes[0].id); - for io in [first_io, second_io] { - assert_eq!( - io.candidate_transaction_reads, 0, - "every stateless multi-version page must skip transaction-history I/O" - ); - } - }) - .await; -} - /// A delete's commit path proves it from the recorded ids, so its page data /// reads stay flat as the table grows. #[tokio::test] diff --git a/crates/omnigraph/tests/integration/composite_flow.rs b/crates/omnigraph/tests/integration/composite_flow.rs index a9dfac8fd..863ab0271 100644 --- a/crates/omnigraph/tests/integration/composite_flow.rs +++ b/crates/omnigraph/tests/integration/composite_flow.rs @@ -309,7 +309,7 @@ async fn composite_flow_canonical_lifecycle() { // version, and this flow exercises exactly that previously-failing // write below. // ───────────────────────────────────────────────────────────────── - let optimize_stats = db.optimize().await.unwrap(); + let optimize_stats = db.optimize_as(None).await.unwrap(); assert!( !optimize_stats.is_empty(), "optimize must return per-table stats" @@ -362,10 +362,13 @@ async fn composite_flow_canonical_lifecycle() { use omnigraph::db::CleanupPolicyOptions; use std::time::Duration; let _cleanup_stats = db - .cleanup(CleanupPolicyOptions { - keep_versions: Some(10), - older_than: Some(Duration::from_secs(3600)), - }) + .cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(10), + older_than: Some(Duration::from_secs(3600)), + }, + None, + ) .await .unwrap(); diff --git a/crates/omnigraph/tests/integration/consistency.rs b/crates/omnigraph/tests/integration/consistency.rs index c7e3cd8fa..ab37588e4 100644 --- a/crates/omnigraph/tests/integration/consistency.rs +++ b/crates/omnigraph/tests/integration/consistency.rs @@ -1369,7 +1369,7 @@ node Thing { db.load_jsonl(&seed, LoadMode::Overwrite).await.unwrap(); // Build the key index explicitly before exercising the merge loads. - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let mut delta = String::new(); for i in (1..=25).chain(51..=55) { @@ -1382,7 +1382,7 @@ node Thing { db.load_jsonl(&delta, LoadMode::Merge) .await .expect("first merge after ensure_indices must succeed"); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db.load_jsonl(&delta, LoadMode::Merge) .await .expect("second merge after ensure_indices must succeed"); diff --git a/crates/omnigraph/tests/integration/end_to_end.rs b/crates/omnigraph/tests/integration/end_to_end.rs index 02e8e5f45..8ed619ced 100644 --- a/crates/omnigraph/tests/integration/end_to_end.rs +++ b/crates/omnigraph/tests/integration/end_to_end.rs @@ -2989,9 +2989,9 @@ async fn ensure_indices_does_not_error_on_repeated_call() { // load commits now enforce required indices; repeated ensure_indices calls // should be a no-op at the manifest level. - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let version_after_first = version_main(&db).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let version_after_second = version_main(&db).await.unwrap(); assert_eq!(version_after_first, version_after_load); @@ -3093,7 +3093,7 @@ async fn full_flow_optimize_then_query_update_and_reopen() { .unwrap(); } - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); assert!( stats.iter().any(|s| s.committed), "a multi-fragment table should have compacted in this flow" diff --git a/crates/omnigraph/tests/integration/engine_v2.rs b/crates/omnigraph/tests/integration/engine_v2.rs index 078c32321..cf54ed2f0 100644 --- a/crates/omnigraph/tests/integration/engine_v2.rs +++ b/crates/omnigraph/tests/integration/engine_v2.rs @@ -413,7 +413,7 @@ async fn indexed_single_hop_switches_to_the_csr_between_input_batches() { db.load_jsonl(&lines.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let v2 = with_setting(&db, "engine", "v2"); let pinned = first_column_sorted( @@ -1187,7 +1187,7 @@ edge Likes: Person -> Doc ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); for (source, name, params) in [ (ADULTS, "adults", int_params(&[("min", 18)])), (RANKED, "ranked", params(&[("q", "graph")])), diff --git a/crates/omnigraph/tests/integration/engine_v2_memory.rs b/crates/omnigraph/tests/integration/engine_v2_memory.rs index 02660da69..36b5a0120 100644 --- a/crates/omnigraph/tests/integration/engine_v2_memory.rs +++ b/crates/omnigraph/tests/integration/engine_v2_memory.rs @@ -520,7 +520,7 @@ node Doc { seed.push('\n'); } db.load_jsonl(&seed, LoadMode::Overwrite).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let v2 = with_setting(&db, "engine", "v2"); let source = r#"query fused($q: String) { match { $d: Doc } @@ -608,7 +608,7 @@ edge Knows: Doc -> Doc db.load_jsonl(&lines.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let v2 = with_setting( &with_setting(&db, "engine", "v2"), "rrf_plan", @@ -724,7 +724,7 @@ edge Knows: Person -> Person ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let v2 = with_setting(&db, "engine", "v2"); let queries = r#" query fused($q: String) { @@ -846,7 +846,7 @@ async fn hash_join_output_is_the_refusing_reservation() { db.load_jsonl(&lines.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let v2 = with_setting(&db, "engine", "v2"); let limit = 8 * MIB; @@ -929,7 +929,7 @@ async fn underestimated_hash_build_retries_id_lookup() { db.load_jsonl(&lines.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let v2 = with_setting(&db, "engine", "v2"); let limit = 16 * MIB; let explain = with_query_memory_limit( diff --git a/crates/omnigraph/tests/integration/engine_v2_plan_replay.rs b/crates/omnigraph/tests/integration/engine_v2_plan_replay.rs index f866b91da..35e4fd92e 100644 --- a/crates/omnigraph/tests/integration/engine_v2_plan_replay.rs +++ b/crates/omnigraph/tests/integration/engine_v2_plan_replay.rs @@ -65,7 +65,7 @@ async fn incomplete_rank_fuse_row_tiebreak_refuses_replay_issue_659() { .collect::>() .join("\n"); db.load_jsonl(&edges, LoadMode::Append).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let query = r#"query selected_page() { match { $p: Person $p $e:(knows | likes) $q } return { $e.@type as edge_type, $e.@id as edge_id, $q.name as target } @@ -305,7 +305,7 @@ async fn selected_cold_indexed_route_uses_persisted_members_without_building_csr ) .await .unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); drop(db); let db = with_traversal( &session(Omnigraph::open(dir.path().to_str().unwrap()).await.unwrap()), @@ -2123,7 +2123,7 @@ async fn a_replay_rechecks_recorded_full_text_coverage() { ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db.load_jsonl( r#"{"type":"Doc","data":{"slug":"t1","text":"graph","year":2022}} {"type":"Doc","data":{"slug":"t2","text":"graph graph graph graph","year":1990}}"#, diff --git a/crates/omnigraph/tests/integration/literal_filters.rs b/crates/omnigraph/tests/integration/literal_filters.rs index e493037db..41588b1db 100644 --- a/crates/omnigraph/tests/integration/literal_filters.rs +++ b/crates/omnigraph/tests/integration/literal_filters.rs @@ -579,7 +579,7 @@ async fn standalone_string_predicate_is_hoisted_into_scan() { async fn string_predicates_compose_with_search_and_negation() { let dir = tempfile::tempdir().unwrap(); let mut db = metric_db(&dir).await; - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let q = r#" query prefix_and_search() { match { diff --git a/crates/omnigraph/tests/integration/maintenance.rs b/crates/omnigraph/tests/integration/maintenance.rs index 15afbc9e8..a6b966ee5 100644 --- a/crates/omnigraph/tests/integration/maintenance.rs +++ b/crates/omnigraph/tests/integration/maintenance.rs @@ -17,7 +17,7 @@ use lance_core::datatypes::BlobHandling; use omnigraph::IndexCoverage; use omnigraph::db::{ CleanupPolicyOptions, MergeOutcome, Omnigraph, ReadTarget, RepairAction, RepairClassification, - RepairOptions, SnapshotId, + SnapshotId, }; use omnigraph::error::{BranchStartRefusal, OmniError}; use omnigraph::loader::LoadMode; @@ -59,6 +59,146 @@ async fn person_pin_and_head(db: &Omnigraph, root: &str) -> (u64, u64, String) { (pin, head, full) } +/// Make every directory in `dirs` read-only, or report that this host does +/// not enforce directory permissions for the test's user (a root user), in +/// which case the fault cannot be planted. +fn refuse_writes_under(dirs: &[std::path::PathBuf]) -> bool { + use std::os::unix::fs::PermissionsExt; + for dir in dirs { + std::fs::create_dir_all(dir).unwrap(); + std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o555)).unwrap(); + } + let probe = dirs[0].join("permission-probe"); + if std::fs::write(&probe, b"x").is_ok() { + std::fs::remove_file(&probe).unwrap(); + allow_writes_under(dirs); + eprintln!("skipping: this host does not enforce directory permissions"); + return false; + } + true +} + +fn allow_writes_under(dirs: &[std::path::PathBuf]) { + use std::os::unix::fs::PermissionsExt; + for dir in dirs { + std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o755)).unwrap(); + } +} + +/// `dir` under every table of the test schema on main. +async fn table_dirs(db: &Omnigraph, dir: &str) -> Vec { + let snapshot = db.snapshot_of(ReadTarget::branch("main")).await.unwrap(); + ["edge:Knows", "edge:WorksAt", "node:Company", "node:Person"] + .into_iter() + .filter_map(|table_key| snapshot.dataset(table_key)) + .map(|entry| { + std::path::Path::new(db.uri()) + .join(entry.dataset_path.trim_start_matches('/')) + .join(dir) + }) + .collect() +} + +/// A storage failure while optimize stages its compaction is definite: the +/// error keeps its storage cause and carries `Unpublished`, so a server +/// keeps serving instead of treating the run as uncertain. The graph head is +/// unchanged and the next run on the same handle publishes. The fault is +/// real backend I/O: the table's `data/` directory refuses new files. +#[cfg(unix)] +#[tokio::test] +async fn optimize_storage_failure_before_publication_is_unpublished() { + use omnigraph::error::CompletionEvidence; + + let dir = tempfile::tempdir().unwrap(); + let db = init_and_load(&dir).await; + for name in ["frag-1", "frag-2", "frag-3"] { + insert_person(&db, "main", name).await; + } + let commits_before = db.list_commits(Some("main")).await.unwrap().len(); + let person = node_table_uri(&db, "Person").await; + let data = vec![std::path::Path::new(&person).join("data")]; + if !refuse_writes_under(&data) { + return; + } + let error = db.optimize_as(None).await; + allow_writes_under(&data); + let error = error.expect_err("the compaction cannot write its files"); + assert!( + error.storage_failure().is_some(), + "the compaction's data-file write fails in storage: {error:?}" + ); + assert_eq!( + error.completion_evidence(), + Some(CompletionEvidence::Unpublished), + "a storage failure before publication is definite: {error:?}" + ); + assert_eq!( + db.list_commits(Some("main")).await.unwrap().len(), + commits_before, + "nothing was published" + ); + assert_eq!(db.maintenance_in_progress(), None, "the lane is released"); + + let stats = db.optimize_as(None).await.expect("the next run optimizes"); + assert!( + stats + .iter() + .any(|row| row.type_key == "node:Person" && row.committed), + "{stats:?}" + ); + assert_eq!( + db.list_commits(Some("main")).await.unwrap().len(), + commits_before + 1 + ); +} + +/// The index writer marks its failures the same way: a storage failure +/// while an index build writes its segment files carries `Unpublished`, and +/// the graph is unchanged. The fault is real backend I/O: every table's +/// `_indices/` directory refuses new segments. +#[cfg(unix)] +#[tokio::test] +async fn index_build_storage_failure_before_publication_is_unpublished() { + use omnigraph::error::CompletionEvidence; + + let dir = tempfile::tempdir().unwrap(); + let db = init_and_load(&dir).await; + // Schema apply records a new index's intent and leaves its build to the + // index writer. + db.apply_schema(&TEST_SCHEMA.replace("age: I32?", "age: I32? @index")) + .await + .unwrap(); + let commits_before = db.list_commits(Some("main")).await.unwrap().len(); + let indices = table_dirs(&db, "_indices").await; + if !refuse_writes_under(&indices) { + return; + } + let error = db.ensure_indices_as(None).await; + allow_writes_under(&indices); + let error = error.expect_err("the index build cannot write its segments"); + assert!( + error.storage_failure().is_some(), + "the segment write fails in storage: {error:?}" + ); + assert_eq!( + error.completion_evidence(), + Some(CompletionEvidence::Unpublished), + "a storage failure before publication is definite: {error:?}" + ); + assert_eq!( + db.list_commits(Some("main")).await.unwrap().len(), + commits_before + ); + + db.ensure_indices_as(None) + .await + .expect("the next build publishes"); + assert_eq!( + db.list_commits(Some("main")).await.unwrap().len(), + commits_before + 1 + ); +} + fn assert_same_dataset_entry( before: &omnigraph::db::DatasetEntry, after: &omnigraph::db::DatasetEntry, @@ -130,7 +270,7 @@ async fn optimize_on_empty_graph_returns_stats_per_table_with_no_changes() { let uri = dir.path().to_str().unwrap(); let db = Omnigraph::init(uri, TEST_SCHEMA).await.unwrap(); - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); assert_eq!(stats.len(), 5); for s in &stats { @@ -154,7 +294,7 @@ async fn optimize_after_load_then_again_is_idempotent() { let db = init_and_load(&dir).await; // First pass may compact (load wrote real fragments). - let _first = db.optimize().await.unwrap(); + let _first = db.optimize_as(None).await.unwrap(); let commits_before = db.list_commits(None).await.unwrap(); let head_before = commits_before @@ -165,7 +305,7 @@ async fn optimize_after_load_then_again_is_idempotent() { // Second pass should be a no-op: already-compacted graph produces no // fragments_removed / fragments_added. - let second = db.optimize().await.unwrap(); + let second = db.optimize_as(None).await.unwrap(); for s in &second { assert_eq!( s.fragments_removed, 0, @@ -229,7 +369,7 @@ async fn optimize_preserves_manifest_registry_fragments() { !cold_files.is_empty(), "the fixture must release more registry rows than the hot bound" ); - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); let after = Dataset::open(&manifest_uri).await.unwrap(); let after_files: BTreeSet<_> = after .fragments() @@ -323,7 +463,7 @@ async fn optimize_clears_stale_auto_cleanup_and_preserves_versions() { .unwrap() .len(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let ds = Dataset::open(&manifest_uri).await.unwrap(); // (a) the stale auto_cleanup config was cleared (non-destructive by construction). @@ -363,10 +503,9 @@ async fn optimize_preserves_versions_under_stale_auto_cleanup_config_on_data_tab add_person_fragments(&db).await; // multiple fragments → will_compact // Simulate an upgraded graph: set an aggressive stored auto_cleanup config on - // the Person table. This is an out-of-band Lance commit (an `UpdateConfig` that - // advances HEAD past the manifest), so realign the manifest with a forced repair - // first — otherwise optimize skips the table as uncovered drift and never - // reaches the scrub. (Forced because UpdateConfig is not verified maintenance.) + // the Person table. This is an out-of-band Lance commit (an `UpdateConfig` + // above the table's last linear version); optimize stages on the pin, never + // the linear HEAD, so it still compacts the table. let (_, _, person_full) = person_pin_and_head(&db, &root).await; { let mut ds = Dataset::open(&person_full).await.unwrap(); @@ -377,12 +516,6 @@ async fn optimize_preserves_versions_under_stale_auto_cleanup_config_on_data_tab .await .unwrap(); } - db.repair(RepairOptions { - confirm: true, - force: true, - }) - .await - .unwrap(); let versions_before = Dataset::open(&person_full) .await @@ -393,7 +526,7 @@ async fn optimize_preserves_versions_under_stale_auto_cleanup_config_on_data_tab .len(); let rows_before = count_rows(&db, "node:Person").await; - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let ds = Dataset::open(&person_full).await.unwrap(); assert!( @@ -440,7 +573,7 @@ node Doc { ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); // A second load with NEW keys appends a fragment the existing BTREEs do not // cover (the existence gate skips re-building an index that already exists). @@ -462,7 +595,7 @@ node Doc { ); } - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); // Postcondition: optimize_indices folded the scalar tail in, but preserves // the existing FTS artifact instead of merging uncertified postings. @@ -487,7 +620,7 @@ node Doc { assert!(deferred[0].reason.contains("rebuild-full-text-indexes")); let commits_before = db.list_commits(None).await.unwrap().len(); for _ in 0..2 { - let repeated = db.optimize().await.unwrap(); + let repeated = db.optimize_as(None).await.unwrap(); let doc = repeated .iter() .find(|stat| stat.type_key == "node:Doc") @@ -506,7 +639,10 @@ node Doc { assert_eq!(recovery_sidecar_count(&dir), 0); } assert_eq!(db.list_commits(None).await.unwrap().len(), commits_before); - let rebuilt = db.rebuild_full_text_indices_on("main").await.unwrap(); + let rebuilt = db + .rebuild_full_text_indices_on_as("main", None) + .await + .unwrap(); assert_eq!(rebuilt.rebuilt_indexes.len(), 1); assert_eq!(rebuilt.rebuilt_indexes[0].property, "slug"); let snap = snapshot_main(&db).await.unwrap(); @@ -644,9 +780,10 @@ node Tag {\n slug: String @key\n}\n"; .clone(); let probes = omnigraph::instrumentation::MergeWriteProbes::default(); - let stats = omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize()) - .await - .expect("optimize must not crash on a graph with a Blob table"); + let stats = + omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize_as(None)) + .await + .expect("optimize must not crash on a graph with a Blob table"); assert_eq!( ( probes.compaction_blob_batch_calls(), @@ -778,9 +915,10 @@ async fn optimize_compacts_large_blob_rows_in_bounded_batches() { let commits_before = db.list_commits(None).await.unwrap().len(); let probes = omnigraph::instrumentation::MergeWriteProbes::default(); - let stats = omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize()) - .await - .unwrap(); + let stats = + omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize_as(None)) + .await + .unwrap(); assert_eq!( probes.compaction_blob_batch_calls(), 1, @@ -933,9 +1071,10 @@ async fn optimize_sizes_a_blob_batch_by_the_sum_of_a_rows_blob_columns() { } let probes = omnigraph::instrumentation::MergeWriteProbes::default(); - let stats = omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize()) - .await - .unwrap(); + let stats = + omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize_as(None)) + .await + .unwrap(); assert_eq!(probes.compaction_blob_batch_calls(), 1); assert_eq!( probes.compaction_blob_batch_rows(), @@ -1022,7 +1161,7 @@ async fn optimize_compacts_blob_fragments_with_deleted_rows() { "test precondition: both fragments carry a deletion vector" ); - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); let doc = stats .iter() .find(|stat| stat.type_key == "node:Doc") @@ -1076,14 +1215,15 @@ async fn optimize_sizes_each_compaction_task_from_its_own_fragments() { }; db.load_jsonl(&line, mode).await.unwrap(); if load == 1 { - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); } } let probes = omnigraph::instrumentation::MergeWriteProbes::default(); - let stats = omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize()) - .await - .unwrap(); + let stats = + omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize_as(None)) + .await + .unwrap(); assert_eq!( probes.compaction_blob_batch_calls(), 2, @@ -1159,9 +1299,10 @@ async fn optimize_does_not_size_a_blob_batch_by_external_references() { ); let probes = omnigraph::instrumentation::MergeWriteProbes::default(); - let stats = omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize()) - .await - .unwrap(); + let stats = + omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize_as(None)) + .await + .unwrap(); assert_eq!(probes.compaction_blob_batch_calls(), 1); assert_eq!( probes.compaction_blob_batch_rows(), @@ -1278,8 +1419,8 @@ async fn optimize_then_cleanup_erases_dropped_property_values() { .await .unwrap(); // Settle index work, then prove Lance's planner leaves both tables alone. - db.optimize().await.unwrap(); - for stat in db.optimize().await.unwrap() { + db.optimize_as(None).await.unwrap(); + for stat in db.optimize_as(None).await.unwrap() { assert!(!stat.committed, "test precondition: {stat:?}"); } assert_eq!( @@ -1325,9 +1466,10 @@ async fn optimize_then_cleanup_erases_dropped_property_values() { let commits_before = db.list_commits(None).await.unwrap().len(); let probes = omnigraph::instrumentation::MergeWriteProbes::default(); - let stats = omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize()) - .await - .unwrap(); + let stats = + omnigraph::instrumentation::with_merge_write_probes(probes.clone(), db.optimize_as(None)) + .await + .unwrap(); let stat = |key: &str| stats.iter().find(|stat| stat.type_key == key).unwrap(); let doc = stat("node:Doc"); assert!( @@ -1375,7 +1517,7 @@ async fn optimize_then_cleanup_erases_dropped_property_values() { doc_blob_values(&db, &["content"]).await == expected, "the remaining properties survive the rewrite" ); - for stat in db.optimize().await.unwrap() { + for stat in db.optimize_as(None).await.unwrap() { assert!( !stat.committed, "a second optimize has nothing left: {stat:?}" @@ -1399,8 +1541,8 @@ async fn optimize_then_cleanup_erases_dropped_property_values() { db.branch_merge("retained", "main").await.unwrap().outcome, MergeOutcome::Merged ); - db.optimize().await.unwrap(); - let stats = db.cleanup(keep_one()).await.unwrap(); + db.optimize_as(None).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); for needle in &needles { assert!( @@ -1409,7 +1551,7 @@ async fn optimize_then_cleanup_erases_dropped_property_values() { ); } db.branch_delete("retained").await.unwrap(); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); for needle in &needles { let found = files_containing(dir.path(), needle); @@ -1463,7 +1605,7 @@ async fn optimize_publishes_compaction_to_manifest_so_schema_apply_succeeds() { .get_fragments() .len(); - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); let person = stats .iter() .find(|s| s.type_key == "node:Person") @@ -1523,7 +1665,7 @@ async fn optimize_compacts_the_pin_under_foreign_head_drift() { let (pin_before, head_before, _) = forge_person_compaction_drift(&db, &root).await; let rows_before = count_rows(&db, "node:Person").await; - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); let person = stats .iter() .find(|s| s.type_key == "node:Person") @@ -1541,10 +1683,11 @@ async fn optimize_compacts_the_pin_under_foreign_head_drift() { assert_eq!(count_rows(&db, "node:Person").await, rows_before); } -/// Repair preview names linear commits above the last linear version as -/// `foreign_drift` and publishes nothing. +/// The repair report names linear commits above the last linear version as +/// `foreign_drift` and publishes nothing, and writers stage on the pin +/// regardless: a strict schema apply still succeeds. #[tokio::test] -async fn repair_preview_reports_foreign_drift_without_adopting_it() { +async fn repair_report_names_foreign_drift_without_adopting_it() { let dir = tempfile::tempdir().unwrap(); let root = dir .path() @@ -1555,14 +1698,13 @@ async fn repair_preview_reports_foreign_drift_without_adopting_it() { let db = init_and_load(&dir).await; let (pin_before, head_before, _) = forge_person_compaction_drift(&db, &root).await; - let stats = db - .repair(RepairOptions { - confirm: false, - force: false, - }) - .await - .unwrap(); - assert_eq!(stats.graph_manifest_version, None); + let commits_before = db.list_commits(None).await.unwrap(); + let stats = db.repair_report_as(None).await.unwrap(); + assert_eq!( + db.list_commits(None).await.unwrap(), + commits_before, + "the repair report publishes nothing" + ); let person = stats .datasets .iter() @@ -1580,44 +1722,6 @@ async fn repair_preview_reports_foreign_drift_without_adopting_it() { let (pin_after, head_after, _) = person_pin_and_head(&db, &root).await; assert_eq!(pin_after, pin_before); assert_eq!(head_after, head_before); -} - -/// A confirmed repair never adopts foreign drift, and writers stage on the -/// pin regardless: the strict schema apply still succeeds. -#[tokio::test] -async fn repair_confirm_never_adopts_foreign_drift() { - let dir = tempfile::tempdir().unwrap(); - let root = dir - .path() - .to_str() - .unwrap() - .trim_end_matches('/') - .to_string(); - let db = init_and_load(&dir).await; - let (pin_before, head_before, _) = forge_person_compaction_drift(&db, &root).await; - - let stats = db - .repair(RepairOptions { - confirm: true, - force: false, - }) - .await - .unwrap(); - assert_eq!( - stats.graph_manifest_version, None, - "confirmed repair publishes nothing for foreign drift" - ); - let person = stats - .datasets - .iter() - .find(|s| s.type_key == "node:Person") - .expect("Person repair stat present"); - assert_eq!(person.classification, RepairClassification::ForeignDrift); - assert_eq!(person.action, RepairAction::NoOp); - - let (pin_after, head_after, _) = person_pin_and_head(&db, &root).await; - assert_eq!(pin_after, pin_before); - assert_eq!(head_after, head_before); let desired = TEST_SCHEMA.replace( " age: I32?\n}", @@ -1630,10 +1734,10 @@ async fn repair_confirm_never_adopts_foreign_drift() { assert!(result.applied); } -/// Force does not change the judgement: a foreign raw delete is reported as -/// `foreign_drift`, never adopted, and reads still resolve the pin. +/// A foreign raw delete is reported as `foreign_drift`, never adopted, and +/// reads still resolve the pin. #[tokio::test] -async fn repair_force_never_adopts_a_foreign_delete() { +async fn repair_report_never_adopts_a_foreign_delete() { let dir = tempfile::tempdir().unwrap(); let root = dir .path() @@ -1644,14 +1748,7 @@ async fn repair_force_never_adopts_a_foreign_delete() { let db = init_and_load(&dir).await; let (pin_before, head_before, _) = forge_person_delete_drift(&db, &root).await; - let stats = db - .repair(RepairOptions { - confirm: true, - force: true, - }) - .await - .unwrap(); - assert_eq!(stats.graph_manifest_version, None); + let stats = db.repair_report_as(None).await.unwrap(); let person = stats .datasets .iter() @@ -1717,17 +1814,7 @@ async fn write_on_a_never_written_table_succeeds_over_foreign_drift_and_repair_r "the write neither moves nor adopts the foreign HEAD" ); - let stats = db - .repair(RepairOptions { - confirm: true, - force: false, - }) - .await - .unwrap(); - assert_eq!( - stats.graph_manifest_version, None, - "repair publishes nothing for foreign drift" - ); + let stats = db.repair_report_as(None).await.unwrap(); let company = stats .datasets .iter() @@ -1775,7 +1862,7 @@ async fn full_text_rebuild_replaces_all_columns_and_segments_in_one_publication( db.load_jsonl(r#"{"type":"Person","data":{"name":"Alice","age":30,"biography":"organism university running"}}"#, LoadMode::Merge, ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); helpers::forge_linear_head_from_pin(&db, "main", "node:Person", 0).await; let person_uri = node_table_uri(&db, "Person").await; @@ -1919,7 +2006,10 @@ async fn full_text_rebuild_replaces_all_columns_and_segments_in_one_publication( assert_eq!(recovery_sidecar_count(&dir), 0); // A force rebuild remains a real operation after coverage is complete. - let repeated = db.rebuild_full_text_indices_on("main").await.unwrap(); + let repeated = db + .rebuild_full_text_indices_on_as("main", None) + .await + .unwrap(); assert_eq!(repeated.rebuilt_indexes, expected); assert_ne!(repeated.graph_commit_id, result.graph_commit_id); assert_eq!( @@ -1936,7 +2026,7 @@ async fn full_text_rebuild_on_a_branch_stages_on_the_inherited_location_without_ let db = init_and_load(&dir).await; db.branch_create("feature").await.unwrap(); let inherited = db.snapshot_of(ReadTarget::branch("feature")).await.unwrap(); - db.ensure_indices_on("feature").await.unwrap(); + db.ensure_indices_on_as("feature", None).await.unwrap(); assert_same_dataset_entry( inherited.dataset("node:Person").unwrap(), db.snapshot_of(ReadTarget::branch("feature")) @@ -1950,7 +2040,10 @@ async fn full_text_rebuild_on_a_branch_stages_on_the_inherited_location_without_ let main_commits_before = db.list_commits(None).await.unwrap(); let feature_commits_before = db.list_commits(Some("feature")).await.unwrap(); - let result = db.rebuild_full_text_indices_on("feature").await.unwrap(); + let result = db + .rebuild_full_text_indices_on_as("feature", None) + .await + .unwrap(); assert_eq!(result.branch, "feature"); assert_eq!(result.rebuilt_indexes.len(), 2); let feature = db.snapshot_of(ReadTarget::branch("feature")).await.unwrap(); @@ -2036,7 +2129,10 @@ async fn full_text_rebuild_reports_empty_builds_but_not_no_work() { .await .unwrap(); let before = snapshot_main(&db).await.unwrap(); - let result = db.rebuild_full_text_indices_on("main").await.unwrap(); + let result = db + .rebuild_full_text_indices_on_as("main", None) + .await + .unwrap(); assert_eq!(result.branch, "main"); assert_eq!(result.graph_commit_id, None); assert!(result.rebuilt_indexes.is_empty()); @@ -2051,7 +2147,10 @@ async fn full_text_rebuild_reports_empty_builds_but_not_no_work() { .await .unwrap(); let before = snapshot_main(&db).await.unwrap(); - let result = db.rebuild_full_text_indices_on("main").await.unwrap(); + let result = db + .rebuild_full_text_indices_on_as("main", None) + .await + .unwrap(); assert_eq!(result.rebuilt_indexes.len(), 2); assert!(result.graph_commit_id.is_some()); let after = snapshot_main(&db).await.unwrap(); @@ -2138,7 +2237,10 @@ async fn full_text_rebuild_refuses_unsupported_physical_inventory_before_effects .version, ); } - let error = db.rebuild_full_text_indices_on("main").await.unwrap_err(); + let error = db + .rebuild_full_text_indices_on_as("main", None) + .await + .unwrap_err(); assert!( error.to_string().contains("unsupported_internal_id_fts"), "{error}" @@ -2181,7 +2283,7 @@ async fn cleanup_without_any_policy_option_errors() { let db = init_and_load(&dir).await; let err = db - .cleanup(CleanupPolicyOptions::default()) + .cleanup_as(CleanupPolicyOptions::default(), None) .await .expect_err("cleanup with no policy options must error"); @@ -2211,10 +2313,13 @@ async fn cleanup_keep_one_preserves_head_and_table_remains_readable() { ); let _stats = db - .cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); @@ -2247,7 +2352,7 @@ async fn cleanup_keep_exceeding_history_preserves_every_available_version() { ); let pins = pins_in_history(&db, "main", "node:Person").await; - db.cleanup(keep_ten).await.unwrap(); + db.cleanup_as(keep_ten, None).await.unwrap(); // Every version a commit pins stays; only a chain's links below its // pinned tip (the first load's effect beneath its full-text declaration) @@ -2269,10 +2374,13 @@ async fn cleanup_older_than_zero_preserves_head() { // Lance must still preserve the head manifest, so the table is openable // afterwards and a subsequent load still works. let _stats = db - .cleanup(CleanupPolicyOptions { - keep_versions: None, - older_than: Some(Duration::from_secs(0)), - }) + .cleanup_as( + CleanupPolicyOptions { + keep_versions: None, + older_than: Some(Duration::from_secs(0)), + }, + None, + ) .await .unwrap(); @@ -2311,10 +2419,13 @@ async fn cleanup_preserves_main_version_pinned_by_live_lazy_branch() { "precondition: lazy-branch pin must fall outside keep=1 retention" ); - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); @@ -2328,10 +2439,13 @@ async fn cleanup_preserves_main_version_pinned_by_live_lazy_branch() { // timestamp and version predicates with AND, so injecting the branch pin // as `before_version` keeps the inherited version even when every old // manifest satisfies the timestamp cutoff. - db.cleanup(CleanupPolicyOptions { - keep_versions: None, - older_than: Some(Duration::from_secs(0)), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: None, + older_than: Some(Duration::from_secs(0)), + }, + None, + ) .await .unwrap(); assert_eq!( @@ -2369,10 +2483,13 @@ async fn cleanup_retains_the_commit_a_branch_starts_at() { db.branch_create_from(at(&loaded), "at-load").await.unwrap(); let people_at_load = count_rows_branch(&db, "at-load", "node:Person").await; - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert_eq!( @@ -2428,10 +2545,13 @@ async fn cleanup_uses_oldest_pin_across_multiple_live_lazy_branches() { .await .unwrap(); - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); @@ -2471,10 +2591,13 @@ async fn cleanup_fails_closed_when_live_lazy_branch_pin_is_unopenable() { let person_before = detached_versions(&person_uri).await; let stats = db - .cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); let row = |type_key: &str| { @@ -2517,7 +2640,7 @@ async fn cleanup_reports_foreign_head_drift_and_keeps_it() { .expect("every registration records its last linear version"); let foreign: Vec = (last_linear + 1..=head_version).collect(); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); let person = stats .iter() .find(|row| row.type_key == "node:Person") @@ -2557,13 +2680,16 @@ async fn cleanup_then_optimize_preserves_rows_and_table_remains_writable() { "fixture should seed both Person and Company rows" ); - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); // Head is preserved through both ops. assert_eq!(count_rows(&db, "node:Person").await, people_before); @@ -2643,10 +2769,13 @@ async fn cleanup_reconciles_orphaned_branch_forks() { ); } - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); @@ -2695,10 +2824,13 @@ async fn cleanup_reconciles_orphaned_branch_forks() { ); // Idempotent: a second cleanup with the orphan already gone is a no-op. - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert!( @@ -2774,10 +2906,13 @@ async fn cleanup_reconciles_live_branch_orphan_fork_and_keeps_the_branch_write() let main_people = count_rows(&db, "node:Person").await; let main_companies = count_rows(&db, "node:Company").await; - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); @@ -2877,10 +3012,13 @@ async fn cleanup_age_window_preserves_a_recent_snapshot_across_a_pointer_switch( .await .unwrap(); assert_eq!(before.num_rows(), base_count + 1); - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: Some(Duration::from_secs(30 * 24 * 60 * 60)), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: Some(Duration::from_secs(30 * 24 * 60 * 60)), + }, + None, + ) .await .unwrap(); assert!( @@ -2898,10 +3036,13 @@ async fn cleanup_age_window_preserves_a_recent_snapshot_across_a_pointer_switch( .await .unwrap(); assert_eq!(after.num_rows(), base_count + 1); - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: Some(Duration::ZERO), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: Some(Duration::ZERO), + }, + None, + ) .await .unwrap(); assert!( @@ -2931,10 +3072,13 @@ async fn cleanup_age_window_preserves_recent_ref_absent_descendant() { .set_times(std::fs::FileTimes::new().set_modified(std::time::SystemTime::UNIX_EPOCH)) .unwrap(); std::fs::write(&recent_file, b"recent interrupted residue").unwrap(); - db.cleanup(CleanupPolicyOptions { - keep_versions: None, - older_than: Some(Duration::from_secs(30 * 24 * 60 * 60)), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: None, + older_than: Some(Duration::from_secs(30 * 24 * 60 * 60)), + }, + None, + ) .await .unwrap(); assert!( @@ -2945,10 +3089,13 @@ async fn cleanup_age_window_preserves_recent_ref_absent_descendant() { recent_file.exists(), "a missing ref is not permission to discard a recent object" ); - db.cleanup(CleanupPolicyOptions { - keep_versions: None, - older_than: Some(Duration::ZERO), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: None, + older_than: Some(Duration::ZERO), + }, + None, + ) .await .unwrap(); assert!( @@ -3018,10 +3165,13 @@ async fn cleanup_reclaims_retirement_archive_after_the_age_window() { > std::time::SystemTime::UNIX_EPOCH, "retirement writes a recent archive even when the tree is old" ); - db.cleanup(CleanupPolicyOptions { - keep_versions: None, - older_than: Some(Duration::from_secs(30 * 24 * 60 * 60)), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: None, + older_than: Some(Duration::from_secs(30 * 24 * 60 * 60)), + }, + None, + ) .await .unwrap(); assert!( @@ -3032,10 +3182,13 @@ async fn cleanup_reclaims_retirement_archive_after_the_age_window() { tree.exists(), "the recent retirement metadata must protect the aged physical tree" ); - db.cleanup(CleanupPolicyOptions { - keep_versions: None, - older_than: Some(Duration::ZERO), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: None, + older_than: Some(Duration::ZERO), + }, + None, + ) .await .unwrap(); assert!( @@ -3094,10 +3247,13 @@ async fn cleanup_age_window_preserves_recent_native_mutation_and_aged_parent() { let changed = child.delete("name = 'Alice'").await.unwrap(); assert_eq!(changed.num_deleted_rows, 1); let latest = changed.new_dataset.version().version; - db.cleanup(CleanupPolicyOptions { - keep_versions: None, - older_than: Some(Duration::from_secs(30 * 24 * 60 * 60)), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: None, + older_than: Some(Duration::from_secs(30 * 24 * 60 * 60)), + }, + None, + ) .await .unwrap(); let branches = Dataset::open(&person_uri) @@ -3129,10 +3285,13 @@ async fn cleanup_age_window_preserves_recent_native_mutation_and_aged_parent() { retained.count_rows(None).await.unwrap(), count_rows(&db, "node:Person").await - 1 ); - db.cleanup(CleanupPolicyOptions { - keep_versions: None, - older_than: Some(Duration::ZERO), - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: None, + older_than: Some(Duration::ZERO), + }, + None, + ) .await .unwrap(); let branches = Dataset::open(&person_uri) @@ -3228,10 +3387,13 @@ async fn cleanup_preserves_the_pin_a_lazy_child_reads_after_its_parent_retires() .clone(); db.branch_delete("feature").await.unwrap(); db.branch_create("feature").await.unwrap(); - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); @@ -3290,10 +3452,13 @@ async fn cleanup_reclaims_dead_incarnation_fork_of_live_branch() { ds.create_branch(&dead_native, base, None).await.unwrap(); } - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); @@ -3359,10 +3524,13 @@ async fn cleanup_reclaims_dead_incarnation_fork_of_live_branch() { db.apply_schema(TEST_SCHEMA).await.unwrap(); let replacement_uri = node_table_uri(&db, "Company").await; assert_ne!(replacement_uri, company_uri); - db.cleanup(CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert!( @@ -3421,7 +3589,7 @@ async fn index_build_tolerates_null_vector_rows() { // Must not abort: the untrainable vector column is deferred, while id, // slug (FTS), and n (BTREE) are built together in one table transaction. let pending = db - .ensure_indices() + .ensure_indices_as(None) .await .expect("ensure_indices must not abort when a vector column has no trainable vectors yet"); assert_eq!(pending.len(), 1, "only the null vector index is pending"); @@ -3488,7 +3656,7 @@ async fn optimize_materializes_index_declared_but_unbuilt() { ); } - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); // Postcondition: optimize's reconciler materialized the declared index. let snap = snapshot_main(&db).await.unwrap(); @@ -3544,7 +3712,7 @@ async fn optimize_materializes_index_after_type_rename() { ); } - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let snap = snapshot_main(&db).await.unwrap(); let ds = snap.open_dataset("node:Item").await.unwrap(); @@ -3576,7 +3744,7 @@ async fn collector_retains_malformed_witnesses_after_retirement_history_is_gone( let identity = manifest.branches().get(&native).await.unwrap().identifier; assert_ne!(identity, BranchIdentifier::main()); db.branch_delete("retired").await.unwrap(); - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!(!dir.path().join("__manifest/tree").join(&native).exists()); let identity = serde_json::to_string(&identity).unwrap(); @@ -3653,7 +3821,7 @@ async fn collector_retains_malformed_witnesses_after_retirement_history_is_gone( ); } } - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); let remaining = detached_versions(&table_uri).await; for (label, version) in versions { @@ -3707,7 +3875,7 @@ async fn collector_sweeps_the_pin_that_only_a_pruned_manifest_version_names() { ); assert!(plan.sweep_paths.is_disjoint(&plan.marked_paths)); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); let row = stats .iter() .find(|row| row.type_key == "node:Person") @@ -3921,7 +4089,7 @@ async fn collector_repeat_run_discovers_what_an_earlier_pass_left() { async fn collector_finds_nothing_to_sweep_after_todays_cleanup() { let dir = tempfile::tempdir().unwrap(); let db = init_and_load(&dir).await; - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); insert_person(&db, "main", "Eve").await; mutate_main( &db, @@ -3931,7 +4099,7 @@ async fn collector_finds_nothing_to_sweep_after_todays_cleanup() { ) .await .unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); db.branch_create("feature").await.unwrap(); insert_person(&db, "feature", "Frank").await; db.branch_merge("feature", "main").await.unwrap(); @@ -3941,7 +4109,7 @@ async fn collector_finds_nothing_to_sweep_after_todays_cleanup() { ) .await .unwrap(); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); for row in &stats { assert!(row.error.is_none(), "every table's version GC ran: {row:?}"); } @@ -3990,10 +4158,13 @@ async fn cleanup_on_a_detached_only_graph_counts_manifest_versions_for_keep() { let third = staged_since(&person_uri, &after_second).await; let stats = db - .cleanup(CleanupPolicyOptions { - keep_versions: Some(2), - older_than: None, - }) + .cleanup_as( + CleanupPolicyOptions { + keep_versions: Some(2), + older_than: None, + }, + None, + ) .await .unwrap(); let person = |stats: &[omnigraph::db::DatasetCleanupStats]| { @@ -4018,7 +4189,7 @@ async fn cleanup_on_a_detached_only_graph_counts_manifest_versions_for_keep() { assert!(!remaining.contains(&first), "{remaining:?}"); assert!(remaining.contains(&second) && remaining.contains(&third)); - let row = person(&db.cleanup(keep_one()).await.unwrap()); + let row = person(&db.cleanup_as(keep_one(), None).await.unwrap()); assert_eq!( row.manifests_removed, 1, "keep 1 prunes the publication that named the second pin: {row:?}" @@ -4027,7 +4198,7 @@ async fn cleanup_on_a_detached_only_graph_counts_manifest_versions_for_keep() { assert!(!remaining.contains(&second), "{remaining:?}"); assert!(remaining.contains(&third)); - let row = person(&db.cleanup(keep_one()).await.unwrap()); + let row = person(&db.cleanup_as(keep_one(), None).await.unwrap()); assert_eq!(row.manifests_removed, 0, "a rerun finds nothing: {row:?}"); let report = db.cleanup_plan(keep_one()).await.unwrap(); for plan in &report.tables { @@ -4222,7 +4393,7 @@ async fn collector_reclaims_aged_orphans_without_deleting_recent_blob_sidecars() .unwrap(); } let rows_before = count_rows(&db, "node:Person").await; - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); for relative in recent { assert!( @@ -4290,7 +4461,7 @@ async fn collector_reclaims_blob_sidecars_after_the_last_retained_pin() { db.load_jsonl(&row(¤t_payload), LoadMode::Overwrite) .await .unwrap(); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); for path in &old_sidecars { assert!( @@ -4311,7 +4482,7 @@ async fn collector_reclaims_blob_sidecars_after_the_last_retained_pin() { let report = db.cleanup_plan(keep_one()).await.unwrap(); let plan = main_plan(&report, "node:Doc"); assert!(plan.errors.is_empty(), "{:?}", plan.errors); - let stats = db.cleanup(keep_one()).await.unwrap(); + let stats = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(stats.iter().all(|row| row.error.is_none()), "{stats:?}"); for path in &old_sidecars { assert!( diff --git a/crates/omnigraph/tests/integration/merge_cleanup.rs b/crates/omnigraph/tests/integration/merge_cleanup.rs index 471842586..bad2124f8 100644 --- a/crates/omnigraph/tests/integration/merge_cleanup.rs +++ b/crates/omnigraph/tests/integration/merge_cleanup.rs @@ -36,7 +36,7 @@ async fn repeated_merge_keeps_its_selected_base_through_cleanup() { for age in 1..=3 { set_age(&db, "source", "p", age).await; set_age(&db, "target", "q", age).await; - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); let reopened = helpers::session(Omnigraph::open(uri).await.unwrap()); assert_eq!( @@ -106,7 +106,7 @@ async fn imported_merge_base_survives_owner_retirement_and_recreation() { .starts_with("zz-unrelated.") }) .expect("unrelated retirement archive exists before cleanup"); - let rows = db.cleanup(keep_one()).await.unwrap(); + let rows = db.cleanup_as(keep_one(), None).await.unwrap(); assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); assert!( !unrelated.exists(), diff --git a/crates/omnigraph/tests/integration/point_in_time.rs b/crates/omnigraph/tests/integration/point_in_time.rs index 6f088bd5c..4cb87dc97 100644 --- a/crates/omnigraph/tests/integration/point_in_time.rs +++ b/crates/omnigraph/tests/integration/point_in_time.rs @@ -1184,7 +1184,10 @@ async fn historical_read_of_reclaimed_version_is_typed() { .await .unwrap(); - let stats = db.cleanup(helpers::collector::keep_one()).await.unwrap(); + let stats = db + .cleanup_as(helpers::collector::keep_one(), None) + .await + .unwrap(); let person = stats .iter() .find(|row| row.type_key == "node:Person") diff --git a/crates/omnigraph/tests/integration/recovery.rs b/crates/omnigraph/tests/integration/recovery.rs index 0a3608558..f0cd513b5 100644 --- a/crates/omnigraph/tests/integration/recovery.rs +++ b/crates/omnigraph/tests/integration/recovery.rs @@ -105,12 +105,12 @@ async fn recovery_ensure_indices_steady_state_no_sidecar() { {"type":"Company","data":{"name":"acme"}} "#; db.load_jsonl(test_data, LoadMode::Append).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); drop(db); let db = Omnigraph::open(uri).await.unwrap(); let before = snapshot_main(&db).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let after = snapshot_main(&db).await.unwrap(); assert_eq!( after.graph_manifest_version(), @@ -153,7 +153,7 @@ async fn recovery_ensure_indices_handles_empty_tables() { let db = Omnigraph::init(uri, TEST_SCHEMA).await.unwrap(); // Don't load any data — every table is empty. let before = snapshot_main(&db).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let after = snapshot_main(&db).await.unwrap(); assert_eq!( after.graph_manifest_version(), @@ -173,7 +173,7 @@ async fn recovery_ensure_indices_handles_empty_tables() { drop(db); let db = Omnigraph::open(uri).await.unwrap(); let before = snapshot_main(&db).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let after = snapshot_main(&db).await.unwrap(); assert_eq!( after.graph_manifest_version(), diff --git a/crates/omnigraph/tests/integration/repro_issue_563.rs b/crates/omnigraph/tests/integration/repro_issue_563.rs index 409ce09ac..733ca35c8 100644 --- a/crates/omnigraph/tests/integration/repro_issue_563.rs +++ b/crates/omnigraph/tests/integration/repro_issue_563.rs @@ -141,7 +141,7 @@ async fn ranked_read_with_join_returns_top_limit_issue_563() { db.load_jsonl(&edges, LoadMode::Append).await.unwrap(); } - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); // The ranked read without the join stays under the i32 offset ceiling. let no_join = query_main( diff --git a/crates/omnigraph/tests/integration/rrf_prefilter_gate.rs b/crates/omnigraph/tests/integration/rrf_prefilter_gate.rs index db0cdf997..121cef7f6 100644 --- a/crates/omnigraph/tests/integration/rrf_prefilter_gate.rs +++ b/crates/omnigraph/tests/integration/rrf_prefilter_gate.rs @@ -265,7 +265,7 @@ async fn init_gate_db(dir: &tempfile::TempDir) -> Session { db.load_jsonl(&gate_seed_data(), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } @@ -457,7 +457,7 @@ async fn init_stats_db(dir: &tempfile::TempDir) -> Session { db.load_jsonl(&stats_seed_data(), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } @@ -614,7 +614,7 @@ async fn natural_gate_prefilters_at_ratio_boundary() { db.load_jsonl(&rows.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let db = db; let text_params = params(&[("$q1", "needle"), ("$q2", "sharp")]); @@ -767,7 +767,7 @@ async fn empty_eligible_set_falls_back() { ) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let db = db; let text_params = params(&[("$q1", "needle"), ("$q2", "sharp")]); @@ -807,7 +807,7 @@ async fn natural_gate_prefilters_selective_fixture() { db.load_jsonl(&rows.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let db = db; let text_params = params(&[("$q1", "needle"), ("$q2", "sharp")]); @@ -880,7 +880,7 @@ async fn decoy_flood_winner_holds_under_both_plans() { db.load_jsonl(&rows.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let db = db; let text_params = params(&[("$q1", "alpha"), ("$q2", "beta")]); @@ -926,7 +926,7 @@ async fn natural_gate_falls_back_on_broad_fixture() { db.load_jsonl(&rows.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let db = db; let text_params = params(&[("$q1", "needle"), ("$q2", "sharp")]); diff --git a/crates/omnigraph/tests/integration/scalar_indexes.rs b/crates/omnigraph/tests/integration/scalar_indexes.rs index a87b5be4a..29443468b 100644 --- a/crates/omnigraph/tests/integration/scalar_indexes.rs +++ b/crates/omnigraph/tests/integration/scalar_indexes.rs @@ -42,7 +42,7 @@ async fn node_scalar_and_enum_index_columns_get_btree() { let uri = dir.path().to_str().unwrap(); let db = session(Omnigraph::init(uri, SCHEMA).await.unwrap()); db.load_jsonl(DATA, LoadMode::Overwrite).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let snap = snapshot_main(&db).await.unwrap(); let ds = snap.open_dataset("node:Item").await.unwrap(); @@ -74,7 +74,7 @@ async fn node_scalar_and_enum_index_columns_get_btree() { // Idempotence: a second run finds every declared index present and // publishes nothing (dual indexes must not re-plan as missing forever). let version_before = ds.published_dataset_version(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let snap2 = snapshot_main(&db).await.unwrap(); let ds2 = snap2.open_dataset("node:Item").await.unwrap(); assert_eq!( diff --git a/crates/omnigraph/tests/integration/traversal_adaptive.rs b/crates/omnigraph/tests/integration/traversal_adaptive.rs index dc3f70ce3..e393372ff 100644 --- a/crates/omnigraph/tests/integration/traversal_adaptive.rs +++ b/crates/omnigraph/tests/integration/traversal_adaptive.rs @@ -83,7 +83,7 @@ async fn clique_db(dir: &tempfile::TempDir) -> Session { .unwrap(); // The endpoint BTREEs must exist, or dispatch prices a degraded scan and // takes CSR outright — the switch under test would never arm. - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); db } @@ -189,7 +189,7 @@ query far($name: String) { jsonl.push('\n'); let db = session(Omnigraph::init(uri, TEST_SCHEMA).await.unwrap()); db.load_jsonl(&jsonl, LoadMode::Overwrite).await.unwrap(); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let p = params(&[("$name", "p0")]); let csr_db = with_traversal(&db, Traversal::Csr); @@ -322,7 +322,7 @@ async fn corrupt_artifact_falls_back_to_the_scan_build() { async fn stale_artifact_is_rejected_after_an_edge_write() { let dir = tempfile::tempdir().unwrap(); let db = init_and_load(&dir).await; - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let artifact = dir.path().join("__graph_index/csr-current.bin"); assert!(artifact.is_file()); diff --git a/crates/omnigraph/tests/integration/warm_read_cost.rs b/crates/omnigraph/tests/integration/warm_read_cost.rs index d9e05e316..172c2d8b4 100644 --- a/crates/omnigraph/tests/integration/warm_read_cost.rs +++ b/crates/omnigraph/tests/integration/warm_read_cost.rs @@ -956,10 +956,13 @@ async fn assert_cached_borrower_survives_branch_delete( "{branch} payload changed" ); } - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); assert!( @@ -995,10 +998,13 @@ async fn assert_cached_borrower_survives_branch_delete( .unwrap(); let recreated_pin = helpers::pinned_version(writer, "feature", "node:Person").await; assert_ne!(recreated_pin, source_pin); - db.cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + db.cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); let pins = helpers::collector::detached_versions(&table_uri).await; diff --git a/crates/omnigraph/tests/integration/write_cost.rs b/crates/omnigraph/tests/integration/write_cost.rs index d33fd8053..7182c64e1 100644 --- a/crates/omnigraph/tests/integration/write_cost.rs +++ b/crates/omnigraph/tests/integration/write_cost.rs @@ -38,7 +38,7 @@ use crate::helpers::{ // `_graph_commits`/`_graph_commit_actors` tables are retired), so the manifest scan // also covers the lineage and actor rows the authenticated write path appends. // Without internal-table compaction these scans are O(fragments) and grow forever; -// step 2 brings the internal tables into `db.optimize()`, so after compaction the +// step 2 brings the internal tables into `db.optimize_as(None)`, so after compaction the // per-write scan is flat. The test runs the **authenticated (actorful) write path** // — every commit carries an actor — and compacts at each depth checkpoint before // measuring, pinning the production invariant "a periodically-compacted graph's @@ -62,7 +62,7 @@ async fn internal_table_scans_are_flat_in_history() { } // Step 2: compaction folds all three internal tables' O(depth) fragments back // to a small constant, so the following write's scan of them is flat. - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); let io = measure_insert_as(&db, &format!("lock_{d}"), ACTOR).await; current += 1; // the measured write advanced depth by one eprintln!( @@ -87,7 +87,7 @@ async fn compacted_graph_at_depth(depth: u64) -> (u64, tempfile::TempDir, helper let dir = tempfile::tempdir().unwrap(); let db = local_graph(&dir).await; commit_many(&db, depth as usize).await; - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); (depth, dir, db) } @@ -106,7 +106,7 @@ async fn ensure_indices_manifest_reads_are_flat_in_history() { for (depth, _dir, db) in [graphs.0, graphs.1] { let indexed_schema = helpers::TEST_SCHEMA.replace("age: I32?", "age: I32? @index"); db.apply_schema(&indexed_schema).await.unwrap(); - let (result, io) = measure(db.ensure_indices()).await; + let (result, io) = measure(db.ensure_indices_as(None)).await; result.unwrap(); eprintln!( "ensure_indices depth~{depth}: __manifest={} data={}", @@ -147,7 +147,7 @@ async fn ensure_indices_writes_no_control_object() { let before_write_text = counts.write_text(); let before_delete = counts.delete(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); assert_eq!( counts.write_text() - before_write_text, 0, @@ -274,7 +274,7 @@ async fn optimize_writes_no_control_object() { let before_write_text = counts.write_text(); let before_delete = counts.delete(); - let stats = db.optimize().await.unwrap(); + let stats = db.optimize_as(None).await.unwrap(); assert!( stats .iter() @@ -310,7 +310,7 @@ async fn optimize_manifest_reads_are_flat_in_history() { commit_many(&db, 3).await; let commits_before = db.list_commits(None).await.unwrap().len(); - let (result, io) = measure(db.optimize()).await; + let (result, io) = measure(db.optimize_as(None)).await; result.unwrap(); assert_eq!( db.list_commits(None).await.unwrap().len(), @@ -357,7 +357,7 @@ async fn internal_table_scans_are_flat_without_compaction() { commit_many_as(&db, (d - current) as usize, ACTOR).await; current = d; } - // NO `db.optimize()` here — that omission is the whole point. The flat gate + // NO `db.optimize_as(None)` here — that omission is the whole point. The flat gate // above compacts before measuring and so never exercises this served regime. let io = measure_insert_as(&db, &format!("served_{d}"), ACTOR).await; current += 1; // the measured write advanced depth by one diff --git a/crates/omnigraph/tests/policy_engine_chassis.rs b/crates/omnigraph/tests/policy_engine_chassis.rs index b0b806cc4..fa75a8553 100644 --- a/crates/omnigraph/tests/policy_engine_chassis.rs +++ b/crates/omnigraph/tests/policy_engine_chassis.rs @@ -8,7 +8,10 @@ //! PR #2 wired `apply_schema_as`. PR #3 fans the same `enforce()` call //! out to the remaining six writers — `mutate_as`, `load_as`, //! `ingest_as`, `branch_create_as` / `branch_create_from_as`, -//! `branch_delete_as`, `branch_merge_as`. Each writer pair below +//! `branch_delete_as`, `branch_merge_as`. The maintenance entry points — +//! `optimize_as`, `ensure_indices_as` / `ensure_indices_on_as`, +//! `rebuild_full_text_indices_on_as`, `repair_report_as` (`maintain`) and +//! `cleanup_as` (`reclaim`) — are covered the same way. Each writer pair below //! covers allow + deny via the engine-side gate; the allow case proves //! the enforce call is correctly scoped (i.e. doesn't reject a legit //! actor), the deny case proves it actually denies an unauthorized @@ -111,6 +114,31 @@ async fn init_with_policy_and_feature_branch( /// One JSONL record for `load_as` / `ingest_as` exercises. const ONE_PERSON_JSONL: &str = r#"{"type": "Person", "data": {"name": "Eve"}}"#; +/// Every directory and file under `root`, with file bytes: what a refused +/// call must leave as it found it. +fn physical_tree(root: &Path) -> std::collections::BTreeMap>> { + fn visit( + root: &Path, + path: &Path, + entries: &mut std::collections::BTreeMap>>, + ) { + for entry in fs::read_dir(path).unwrap() { + let entry = entry.unwrap(); + let path = entry.path(); + let relative = path.strip_prefix(root).unwrap().to_path_buf(); + if entry.file_type().unwrap().is_dir() { + entries.insert(relative, None); + visit(root, &path, entries); + } else { + entries.insert(relative, Some(fs::read(path).unwrap())); + } + } + } + let mut entries = std::collections::BTreeMap::new(); + visit(root, root, &mut entries); + entries +} + fn assert_denied(result: Result, what: &str) { match result { Err(OmniError::Policy(msg)) => { @@ -801,51 +829,44 @@ async fn non_canonical_branch_names_cannot_reach_a_protected_branch() { } #[tokio::test] -async fn full_text_rebuild_enforces_selected_branch_before_effects_and_records_actor() { - fn physical_tree( - root: &Path, - ) -> std::collections::BTreeMap>> { - fn visit( - root: &Path, - path: &Path, - entries: &mut std::collections::BTreeMap>>, - ) { - for entry in fs::read_dir(path).unwrap() { - let entry = entry.unwrap(); - let path = entry.path(); - let relative = path.strip_prefix(root).unwrap().to_path_buf(); - if entry.file_type().unwrap().is_dir() { - entries.insert(relative, None); - visit(root, &path, entries); - } else { - entries.insert(relative, Some(fs::read(path).unwrap())); - } - } - } - let mut entries = std::collections::BTreeMap::new(); - visit(root, root, &mut entries); - entries - } - +async fn full_text_rebuild_enforces_change_or_maintain_on_the_selected_branch_and_records_actor() { let dir = tempfile::tempdir().unwrap(); let db = init_and_load(&dir).await; db.branch_create("feature").await.unwrap(); drop(db); - // This actor can change unprotected feature, but not protected main. - // A hardcoded main scope or a different maintenance action must fail. - let policy = POLICY_YAML.replace(" branch_scope: any", " branch_scope: unprotected"); + // `act-allowed` can change unprotected feature, `act-maintainer` can + // maintain it, and neither reaches protected main. A hardcoded main scope + // or an action other than `change` or `maintain` must fail. + let policy = format!( + "{} - id: maintainers\n allow:\n actors: {{ group: maintainers }}\n actions: [maintain]\n branch_scope: unprotected\n", + POLICY_YAML + .replace(" branch_scope: any", " branch_scope: unprotected") + .replace( + " readers: [act-denied]\n", + " readers: [act-denied]\n maintainers: [act-maintainer]\n", + ) + ); let db = Omnigraph::open(dir.path().to_str().unwrap()).await.unwrap(); let (db, _engine) = install_policy_source(db, dir.path(), &policy); let main_before = db.list_commits(None).await.unwrap(); let feature_before = db.list_commits(Some("feature")).await.unwrap(); let files_before = physical_tree(dir.path()); - for (branch, actor) in [("feature", "act-denied"), ("main", "act-allowed")] { - assert_denied( - db.rebuild_full_text_indices_on_as(branch, Some(actor)) - .await, - "rebuild_full_text_indices_on_as", + for (branch, actor) in [ + ("feature", "act-denied"), + ("main", "act-allowed"), + ("main", "act-maintainer"), + ] { + let result = db + .rebuild_full_text_indices_on_as(branch, Some(actor)) + .await; + // Either grant would admit the rebuild, so the refusal names both. + assert!( + matches!(&result, Err(OmniError::Policy(message)) + if message.contains("'maintain'") && message.contains("'change'")), + "{actor} on {branch}: {result:?}" ); + assert_denied(result, "rebuild_full_text_indices_on_as"); assert_eq!( physical_tree(dir.path()), files_before, @@ -853,7 +874,7 @@ async fn full_text_rebuild_enforces_selected_branch_before_effects_and_records_a ); } let error = db - .rebuild_full_text_indices_on("feature") + .rebuild_full_text_indices_on_as("feature", None) .await .unwrap_err(); assert!( @@ -863,25 +884,36 @@ async fn full_text_rebuild_enforces_selected_branch_before_effects_and_records_a assert_eq!( physical_tree(dir.path()), files_before, - "the no-actor wrapper must not bypass policy" + "a missing actor must not bypass policy" ); - let result = db - .rebuild_full_text_indices_on_as("feature", Some("act-allowed")) - .await - .expect("Change on the selected unprotected branch must permit a real rebuild"); - assert_eq!(result.branch, "feature"); - assert_eq!(result.rebuilt_indexes.len(), 2); - let feature_after = db.list_commits(Some("feature")).await.unwrap(); - assert_eq!(feature_after.len(), feature_before.len() + 1); - assert_eq!( - result.graph_commit_id.as_deref(), - Some(feature_after[0].graph_commit_id.as_str()) - ); - assert_eq!(feature_after[0].actor_id.as_deref(), Some("act-allowed")); + let mut parent = feature_before[0].graph_commit_id.clone(); + for (actor, grant) in [("act-allowed", "change"), ("act-maintainer", "maintain")] { + let result = db + .rebuild_full_text_indices_on_as("feature", Some(actor)) + .await + .unwrap_or_else(|error| { + panic!( + "{grant} on the selected unprotected branch must permit a real rebuild: {error}" + ) + }); + assert_eq!(result.branch, "feature"); + assert_eq!(result.rebuilt_indexes.len(), 2); + let feature_after = db.list_commits(Some("feature")).await.unwrap(); + assert_eq!( + result.graph_commit_id.as_deref(), + Some(feature_after[0].graph_commit_id.as_str()) + ); + assert_eq!(feature_after[0].actor_id.as_deref(), Some(actor)); + assert_eq!( + feature_after[0].parent_commit_id.as_deref(), + Some(parent.as_str()) + ); + parent = feature_after[0].graph_commit_id.clone(); + } assert_eq!( - feature_after[0].parent_commit_id.as_deref(), - Some(feature_before[0].graph_commit_id.as_str()) + db.list_commits(Some("feature")).await.unwrap().len(), + feature_before.len() + 2 ); let main_after = db.list_commits(None).await.unwrap(); assert_eq!(main_after.len(), main_before.len()); @@ -891,6 +923,176 @@ async fn full_text_rebuild_enforces_selected_branch_before_effects_and_records_a ); } +/// Maintenance grants, apart from data writes: `act-maintainer` may maintain +/// any branch and `act-branch-maintainer` only unprotected ones; +/// `act-reclaimer` may reclaim; `act-writer` holds `change` everywhere and no +/// maintenance grant; `act-denied` holds nothing. +const MAINTENANCE_POLICY_YAML: &str = r#" +version: 1 +groups: + maintainers: [act-maintainer] + branch-maintainers: [act-branch-maintainer] + reclaimers: [act-reclaimer] + writers: [act-writer] + readers: [act-denied] +protected_branches: [main] +rules: + - id: maintainers + allow: + actors: { group: maintainers } + actions: [maintain] + branch_scope: any + - id: branch-maintainers + allow: + actors: { group: branch-maintainers } + actions: [maintain] + branch_scope: unprotected + - id: reclaimers + allow: + actors: { group: reclaimers } + actions: [reclaim] + - id: writers + allow: + actors: { group: writers } + actions: [change] + branch_scope: any +"#; + +/// A graph with a `feature` branch and two extra Person fragments on main, so +/// optimize has a compaction to publish, under [`MAINTENANCE_POLICY_YAML`]. +async fn init_with_maintenance_policy(dir: &tempfile::TempDir) -> Session { + let db = init_and_load(dir).await; + db.branch_create("feature").await.unwrap(); + for (name, age) in [("Eve", 40), ("Frank", 41)] { + let params = mixed_params(&[("$name", name)], &[("$age", age)]); + db.mutate("main", MUTATION_QUERIES, "insert_person", ¶ms) + .await + .unwrap(); + } + drop(db); + let db = Omnigraph::open(dir.path().to_str().unwrap()).await.unwrap(); + install_policy_source(db, dir.path(), MAINTENANCE_POLICY_YAML).0 +} + +fn assert_no_actor(result: Result, what: &str) { + match result { + Err(OmniError::Policy(message)) if message.contains("no actor") => {} + other => panic!("{what}: expected the no-actor refusal, got {other:?}"), + } +} + +/// Optimize, index builds and the repair report require `maintain` on the +/// branch they maintain, before any effect: `change` does not imply it, a +/// grant on unprotected branches does not reach main, and a missing actor is +/// refused. An allowed optimize records its actor on the commit it publishes. +#[tokio::test] +async fn maintenance_entry_points_enforce_maintain_on_their_branch_before_effects() { + let dir = tempfile::tempdir().unwrap(); + let db = init_with_maintenance_policy(&dir).await; + let main_before = db.list_commits(None).await.unwrap(); + let files_before = physical_tree(dir.path()); + + for actor in [ + "act-denied", + "act-writer", + "act-reclaimer", + "act-branch-maintainer", + ] { + let actor = Some(actor); + assert_denied(db.optimize_as(actor).await, "optimize_as"); + assert_denied(db.ensure_indices_as(actor).await, "ensure_indices_as"); + assert_denied( + db.ensure_indices_on_as("main", actor).await, + "ensure_indices_on_as(main)", + ); + assert_denied(db.repair_report_as(actor).await, "repair_report_as"); + } + for actor in ["act-denied", "act-writer", "act-reclaimer"] { + assert_denied( + db.ensure_indices_on_as("feature", Some(actor)).await, + "ensure_indices_on_as(feature)", + ); + } + assert_no_actor(db.optimize_as(None).await, "optimize_as"); + assert_no_actor(db.ensure_indices_as(None).await, "ensure_indices_as"); + assert_no_actor( + db.ensure_indices_on_as("feature", None).await, + "ensure_indices_on_as", + ); + assert_no_actor(db.repair_report_as(None).await, "repair_report_as"); + assert_eq!( + physical_tree(dir.path()), + files_before, + "a refused maintenance call must not write index artifacts, compactions or publications" + ); + assert_eq!(db.list_commits(None).await.unwrap(), main_before); + + db.ensure_indices_on_as("feature", Some("act-branch-maintainer")) + .await + .expect("maintain on an unprotected branch admits its index builds"); + let report = db + .repair_report_as(Some("act-maintainer")) + .await + .expect("maintain on main admits the repair report"); + assert!(!report.datasets.is_empty()); + db.ensure_indices_as(Some("act-maintainer")) + .await + .expect("maintain on main admits index builds"); + + let head_before_optimize = db.list_commits(None).await.unwrap()[0] + .graph_commit_id + .clone(); + let stats = db + .optimize_as(Some("act-maintainer")) + .await + .expect("maintain on main admits optimize"); + assert!( + stats + .iter() + .any(|stat| stat.type_key == "node:Person" && stat.committed), + "{stats:?}" + ); + let main_after = db.list_commits(None).await.unwrap(); + assert_eq!( + main_after[0].parent_commit_id.as_deref(), + Some(head_before_optimize.as_str()) + ); + assert_eq!(main_after[0].actor_id.as_deref(), Some("act-maintainer")); +} + +/// Cleanup requires `reclaim`, which no other grant implies, before any +/// effect. +#[tokio::test] +async fn cleanup_as_enforces_reclaim_before_effects() { + let dir = tempfile::tempdir().unwrap(); + let db = init_with_maintenance_policy(&dir).await; + let keep_one = || omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }; + let files_before = physical_tree(dir.path()); + for actor in ["act-denied", "act-writer", "act-maintainer"] { + assert_denied(db.cleanup_as(keep_one(), Some(actor)).await, "cleanup_as"); + } + assert_no_actor(db.cleanup_as(keep_one(), None).await, "cleanup_as"); + assert_eq!( + physical_tree(dir.path()), + files_before, + "a refused cleanup must not delete anything" + ); + + let rows = db + .cleanup_as(keep_one(), Some("act-reclaimer")) + .await + .expect("reclaim admits cleanup"); + assert!(rows.iter().all(|row| row.error.is_none()), "{rows:?}"); + assert_ne!( + physical_tree(dir.path()), + files_before, + "the admitted cleanup reclaims the history keep 1 does not retain" + ); +} + #[tokio::test] async fn schema_settlement_authorizes_current_executor_and_preserves_original_authorship() { use omnigraph::db::{SchemaApplySettlement, SchemaNonPublicationProof}; diff --git a/crates/omnigraph/tests/schema_apply.rs b/crates/omnigraph/tests/schema_apply.rs index 9ac4ee162..692b44ddd 100644 --- a/crates/omnigraph/tests/schema_apply.rs +++ b/crates/omnigraph/tests/schema_apply.rs @@ -308,7 +308,7 @@ query insert_project($name: String) { ); let detached_before_indices = helpers::collector::detached_versions(&project_uri).await; stale_handle - .ensure_indices_on("source") + .ensure_indices_on_as("source", None) .await .expect("index planning must use the same token-bound post-apply catalog"); let project_after_indices = project_published( @@ -372,7 +372,7 @@ async fn stale_handle_branch_delete_gates_tables_added_by_schema_apply() { let index_handle = Arc::clone(&index_reconciler); let mut index_task = - tokio::spawn(async move { index_handle.ensure_indices_on("target").await }); + tokio::spawn(async move { index_handle.ensure_indices_on_as("target", None).await }); let index_blocked = tokio::time::timeout(std::time::Duration::from_millis(250), &mut index_task) .await @@ -1821,7 +1821,7 @@ node Tag { .collect::>() .join("\n"); db.load_jsonl(&rows, LoadMode::Overwrite).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let before = db.snapshot_of(ReadTarget::branch("main")).await.unwrap(); let before_snapshot = db.resolve_snapshot("main").await.unwrap(); @@ -1937,7 +1937,7 @@ node Tag { } // Reconciliation builds from the desired declarations, so nothing indexes // a dropped column again. - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); for (table, dropped_id) in dropped_ids { assert!( coverage(&db, table) @@ -2124,7 +2124,7 @@ query strong($min: I32) { "an update carries the untouched Blob of an evolved row" ); - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); assert_eq!( read_managed_blob_bytes( &db, @@ -2479,7 +2479,7 @@ async fn apply_schema_pure_type_rename_preserves_identity_path_and_version() { ); let db = helpers::session(Omnigraph::init(uri, &initial).await.unwrap()); db.load_jsonl(&data, LoadMode::Overwrite).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let before_snapshot_id = db.resolve_snapshot("main").await.unwrap(); let before_snapshot = db.snapshot_of(ReadTarget::branch("main")).await.unwrap(); let before_version = before_snapshot.graph_manifest_version(); @@ -2675,10 +2675,13 @@ edge WorksAt: Human -> Company /// prunes the `__manifest` version naming it. async fn reclaim_dropped_history(db: &Session) { let stats = db - .cleanup(omnigraph::db::CleanupPolicyOptions { - keep_versions: Some(1), - older_than: None, - }) + .cleanup_as( + omnigraph::db::CleanupPolicyOptions { + keep_versions: Some(1), + older_than: None, + }, + None, + ) .await .unwrap(); for row in &stats { @@ -3110,7 +3113,7 @@ async fn apply_schema_defers_vector_index_on_empty_table() { db.load_jsonl(r#"{"type":"Doc","data":{"slug":"d1","body":"hello","embedding":[0.1,0.2,0.3,0.4,0.5,0.6,0.7,0.8]}}"#, LoadMode::Merge, ) .await .expect("loading a Doc with an embedding must succeed"); - db.ensure_indices() + db.ensure_indices_as(None) .await .expect("the deferred vector index must build once the table has a trainable vector"); } @@ -3261,7 +3264,7 @@ async fn full_text_analyzer_survives_schema_evolution_issue_904() { assert_eq!(search("summary").await, ["d3"]); // The reconciler builds the postings over the declarations. - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); assert_eq!(segments("text").await, [Some(2)]); assert_eq!(search("text").await, ["d1", "d2"]); assert_eq!(search("summary").await, ["d3"]); diff --git a/crates/omnigraph/tests/search.rs b/crates/omnigraph/tests/search.rs index fd79b55e4..2715c2b0c 100644 --- a/crates/omnigraph/tests/search.rs +++ b/crates/omnigraph/tests/search.rs @@ -207,7 +207,7 @@ async fn init_search_db(dir: &tempfile::TempDir) -> Session { db.load_jsonl(SEARCH_DATA, LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } @@ -226,7 +226,7 @@ async fn init_mock_embedding_search_db(dir: &tempfile::TempDir) -> Session { db.load_jsonl(&mock_embedding_seed_data(), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } @@ -236,7 +236,7 @@ async fn init_model_recorded_search_db(dir: &tempfile::TempDir) -> Session { db.load_jsonl(&mock_embedding_seed_data(), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } @@ -561,7 +561,7 @@ query delete_middle() { "#; let db = session(Omnigraph::init(uri, schema).await.unwrap()); db.load_jsonl(&seed, LoadMode::Overwrite).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); helpers::forge_linear_head_from_pin(&db, "main", "node:Doc", 0).await; { use lance::index::DatasetIndexExt; @@ -1225,7 +1225,7 @@ query nearest_all($q: Vector(4)) { let uri = dir.path().to_str().unwrap(); let db = session(Omnigraph::init(uri, schema).await.unwrap()); db.load_jsonl(&rows, LoadMode::Overwrite).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let db = with_setting(&db, "ann_nprobes", "100000"); use omnigraph::instrumentation::{QueryIoProbes, with_query_io_probes}; @@ -1306,7 +1306,7 @@ query nearest_all($q: Vector(4)) { let uri = dir.path().to_str().unwrap(); let db = session(Omnigraph::init(uri, schema).await.unwrap()); db.load_jsonl(&rows, LoadMode::Overwrite).await.unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let probes = QueryIoProbes::default(); let result = with_query_io_probes(probes.clone(), async { @@ -1413,7 +1413,7 @@ async fn issue_567_line_docs(uri: &str, rows: usize, edge_every: Option) db.load_jsonl(&lines.join("\n"), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } @@ -2069,7 +2069,7 @@ async fn injected_embedding_config_is_used_instead_of_env() { db.load_jsonl(&mock_embedding_seed_data(), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let result = query_main( &db, @@ -2160,7 +2160,7 @@ async fn bm25_ordered_aggregate_counts_all_matches_not_the_capped_scan() { db.load_jsonl(&underfill_seed_data(), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let db = db; use omnigraph::instrumentation::{QueryIoProbes, with_query_io_probes}; @@ -2209,7 +2209,7 @@ async fn rrf_arms_scan_uncapped_in_one_pass() { db.load_jsonl(&underfill_seed_data(), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); use omnigraph::instrumentation::{QueryIoProbes, with_query_io_probes}; // This test pins the POSTFILTER plan's invariants (uncapped one-pass @@ -2281,7 +2281,7 @@ query uncapped_all($q: String) { db.load_jsonl(&underfill_seed_data(), LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); // Appended AFTER the index build: their fragment is uncovered, and their // term frequency (40 > the seed's max 20) puts them at the top of any // correct ranking. @@ -2426,7 +2426,7 @@ async fn mutation_with_deferred_index_coverage_remains_searchable() { // Ordinary optimize must preserve certified postings, not silently replace // them with an uncertified incremental fold. Both old and tail rows remain // searchable after data compaction and unrelated index maintenance. - db.optimize().await.unwrap(); + db.optimize_as(None).await.unwrap(); for (term, slug) in [("Quasar", "quasar-notes"), ("Learning", "ml-intro")] { let result = query_main(&db, SEARCH_QUERIES, "text_search", ¶ms(&[("$q", term)])) .await @@ -2587,7 +2587,10 @@ async fn uncertified_full_text_refuses_all_search_routes_but_not_ordinary_reads( Err(OmniError::FullTextIndexRebuildRequired { .. }) )); - let rebuilt = db.rebuild_full_text_indices_on("main").await.unwrap(); + let rebuilt = db + .rebuild_full_text_indices_on_as("main", None) + .await + .unwrap(); assert!(!rebuilt.rebuilt_indexes.is_empty()); assert!( query_main( @@ -2690,7 +2693,7 @@ node Doc { 0, "load publishes exact data effects and leaves physical indexes pending" ); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); let ds = snapshot_main(&db) .await @@ -2788,7 +2791,7 @@ async fn full_text_analyzer_holds_after_first_rows_and_overwrites_issue_904() { .unwrap(); assert_eq!(hits("main", "docs").await, ["d1", "d2"], "first rows"); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db.load("main", &rows("Doc", "d"), LoadMode::Overwrite) .await .unwrap(); @@ -2880,7 +2883,7 @@ async fn a_full_text_index_with_no_segment_refuses_the_call_issue_747() { .unwrap_err(), ); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); assert_eq!( first_column_sorted( &query_branch(&db, "main", ANALYZER_QUERIES, "docs", &deep) diff --git a/crates/omnigraph/tests/system_column_upgrade.rs b/crates/omnigraph/tests/system_column_upgrade.rs index cfaf0e354..ca5a2dc84 100644 --- a/crates/omnigraph/tests/system_column_upgrade.rs +++ b/crates/omnigraph/tests/system_column_upgrade.rs @@ -71,7 +71,7 @@ async fn legacy_graph_with_data(dir: &tempfile::TempDir) -> Session { db.load_jsonl(LEGACY_DATA, LoadMode::Overwrite) .await .unwrap(); - db.ensure_indices().await.unwrap(); + db.ensure_indices_as(None).await.unwrap(); db } @@ -226,7 +226,7 @@ async fn assert_upgraded(db: &Session, dir: &tempfile::TempDir, expected_export: .await .expect_err("the respelled @unique(@src, @dst) still refuses a duplicate endpoint pair"); assert_eq!(count_rows(db, "edge:WorksAt").await, 3); - db.ensure_indices() + db.ensure_indices_as(None) .await .expect("index reconciliation after the rename is a no-op by field id"); } @@ -407,7 +407,7 @@ async fn system_column_upgrade_history_survives_unrelated_reclaimed_tables() { main.retained.contains(&company_moved) && main.would_prune.contains(&version_before), "the run keeps the Company write's `__manifest` version and drops the pre-write one: {main:?}" ); - let stats = db.cleanup(keep).await.unwrap(); + let stats = db.cleanup_as(keep, None).await.unwrap(); let row = stats.iter().find(|row| row.type_key == first).unwrap(); assert!(row.error.is_none(), "{row:?}"); assert!( diff --git a/docs/dev/control-plane.md b/docs/dev/control-plane.md index 72a1959cb..176ca9a0c 100644 --- a/docs/dev/control-plane.md +++ b/docs/dev/control-plane.md @@ -514,8 +514,12 @@ including a 500. Mutation, load, Blob write and merge mark opaque errors (storag steps, alongside the table store's staged-write and detached-commit sites, and the publisher marks every failure whose manifest request cannot land. A contained write logs the failure's redacted class before admission closes. -A typed refusal keeps its shape. Other opaque schema and maintenance -preparation errors without completion evidence still fail closed as uncertain. Proven publication followed by +A typed refusal keeps its shape. Optimize, cleanup, the index writers and +the repair report carry evidence for every failure before their publication +([write path](writes.md#policy-and-attribution)), so a failed maintenance run +does not contain the server; other opaque schema preparation errors without +completion evidence still fail closed as uncertain. A maintenance run stopped by +`cancel_maintenance` answers `service_unavailable` with its evidence. Proven publication followed by `RecoveryRequired`, and uncertain publication itself, are never downgraded. Read/write body and response lanes have independent capacity; bodyless reads consume only read observers. Once HTTP connections and the remaining known @@ -608,6 +612,7 @@ server. Public actor construction cannot grant signed-token permissions. on `AuthenticatedActor` exposes only verified claims. Policy embedders must handle `PolicyAction::ConfigManage`, +`PolicyAction::Maintain`, `PolicyAction::Reclaim`, `PolicyResourceKind::Cluster`, and `PolicyEngineKind::Cluster`. These public enums are exhaustive, so an external exhaustive match must add the relevant arm when upgrading. Preserving existing constructors and direct storage-holder diff --git a/docs/dev/lance.md b/docs/dev/lance.md index b25c6e4f6..df3bfa202 100644 --- a/docs/dev/lance.md +++ b/docs/dev/lance.md @@ -181,6 +181,7 @@ history of dependency bumps. | Schema evolution | Schema apply changes an existing table's columns without writing a data file (`TableStore::stage_schema_evolution`). Renames and drops commit as `Operation::Project` over the same field ids (Lance's own `alter_columns` rename and `drop_columns`); additions commit as `Operation::Merge` over the unchanged fragments with nullable fields whose ids exceed `Manifest::max_field_id` (Lance's own all-null `add_columns`). Lance 13 validates detached commits too, and its `merge_schema_valid` refuses a Merge that moves a field id to another path, so a table with renames or drops and additions chains the Merge on the Project. Lance 13 reads a field that no data file of a fragment holds as null (`NullReader`) and skips a data file holding no projected field; Project drops a data file whose every field is dropped; `retain_relevant_indices` keeps each index whose fields survive, with its fragment bitmap, and removes an index on a dropped field. Merge stamps no row version for fragments whose files are unchanged. Keyed writes send batches in catalog column order, and Lance 13's merge-insert matches a source to the full table schema only in the same field order (otherwise it takes its subschema path), so the evolved schema keeps the catalog order. A dropped column's values stay in the data files until optimize rewrites the fragment (dropped-column erasure, next row). | `lance_surface_guards.rs::metadata_only_schema_evolution_keeps_blob_files_ids_and_index_coverage`, `schema_apply.rs`, the deferred `benchmarks/deferred/crates/omnigraph/tests/compaction_memory.rs.disabled` schema-apply instrument (not an active test) | | Dropped-column erasure | A drop's `Operation::Project` keeps every data file that also holds a surviving column, and that file's `DataFile::fields` still lists the dropped field id. A Blob-v2 column is listed by its own id, never by its descriptor children's, so on a table without drops every listed id is a schema field id. Lance 13's planner also selects a fragment when a file contains only dropped user columns but remains necessary for row lineage. It does not select a lone fragment solely because a file mixes kept and dropped columns, so those bytes still need the engine's extra task. `TableStore::plan_table_compaction`, the one plan behind optimize's planning gate and `stage_compaction`, appends a single-fragment `TaskData` (public, as is `CompactionPlan::tasks`) for every other fragment whose data or overlay files list a non-negative id outside the schema. Lance's planner applies its per-run source budgets (`max_source_fragments`, `max_source_rows`, `max_source_bytes`, in `limit_tasks_to_source_budget`) to the tasks it returns, so the appended tasks would fall outside them; the engine sets no budget, and `plan_table_compaction` refuses options that set one. A single-fragment group is the shape Lance plans for deletion materialization, and `Rewrite` replaces it in place. Compaction re-encodes by default (`CompactionMode::Reencode`; binary copy would carry pages) and scans the current schema, so the new files list only current field ids and only the kept Blob values get new sidecars; the task is sized like any other, from the kept Blob columns alone. `cleanup` then sweeps the old files with their sidecars once no retained version names them. | `lance_surface_guards.rs::compacting_a_chosen_fragment_drops_the_bytes_of_dropped_columns`, `integration/maintenance.rs::optimize_then_cleanup_erases_dropped_property_values`, `staged_tests.rs::table_compaction_planning_refuses_source_budgets`, the deferred `benchmarks/deferred/crates/omnigraph/tests/compaction_memory.rs.disabled` erasure instrument (not an active test) | | Compaction of Blob tables | Lance 13 compaction cannot binary-copy a Blob table; it rewrites through a scanner and materializes every managed Blob-v2 payload of one scanner batch (external descriptors are carried unread). A batch never crosses a fragment, so at the default batch size one batch can hold a whole fragment's payloads. `TableStore::stage_compaction` executes the plan's tasks itself and sets each task's `CompactionTask::options.batch_size` from a descriptor-only scan of that task's fragments: the largest row's managed Blob bytes, summed over its Blob columns, with external descriptors counting nothing (`compaction_blob_batch_rows`: a 32 MiB budget, 1 to 8192 rows, so a row over 16 MiB puts its whole task on one-row batches; a nested or non-Blob-v2 Blob field fails closed to one row). Every task of a Blob table gets the derived size explicitly, capped by a caller's smaller one, so it takes precedence over `LANCE_DEFAULT_BATCH_SIZE`, including when the variable is smaller; all tasks are sized before any executes, so when a fragment being compacted holds a descriptor the decoder refuses, sizing can refuse the compaction with a Blob integrity error before that table is rewritten, and nothing is committed; a table with nothing to compact is not scanned. Lance reads a task's options into its scanner and ignores the batch size when planning. A set batch size beats `LANCE_DEFAULT_BATCH_SIZE` on the V2.x read path, where the variable is only the fallback. The fence is the scanner shape: a scanner built as compaction builds it honors an explicit batch size over the variable. No CI-run test observes Lance passing `CompactionOptions::batch_size` into that scanner; the historical `compaction_memory.rs` instrument was the only observer and is now deferred under `benchmarks/deferred/` pending GQ optimization support. This bounds managed payload bytes per scanner batch, not total peak memory: decoder, I/O and copy buffers are additional, and one oversized value may exceed the target. Drop the derivation at the bump whose compaction streams Blob payloads. | `lance_surface_guards.rs::explicit_scanner_batch_size_beats_lance_default_batch_size_env` (Guard 10b: the compaction scanner shape honors an explicit batch size over the variable), `integration/maintenance.rs::optimize_compacts_large_blob_rows_in_bounded_batches`, `integration/maintenance.rs::optimize_sizes_a_blob_batch_by_the_sum_of_a_rows_blob_columns`, `integration/maintenance.rs::optimize_sizes_each_compaction_task_from_its_own_fragments`, `integration/maintenance.rs::optimize_does_not_size_a_blob_batch_by_external_references`, `integration/maintenance.rs::optimize_compacts_blob_fragments_with_deleted_rows`, `staged_tests.rs::compaction_blob_batch_rows_bounds_one_batch`, the deferred `benchmarks/deferred/crates/omnigraph/tests/compaction_memory.rs.disabled` instrument (not an active test) | +| Maintenance CPU work | Lance 13 decodes the batches a compaction scan reads in tasks it spawns with `tokio::spawn` (`lance-encoding` `decoder.rs`), tokenizes a full-text build's documents in `JoinSet` tasks (`lance-index` `scalar/inverted/builder.rs`), and sorts a BTREE's training data in the DataFusion stream it polls; only page encoding, posting-list and BTREE page building go to its CPU pool (`spawn_cpu`). The spawned work lands on whichever runtime polls the call, and neither `spawn_blocking` nor the CPU pool, which takes synchronous closures, can hold it. Maintenance therefore polls each compaction task (`CompactionTask::execute`) and each index build (`execute_uncommitted`) on a two-worker runtime of its own (`db::maintenance::off_serving_runtime`), except on a deterministic-simulation harness thread. Optimize also sets every compaction task's `io_buffer_size` (64 MiB by default; Lance's default is 32 MiB per I/O thread of the store) and, where no Blob sizing derives one, a batch size of 8192 rows, and each full-text build's `InvertedIndexParams::memory_limit_mb` and `num_workers` (512 MiB over two workers), which Lance splits across the workers and does not persist. Drop the runtime at the bump whose decode and tokenization run on the CPU pool. | in-source `db::maintenance::tests` (`work_off_the_serving_runtime_returns_its_output_and_aborts_on_drop`, `compaction_options_carry_the_budget`) | | Index coverage | Indexes are derived. Rewrites and compaction may leave an uncovered tail; reads must combine indexed and scan paths until explicit reconciliation. | `integration/scalar_indexes.rs`, `search.rs`, `integration/maintenance.rs` | | Index segments | Lance masks a logical index's results by the union of its segments' fragment coverage. On a stable-row-ID table an update keeps the row ID and moves the row to a new fragment; once a delta segment covers that fragment, the older segment's entry for the row passes the mask, so full-text search returns the row for a term it no longer holds (twice for a term both versions hold) and a BTREE equality returns it for its old value. Lance applies per-segment ownership to vector search only (lance#7371, lance#8351); the full-text and scalar paths are reported as lance#9817. OmniGraph never adds a delta segment: `stage_index_fold` rebuilds a lagging index whole and the full-text rebuild replaces every segment. An untrained full-text segment (an empty fragment bitmap) carries its analyzer to every row it does not cover, with rows and scores equal to a full build; with no segment, Lance's flat path tokenizes with a bare `SimpleTokenizer`. Drop the whole-rebuild rule at the bump whose scalar and full-text search apply segment ownership. | `lance_surface_guards.rs::index_delta_segment_serves_stale_entries_of_an_updated_row` (goes red at that bump), `lance_surface_guards.rs::fts_untrained_segment_applies_its_analyzer_to_rows_it_does_not_cover` | | Inherited index files | Lance 13 preserves inherited index bases during nested clones and resolves external fragment-reuse details at their original base. The adapter checks the exact source version and delegates clone metadata to Lance. Production branch creation clones only `__manifest`, which has no fragment-reuse index; graph tables are not cloned. Malformed external fragment-reuse payloads are outside this adapter's supported graph contract, and native branch creation need not read or reject them before publication. | `lance_clone` tests; `lance_surface_guards.rs::stock_lance_nested_clone_preserves_inherited_index_base_issue_7840` | diff --git a/docs/dev/testing.md b/docs/dev/testing.md index a4038d9c5..32d1ee79f 100644 --- a/docs/dev/testing.md +++ b/docs/dev/testing.md @@ -69,7 +69,7 @@ The engine integration suite is grouped by behavior, not implementation module: | Writes, validation, schema, and policy | `writes.rs`, `integration/validators.rs`, `schema_apply.rs`, `policy_engine_chassis.rs` | | Branches, snapshots, diffs, and merges | `integration/branching.rs`, `integration/point_in_time.rs`, `integration/changes.rs`, `integration/merge_truth_table.rs`, `integration/merge_fast_forward.rs` | | Recovery and crash windows | `integration/recovery.rs`, `failpoints.rs` (including the `live_handle_*` liveness owners: a live handle writes again once faults stop, without reopening), `detached_commit_matrix.rs` (the RFC 0067 writer × window × fault × recovery-actor matrix over the insert, multi-table, load, cleanup, ensure-indices, full-text-rebuild, merge, schema-apply, optimize, system-column-upgrade and Blob put and clear writers; the same-handle liveness actor runs by default and `OMNIGRAPH_MATRIX=full` adds the other-process and cleanup actors), in-source manifest/recovery tests | -| Maintenance and substrate fences | `integration/maintenance.rs`, `lance_surface_guards.rs`, `integration/lance_version_columns.rs`, `forbidden_apis.rs` | +| Maintenance and substrate fences | `integration/maintenance.rs` (with the real storage faults that prove a failure before publication is `Unpublished`), `lance_surface_guards.rs`, `integration/lance_version_columns.rs`, `forbidden_apis.rs`; in-source `db::maintenance` tests own the budget, the lane, the evidence a run gives its failures and the maintenance runtime; `failpoints.rs` owns a cancelled run and the lane's turns across handles | | Export and lineage | `integration/export.rs`, `integration/lineage_projection.rs` | | Legacy-vintage graphs (`id`/`src`/`dst` spellings, born at the current stamp) | `integration/legacy_columns.rs` — load, query, export round trip, evolution; needs `--features failpoints` | | System-column upgrade (RFC 0040 step 3: respelling in place on a supported standalone graph; vintage is independent of the storage stamp) | `system_column_upgrade.rs`: check and execute, preflight refusals, every window before the manifest commit leaving no residue, a complete contract and table state after a post-commit failure, same-handle retry, the control-object cost; needs `--features failpoints` | diff --git a/docs/dev/writes.md b/docs/dev/writes.md index 120fff36a..fabf7c416 100644 --- a/docs/dev/writes.md +++ b/docs/dev/writes.md @@ -564,9 +564,10 @@ carried stored external reference the policy refuses fails as | A proposal tag's creation or deletion reports an ambiguous result | The tag is read back: a matching tag is success, a differing tag an integrity error, an unavailable readback `manifest_publish_in_doubt`. After a proposal or unattached snapshot tag is created, a failed authority probe remains in doubt even if rollback deletes the tag: an earlier conditional create may recreate it. Only proven expiry of the captured authority makes that refusal definite | The `Unpublished` annotations cover the named writer envelopes, the table -store's staged-write and detached-commit sites, and every publisher failure -whose manifest request cannot land. Other opaque schema and maintenance -preparation errors without completion evidence remain uncertain. A +store's staged-write and detached-commit sites, every publisher failure +whose manifest request cannot land, and the maintenance runs (below). Other +opaque schema preparation errors without completion evidence remain +uncertain. A post-publication `RecoveryRequired` or an uncertain publication is never changed into definite non-publication by these annotations. @@ -582,6 +583,38 @@ callers must pass one when a policy checker is installed. Actor attribution travels with the pre-minted graph lineage and is published with the same manifest CAS. +Maintenance has no actor-less public form. `optimize_as`, `ensure_indices_as`, +`ensure_indices_on_as` and `rebuild_full_text_indices_on_as` check `maintain` +on the branch they maintain, `main` for optimize; the full-text rebuild also +admits `change`, its grant before `maintain` existed (`Omnigraph::enforce_any`, +`PolicyChecker::check_any`). `cleanup_as` checks `reclaim` with no branch, and +the read-only `repair_report_as` checks `maintain` on `main`. +`forbidden_apis.rs::maintenance_entry_points_take_an_actor` keeps every +registered maintenance surface an `_as` form with an `actor` parameter. + +Optimize, cleanup, the index writer and the full-text rebuild each run in +their graph's maintenance lane (`db/maintenance.rs`), entered after the +policy check: one maintenance run per root at a time in a process, the others +waiting in memory in arrival order. The lane is process-local derived state +held by the root's write-queue manager, never writer fencing; the +publication's exact CAS still decides between processes. A run records how +far it has gone, and every failure it returns carries the matching +completion evidence: an opaque failure before its first durable effect is +`BeforeEffect`, after detached commits, staged index files or cleanup +deletions it is `Unpublished`, a publisher failure keeps the publisher's own +evidence (a publication in doubt stays uncertain), and a failure after the +publication is `PartlyPublished`; a typed refusal before the publication +keeps its shape. The read-only repair report marks every opaque failure +`BeforeEffect`. `Omnigraph::cancel_maintenance` cancels the run holding the +lane and every run waiting for it. A run stops at its next cancel point with +`OmniError::MaintenanceCancelled`: before every publication, between tables +and between the links of a table's chain, and, for work whose effects stay +unreachable (planning, a table's detached chain, an index build), at once, +by dropping that work. A publication in progress and a cleanup deletion in +progress always finish. A cancelled run leaves only staging the collector +reclaims by its recorded authority, and segment files a dropped index build +wrote, which no manifest names, under the collector's orphan age rule. + Bearer authentication (`401 unauthorized`), the HTTP contract check (`400 api_contract_mismatch`) and policy (`403 forbidden`) refuse a served write only before its first durable effect. The server runs diff --git a/docs/user/operations/maintenance.md b/docs/user/operations/maintenance.md index 7bf20669a..51c625a1c 100644 --- a/docs/user/operations/maintenance.md +++ b/docs/user/operations/maintenance.md @@ -21,6 +21,14 @@ accepts local-file direct targets only; cloud-direct maintenance is not exposed by this CLI. Engine/server cloud support does not change that client boundary. See [operator configuration](../cli/targets.md#transport-and-capability-boundaries). +Each command acts as the operator configuration's `operator.actor`; +`rebuild-full-text-indexes` also accepts `--as` in its place. A commit the +command publishes records that actor. Direct CLI access, including `--target`, +does not load the server's Cedar policy; storage permissions are its trust +boundary. An embedded host that installs a policy checker enforces the +[policy actions](policy.md#actions) before any effect: `maintain` for optimize, +index builds, the full-text rebuild and repair, and `reclaim` for cleanup. + ## Optimize ```bash @@ -74,9 +82,8 @@ including when the variable is smaller. When a fragment being compacted holds a Blob descriptor the engine's decoder rejects, sizing can refuse the compaction with a Blob integrity error before that table is rewritten, and nothing is committed; a table with nothing to compact is not scanned. -Optimize works on up to -`OMNIGRAPH_MAINTENANCE_CONCURRENCY` tables at once (default 8), so budget for -that many Blob tables compacting together. +Optimize rewrites one table at a time by default, and two at most (see +[budgets](#budgets)), so budget for that many Blob tables compacting together. Optimize also persists the traversal-adjacency artifact (`__graph_index/csr-current.bin`), which cold traversal builds load instead of @@ -118,10 +125,9 @@ Success means the selected branch's planned rebuild was published, or an explici no-op with an empty index list, null commit, and empty `warnings` array. The default branch is `main`. -`--as` supplies actor attribution for this command. Direct CLI access, including -`--target`, does not load the server's Cedar policy; storage permissions are its -trust boundary. An embedded host that installs a policy checker also enforces -the Change permission before rebuilding. +`--as` supplies actor attribution for this command. An embedded host that +installs a policy checker admits the rebuild under `maintain` or `change` on the +selected branch; a later release admits it under `maintain` only. Other branches and historical snapshots are not rewritten. Rebuild every live branch that needs full-text search; restoring an older snapshot may require @@ -305,6 +311,31 @@ Exit 0 means every table was visited and that nothing a retained graph commit pins, and no file such a commit references, was removed. It does not mean every table was collected: read the result rows for tables with an `error`. +## Budgets + +Optimize, cleanup, index builds and the full-text rebuild share one budget, +read from the environment at the start of each run by the direct CLI, a +server or an embedding process. A value that is not a positive integer keeps +its default, and the run logs the variable it ignored. + +| Variable | Default | Bounds | +|---|---|---| +| `OMNIGRAPH_MAINTENANCE_CONCURRENCY` | `1` | The tables whose compaction or index work runs at once. At most `2`; a larger value runs `2` and is logged. Planning and cleanup's sweep read up to eight tables at once regardless. | +| `OMNIGRAPH_MAINTENANCE_SCAN_IO_BUFFER_BYTES` | `67108864` (64 MiB) | The bytes a compaction scan may queue in Lance's I/O buffer. | +| `OMNIGRAPH_MAINTENANCE_FULL_TEXT_MEMORY_MB` | `512` | The memory one full-text index build splits across its workers. | +| `OMNIGRAPH_MAINTENANCE_FULL_TEXT_WORKERS` | `2` | The workers a full-text index build tokenizes with. | + +Compaction scans in batches of 8,192 rows, Lance's own fallback, set +explicitly so `LANCE_DEFAULT_BATCH_SIZE` does not size it; Blob tables use the +smaller batches above. The caps bound Lance's buffers, not the process, and +Lance's process-wide `LANCE_*` thread and memory variables are left alone. + +One graph runs one maintenance operation at a time in a process: an optimize, +cleanup or index build that starts while another runs on the same graph waits +for it. Lance's CPU-heavy maintenance work (decoding and re-encoding a +compaction's rows, building an index) runs on two threads of its own, so a +process that also serves requests keeps its request threads. + ## Suggested cadence - Run `optimize` after large loads or on a regular cadence for write-heavy diff --git a/docs/user/operations/policy.md b/docs/user/operations/policy.md index 2187d6e8d..b3d122450 100644 --- a/docs/user/operations/policy.md +++ b/docs/user/operations/policy.md @@ -21,6 +21,8 @@ Graph-scoped actions: | `branch_merge` | Branch merge | | `commit_publish` | Publication of an unpublished commit onto its recorded target branch | | `commit_drop` | Drop of an unpublished commit | +| `maintain` | Content-preserving maintenance: optimize, index builds, the full-text rebuild and the repair report | +| `reclaim` | Cleanup, which permanently removes table versions and files no retained commit needs | | `invoke_query` | Entry to a stored query | | `admin` | Reserved graph administration | @@ -42,6 +44,17 @@ The reserved graph `admin` action does not grant cluster management. A stored mutation requires both `invoke_query` and `change`. A stored read requires `invoke_query` and `read`. +`maintain` and `reclaim` are separate from `change`, so an automation account +can compact a graph without the right to write data, and can hold `maintain` +without the right to delete history. `maintain` is judged on the branch the +operation maintains (`main` for optimize and the repair report) and takes +`branch_scope`; `reclaim` covers every branch's history and takes no branch +scope. The full-text rebuild accepts `change` or `maintain` on its branch; a +later release accepts `maintain` only, so grant `maintain` to the accounts that +run it. The engine checks these actions whenever a policy is installed on the +graph handle; the direct maintenance commands load no policy (see +[maintenance](maintenance.md)). + ## Bind a policy ```yaml @@ -124,7 +137,7 @@ empty, has leading or trailing whitespace, or contains a control character fails policy validation, and a request naming such a branch is refused as a bad request before any rule is evaluated (see [branch names](../branching/index.md#branch-names)). Server actions and graph-wide -`invoke_query` rules do not take branch scopes. `commit_publish` and +`invoke_query` and `reclaim` rules do not take branch scopes. `commit_publish` and `commit_drop` take `target_branch_scope`: the scope is the branch the unpublished commit recorded as its target. Writing an unpublished commit needs no new action: a mutation `as commit` is a `change` on its branch and a @@ -211,7 +224,8 @@ authentication allows every graph action and reports `actor_id: null`. The MCP server target ([what you may do at a target](../cli/targets.md#what-you-may-do-at-a-target)). It reports policy actions, not actor views, so a view can still refuse a -`change` the policy allows. +`change` the policy allows. It does not report `maintain` or `reclaim`, since +no server route runs maintenance. ## Actor identity diff --git a/docs/user/operations/storage-failures.md b/docs/user/operations/storage-failures.md index 78476b48f..08a22f65d 100644 --- a/docs/user/operations/storage-failures.md +++ b/docs/user/operations/storage-failures.md @@ -115,3 +115,8 @@ paths; each records the failure's class. - `took over a silent owner's cluster lock` when a restarted server takes over its predecessor's lock, and an error when an owner's lease lapses or another server claims it. +- `cluster lease beat failed; it is sent again` for each heartbeat request that + fails, `cluster lease claim read failed; the beat is sent anyway` for a failed + check for a successor's claim, and `cluster lease beat got no answer before the + owner's deadline` before the lapse it causes. Each carries the beat's `seq` + and the failure's `class` and storage `kind`, and comes before the fence. diff --git a/skills/omnigraph/references/server-policy.md b/skills/omnigraph/references/server-policy.md index 5eccf49d8..9486333b6 100644 --- a/skills/omnigraph/references/server-policy.md +++ b/skills/omnigraph/references/server-policy.md @@ -120,6 +120,8 @@ Graph-scoped actions are: | `schema_apply` | Schema changes: `cluster apply --target` needs it on each existing graph whose schema changes | | `branch_create`, `branch_delete`, `branch_merge` | Corresponding branch operation | | `commit_publish`, `commit_drop` | Publish or drop an unpublished commit; drop an unattached commit | +| `maintain` | Optimize, index builds, the full-text rebuild (which also accepts `change` for now) and the repair report | +| `reclaim` | Cleanup, which permanently removes unretained history | | `invoke_query` | Entry to a stored query | | `admin` | Reserved; no current public operation | @@ -165,12 +167,12 @@ rules: actions: [invoke_query] ``` -`branch_scope` is valid only with `read`, `export`, and `change`; +`branch_scope` is valid only with `read`, `export`, `change`, and `maintain`; `target_branch_scope` only with `branch_create`, `branch_delete`, `branch_merge`, `schema_apply`, `commit_publish`, and `commit_drop`. Any other pairing, or both scopes on one rule, is refused. Values are `any`, `protected`, or `unprotected`, where protected branches are the bundle's top-level `protected_branches: [main, …]` -list. `invoke_query`, `graph_list` and `config_manage` take no branch scope. +list. `invoke_query`, `reclaim`, `graph_list` and `config_manage` take no branch scope. ```bash omnigraph policy validate --target . --graph knowledge