diff --git a/AGENTS.md b/AGENTS.md index 59971b5f..12e0aef8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -254,7 +254,7 @@ omnigraph policy explain --cluster ./company-brain --graph knowledge --actor act | Per-dataset versioning + time travel | ✅ | `snapshot_at_version`, `entity_at`, snapshot-pinned reads across many tables | | Per-dataset branches | ✅ | **Graph-level** refs are logically atomic through authoritative `__manifest` `BranchContents`; native create/delete crash gaps are classified and reclaimed under a single-writer-process boundary; live names are path-prefix-disjoint; data-table forks are lazy; system branches are filtered | | Atomic single-dataset commits | ✅ | **Multi-table publish via three layers**, NOT a single Lance primitive: (1) per-table Lance `commit_staged` for the data write, (2) `__manifest` row-level CAS via `ManifestBatchPublisher` for cross-table ordering, (3) the open-time recovery sweep for the residual gap between (1) and (2). All three layers ship; the five migrated writers (`MutationStaging::commit_all`, `schema_apply`, `branch_merge`, `ensure_indices`, `optimize_all_tables`) write a `__recovery/{ulid}.json` sidecar before Phase B and delete it after Phase C. Under their final schema → branch → table gates, mutation/load, SchemaApply, BranchMerge, and EnsureIndices prove every existing effect target still equals its manifest pin before arming; first-touch refs are created only after the sidecar is durable. The next `Omnigraph::open` (gated on `OpenMode::ReadWrite`) runs the sweep in `db/manifest/recovery.rs`: classify, decide all-or-nothing per sidecar, roll forward via single `ManifestBatchPublisher::publish` or roll back via `Dataset::restore` followed by a manifest publish of the restored version (so both directions converge to `manifest == HEAD` — no residual drift), and record an internal audit row in `_graph_commit_recoveries.lance`. Schema-v3 Mutation/Load and schema-v4 BranchMerge roll-forward preserve the interrupted writer's fixed commit lineage and actor. SchemaApply v5 records the target schema identity plus durable Phase-C confirmation so metadata-only recovery is unambiguous. EnsureIndices v6 retains loose effect classification but pre-mints rollback lineage and persists the rollback audit plan before restore, making compensation retry-exact. Other rollback and legacy recovery commits use `omnigraph:recovery`. BranchMerge v4 also owns first-touch refs, pre-mints each table's exact ordered data-transaction chain with zero transparent conflict retries, carries pointer-only updates in its complete confirmed manifest delta, and recognizes an interrupted compensation restore on restart. There is currently no public CLI query for the recovery-audit table, and ordinary commit history is not a complete recovery enumeration. The write entry points (`load_as`, `mutate_as`, `apply_schema_as`, `branch_merge_as`) and `refresh` additionally run an in-process roll-forward-only heal, serialized against same-process live writers through the shared root-scoped gate manager, so a long-lived server converges on its next write without restart; only rollback-eligible sidecars still defer to the next read-write open (a future background reconciler's goal). Engine writes route through a sealed `TableStorage` trait (`db.storage()`) exposing only `stage_*` + `commit_staged` + reads; the sole inline-commit residual (`create_vector_index`) is split onto a separate sealed `InlineCommitResidual` trait reached via `db.storage_inline_residual()` (MR-854), so the default surface cannot couple a write with a HEAD advance — §1 holds by construction. `delete` migrated to the staged path in MR-A (`stage_delete` via Lance 7.0 `DeleteBuilder::execute_uncommitted`, [#6658](https://github.com/lance-format/lance/issues/6658)); `create_vector_index` stays inline until upstream Lance ships a public two-phase API ([#6666](https://github.com/lance-format/lance/issues/6666)); `LoadMode::Overwrite` uses Lance `Overwrite` staged transactions. | -| Compaction (`compact_files`) + reindex (`optimize_indices`) | ✅ | `omnigraph optimize` orchestrates over all node/edge tables, bounded concurrency; per table runs `compact_files` **then Lance `optimize_indices`** (folds appended/rewritten fragments back into existing indexes — incremental merge, not retrain) and **publishes the resulting version to `__manifest`** (so the manifest tracks the Lance HEAD — required for reads to observe the work and for schema apply / strict writes to pass their HEAD-vs-manifest precondition), under the per-`(table, main)` write queue with `SidecarKind::Optimize` recovery coverage spanning both ops; **commits even with no compaction work if index coverage is stale**; **refuses on an unrecovered graph**; **skips uncovered HEAD > manifest drift** with `DriftNeedsRepair`; **compacts blob-bearing tables** (the pre-9 `LANCE_SUPPORTS_BLOB_COMPACTION` skip was removed once Lance 8.0.0+ shipped blob-v2 compaction — see [docs/dev/invariants.md](docs/dev/invariants.md) Known Gaps) | +| Compaction (`compact_files`) + reindex (`optimize_indices`) | ✅ | `omnigraph optimize` orchestrates over all node/edge tables, bounded concurrency; per table runs `compact_files` **then Lance `optimize_indices`** (folds appended/rewritten fragments back into existing indexes — incremental merge, not retrain) and **publishes the resulting version to `__manifest`** (so the manifest tracks the Lance HEAD — required for reads to observe the work and for schema apply / strict writes to pass their HEAD-vs-manifest precondition), under the per-`(table, main)` write queue with `SidecarKind::Optimize` recovery coverage spanning both ops. Because every published table pointer also advances shared `graph_head:main`, Optimize's final recovery barrier is branch-wide: after the graph-wide fast probe it acquires the process-local main gate, relists, and rejects every main-target sidecar plus graph-global SchemaApply before reading table HEADs or arming its own sidecars. It retains that gate across its internally parallel table phase, and through final physical `__manifest` compaction so a new main recovery intent cannot arm before raw manifest movement finishes. The coarse legacy-adapter cost is that same-process sidecar-enrolled main writers wait for the entire Optimize run, although Optimize's own table tasks remain parallel. It **commits even with no compaction work if index coverage is stale**; **refuses on an unrecovered graph**; **skips uncovered HEAD > manifest drift** with `DriftNeedsRepair`; and **compacts blob-bearing tables** (the pre-9 `LANCE_SUPPORTS_BLOB_COMPACTION` skip was removed once Lance 8.0.0+ shipped blob-v2 compaction — see [docs/dev/invariants.md](docs/dev/invariants.md) Known Gaps) | | Repair uncovered drift | — | `omnigraph repair` explicitly classifies uncovered table `HEAD > manifest` drift: verified maintenance drift (`ReserveFragments`/`Rewrite`) can be published with `--confirm`; suspicious or unverifiable drift requires `--force --confirm`. Sidecar-covered crash residuals still recover automatically on open. | | Cleanup (`cleanup_old_versions`) | ✅ | `omnigraph cleanup` derives requested `--keep` / `--older-than` cutoffs from each table's available versions; Lance refs plus OmniGraph's live-lazy-branch and recovery floors may retain additional versions. It fails closed on unopenable pins, recovery intent, or uncovered main-table HEAD drift | | BTREE / inverted (FTS) / vector indexes | ✅ | `@index`/`@key` declares intent; the physical index is derived state that never fails a logical op. Built per column through one chokepoint (`build_indices_on_dataset_for_catalog`, type-dispatched by `node_prop_index_kind`: enum + orderable scalar → BTREE, free-text String → FTS, Vector → vector); idempotent; lazy across branches. **Schema apply and mutation/load build no indexes inline**: the latter publish only their exact data effects, leaving physical intent pending. `ensure_indices`/`optimize` materializes declared-but-missing indexes, restores fragment coverage, and continues to report untrainable Vector columns as pending. | diff --git a/crates/omnigraph/src/db/omnigraph/optimize.rs b/crates/omnigraph/src/db/omnigraph/optimize.rs index 43d1f190..344d3745 100644 --- a/crates/omnigraph/src/db/omnigraph/optimize.rs +++ b/crates/omnigraph/src/db/omnigraph/optimize.rs @@ -195,6 +195,24 @@ pub async fn optimize_all_tables(db: &Omnigraph) -> Result Result<()> { + let sidecars = crate::db::manifest::list_sidecars(db.root_uri(), db.storage_adapter()).await?; + let blocking = sidecars.iter().find(|sidecar| { + sidecar.writer_kind == crate::db::manifest::SidecarKind::SchemaApply + || sidecar + .branch + .as_deref() + .filter(|branch| *branch != "main") + .is_none() + }); + if let Some(sidecar) = blocking { + return Err(OmniError::recovery_required( + sidecar.operation_id.clone(), + format!( + "pending {:?} recovery operation on branch '{}' blocks optimize", + sidecar.writer_kind, + sidecar.branch.as_deref().unwrap_or("main"), + ), + )); + } + Ok(()) +} + /// Bound on the app-level retry of an internal-table compaction against a /// concurrent live writer (see [`is_retryable_lance_conflict`]). const COMPACTION_RETRY_BUDGET: u32 = 5; diff --git a/crates/omnigraph/src/failpoints.rs b/crates/omnigraph/src/failpoints.rs index 3bee1d85..15cd9224 100644 --- a/crates/omnigraph/src/failpoints.rs +++ b/crates/omnigraph/src/failpoints.rs @@ -123,6 +123,11 @@ pub mod names { pub const OPEN_BEFORE_SCHEMA_CONTRACT_READ: &str = "open.before_schema_contract_read"; pub const OPTIMIZE_BEFORE_COMPACT: &str = "optimize.before_compact"; pub const OPTIMIZE_INJECT_REINDEX_CONFLICT: &str = "optimize.inject_reindex_conflict"; + /// After Optimize's broad recovery fast-path check, before the main-branch + /// writer gate is acquired. Tests arm a late recovery intent in this window + /// and prove the under-branch-gate check refuses to advance around it. + pub const OPTIMIZE_POST_RECOVERY_CHECK_PRE_MAIN_GATE: &str = + "optimize.post_recovery_check_pre_main_gate"; pub const OPTIMIZE_POST_PHASE_B_PRE_MANIFEST_COMMIT: &str = "optimize.post_phase_b_pre_manifest_commit"; pub const RECOVERY_BEFORE_ROLL_FORWARD_PUBLISH: &str = "recovery.before_roll_forward_publish"; diff --git a/crates/omnigraph/tests/failpoints.rs b/crates/omnigraph/tests/failpoints.rs index 9b4ff58a..04495077 100644 --- a/crates/omnigraph/tests/failpoints.rs +++ b/crates/omnigraph/tests/failpoints.rs @@ -5425,13 +5425,281 @@ async fn optimize_phase_b_failure_recovered_on_next_open() { .expect("schema apply after optimize recovery must succeed"); } -/// Separately-opened handles for one root share the table/recovery gates. A -/// served insert that starts while optimize is paused before compaction must -/// wait, then commit after optimize releases its sidecar lifetime; it must not -/// deadlock in the synchronous recovery barrier or lose either result. +async fn seed_optimize_late_sidecar_race(dir: &tempfile::TempDir) { + let mut seed = helpers::init_and_load(dir).await; + // Leave real compaction work behind so a missing barrier advances Person + // instead of accidentally passing because Optimize was a no-op. + helpers::commit_many(&mut seed, 4).await; +} + +/// Optimize's entry recovery probe is only a fast path. A graph-global +/// SchemaApply can arm its durable sidecar after that probe while Optimize is +/// waiting for the main-branch gate, then fail before staging any schema/table +/// effect. Once Optimize owns that gate it must relist recovery state and return +/// the older operation's exact RecoveryRequired id before opening HEAD, writing +/// an Optimize sidecar, or moving either the table or manifest. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +#[serial] +#[serial(optimize)] +async fn optimize_rechecks_late_schema_apply_sidecar_after_main_gate() { + let _scenario = FailScenario::setup(); + let dir = tempfile::tempdir().unwrap(); + let uri = dir.path().to_str().unwrap().to_string(); + + seed_optimize_late_sidecar_race(&dir).await; + + // Both handles must exist before the late sidecar: a new read-write open + // would recover it instead of exercising the long-lived-handle race. + let optimize_db = std::sync::Arc::new(Omnigraph::open(&uri).await.unwrap()); + let schema_db = Omnigraph::open(&uri).await.unwrap(); + let person_uri = node_table_uri(&uri, "Person"); + let manifest_uri = format!("{uri}/__manifest"); + let manifest_before = version_main(optimize_db.as_ref()).await.unwrap(); + let physical_manifest_before = lance::Dataset::open(&manifest_uri) + .await + .unwrap() + .version() + .version; + let graph_head_before = branch_head_commit_id(dir.path(), "main").await.unwrap(); + let person_head_before = lance::Dataset::open(&person_uri) + .await + .unwrap() + .version() + .version; + + let rendezvous = helpers::failpoint::Rendezvous::park_first( + names::OPTIMIZE_POST_RECOVERY_CHECK_PRE_MAIN_GATE, + ); + let optimize_task_db = std::sync::Arc::clone(&optimize_db); + let optimize = tokio::spawn(async move { optimize_task_db.optimize().await }); + rendezvous.wait_until_reached().await; + + // Adding @index changes only accepted schema metadata. The failpoint fires + // after SchemaApply's zero-table v5 sidecar is durable, but before schema + // staging or any physical table effect. + let indexed_schema = helpers::TEST_SCHEMA.replace("age: I32?", "age: I32? @index"); + { + let _failpoint = ScopedFailPoint::new(names::SCHEMA_APPLY_BEFORE_STAGING_WRITE, "return"); + let error = schema_db.apply_schema(&indexed_schema).await.unwrap_err(); + assert!( + error + .to_string() + .contains("injected failpoint triggered: schema_apply.before_staging_write"), + "unexpected SchemaApply failure: {error}", + ); + } + let schema_operation_id = single_sidecar_operation_id(dir.path()); + + rendezvous.release(); + let optimize_error = tokio::time::timeout(std::time::Duration::from_secs(20), optimize) + .await + .expect("Optimize task hung after releasing the recovery-check rendezvous") + .unwrap() + .expect_err("Optimize must refuse the late graph-global recovery intent"); + match optimize_error { + OmniError::RecoveryRequired { operation_id, .. } => { + assert_eq!(operation_id, schema_operation_id) + } + other => panic!("expected exact RecoveryRequired attribution, got {other}"), + } + drop(rendezvous); + + assert_eq!( + single_sidecar_operation_id(dir.path()), + schema_operation_id, + "Optimize must not add its own sidecar beside the older SchemaApply intent", + ); + assert_eq!( + version_main(optimize_db.as_ref()).await.unwrap(), + manifest_before, + "late-barrier refusal must happen before any data or internal manifest effect", + ); + assert_eq!( + lance::Dataset::open(&manifest_uri) + .await + .unwrap() + .version() + .version, + physical_manifest_before, + "late-barrier refusal must not compact the physical __manifest afterward", + ); + assert_eq!( + branch_head_commit_id(dir.path(), "main").await.unwrap(), + graph_head_before, + "late-barrier refusal must not manufacture graph lineage", + ); + assert_eq!( + lance::Dataset::open(&person_uri) + .await + .unwrap() + .version() + .version, + person_head_before, + "Optimize must not move the target table HEAD around the older sidecar", + ); + + drop(optimize_db); + drop(schema_db); + + // Full recovery owns the abandoned SchemaApply intent. Once it rolls the + // metadata-only attempt back, the same Optimize work is safe and succeeds. + let recovered = Omnigraph::open(&uri) + .await + .expect("read-write open must recover the abandoned SchemaApply intent"); + assert_post_recovery_invariants( + dir.path(), + &schema_operation_id, + RecoveryExpectation::RolledBack { tables: vec![] }, + ) + .await + .unwrap(); + recovered + .optimize() + .await + .expect("Optimize must succeed after the older recovery intent is resolved"); +} + +/// A main-branch recovery intent overlaps Optimize even when it touched a +/// different table: both operations publish through the shared +/// `graph_head:main`. This pins the branch-wide half of the final barrier. A +/// confirmed Company mutation fails before visibility while Optimize is parked; +/// Optimize must not compact Person or advance main around that fixed authority. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +#[serial] +#[serial(optimize)] +async fn optimize_rechecks_late_disjoint_main_sidecar_after_main_gate() { + let _scenario = FailScenario::setup(); + let dir = tempfile::tempdir().unwrap(); + let uri = dir.path().to_str().unwrap().to_string(); + seed_optimize_late_sidecar_race(&dir).await; + + let optimize_db = std::sync::Arc::new(Omnigraph::open(&uri).await.unwrap()); + let writer_db = Omnigraph::open(&uri).await.unwrap(); + let person_uri = node_table_uri(&uri, "Person"); + let manifest_uri = format!("{uri}/__manifest"); + + let rendezvous = helpers::failpoint::Rendezvous::park_first( + names::OPTIMIZE_POST_RECOVERY_CHECK_PRE_MAIN_GATE, + ); + let optimize_task_db = std::sync::Arc::clone(&optimize_db); + let optimize = tokio::spawn(async move { optimize_task_db.optimize().await }); + rendezvous.wait_until_reached().await; + + // The interrupted writer affects Company, while the productive Optimize + // work this test protects is Person compaction. Its v3 sidecar is confirmed + // and durable; only the main graph-head authority overlaps. + { + let _failpoint = + ScopedFailPoint::new(names::MUTATION_POST_FINALIZE_PRE_PUBLISHER, "return"); + let error = writer_db + .mutate( + "main", + OCC_DISJOINT_MUTATIONS, + "insert_company", + ¶ms(&[("$name", "InterruptedCo")]), + ) + .await + .expect_err("Company mutation must stop after confirming its physical effect"); + assert!( + error + .to_string() + .contains("mutation.post_finalize_pre_publisher"), + "unexpected mutation failure: {error}", + ); + } + let writer_operation_id = single_sidecar_operation_id(dir.path()); + + // Baseline after the older writer's owned Company effect. Nothing below may + // add Optimize movement before recovery resolves that intent. + let manifest_before_optimize = version_main(optimize_db.as_ref()).await.unwrap(); + let physical_manifest_before_optimize = lance::Dataset::open(&manifest_uri) + .await + .unwrap() + .version() + .version; + let graph_head_before_optimize = branch_head_commit_id(dir.path(), "main").await.unwrap(); + let person_head_before_optimize = lance::Dataset::open(&person_uri) + .await + .unwrap() + .version() + .version; + + rendezvous.release(); + let optimize_error = tokio::time::timeout(std::time::Duration::from_secs(20), optimize) + .await + .expect("Optimize task hung after releasing the recovery-check rendezvous") + .unwrap() + .expect_err("Optimize must refuse a late main-branch recovery intent"); + match optimize_error { + OmniError::RecoveryRequired { operation_id, .. } => { + assert_eq!(operation_id, writer_operation_id) + } + other => panic!("expected exact RecoveryRequired attribution, got {other}"), + } + drop(rendezvous); + + assert_eq!( + single_sidecar_operation_id(dir.path()), + writer_operation_id, + "Optimize must not add a sidecar beside the disjoint main writer", + ); + assert_eq!( + version_main(optimize_db.as_ref()).await.unwrap(), + manifest_before_optimize, + "Optimize must not publish a main-table pointer around the older intent", + ); + assert_eq!( + lance::Dataset::open(&manifest_uri) + .await + .unwrap() + .version() + .version, + physical_manifest_before_optimize, + "Optimize must not compact the physical __manifest after refusing", + ); + assert_eq!( + branch_head_commit_id(dir.path(), "main").await.unwrap(), + graph_head_before_optimize, + "Optimize must not invalidate the older intent's fixed graph-head authority", + ); + assert_eq!( + lance::Dataset::open(&person_uri) + .await + .unwrap() + .version() + .version, + person_head_before_optimize, + "disjoint Company recovery must still block Person compaction", + ); + + drop(optimize_db); + drop(writer_db); + + let recovered = Omnigraph::open(&uri) + .await + .expect("read-write open must recover the confirmed Company mutation"); + assert_post_recovery_invariants( + dir.path(), + &writer_operation_id, + RecoveryExpectation::RolledForwardOriginalLineage { + tables: vec![TableExpectation::main("node:Company")], + }, + ) + .await + .unwrap(); + recovered + .optimize() + .await + .expect("Optimize must succeed after the main recovery intent is resolved"); +} + +/// Optimize retains main's branch gate after its final recovery relist and +/// through its effects. A Company insert started while productive Person +/// compaction is paused must therefore wait despite sharing no table gate, then +/// commit after Optimize releases the branch-wide legacy-adapter envelope. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] #[serial(optimize)] -async fn optimize_serializes_concurrent_insert_across_handles() { +async fn optimize_holds_main_gate_through_disjoint_table_effects() { let _scenario = FailScenario::setup(); let dir = tempfile::tempdir().unwrap(); let uri = dir.path().to_str().unwrap().to_string(); @@ -5452,8 +5720,8 @@ async fn optimize_serializes_concurrent_insert_across_handles() { let db_b = std::sync::Arc::new(Omnigraph::open(&uri).await.unwrap()); - // Pause optimize before compaction while it owns the Person table's - // sidecar/queue lifetime. + // Pause productive Person optimize after the under-main-gate recovery + // relist and after its Person sidecar is durable. let failpoint = ScopedFailPoint::new(names::OPTIMIZE_BEFORE_COMPACT, "pause"); let uri_opt = uri.clone(); @@ -5473,16 +5741,16 @@ async fn optimize_serializes_concurrent_insert_across_handles() { writer_db .mutate( "main", - MUTATION_QUERIES, - "insert_person", - &mixed_params(&[("$name", "eve")], &[("$age", 34)]), + OCC_DISJOINT_MUTATIONS, + "insert_company", + ¶ms(&[("$name", "QueuedCo")]), ) .await }); tokio::time::sleep(std::time::Duration::from_millis(50)).await; assert!( !writer.is_finished(), - "same-root writer must wait for optimize's guarded sidecar lifetime" + "table-disjoint main writer must wait for Optimize's branch-gate lifetime" ); drop(failpoint); // release optimize @@ -5490,18 +5758,23 @@ async fn optimize_serializes_concurrent_insert_across_handles() { .await .expect("optimize task hung") .unwrap(); - result.expect("optimize must finish before the queued same-table writer"); + result.expect("optimize must finish before the queued disjoint main writer"); writer .await .expect("writer task panicked") - .expect("queued insert must resume after optimize"); + .expect("queued Company insert must resume after Optimize"); - // No lost write: 4 seed + eve all present; graph remains re-optimizable. + // No lost work on either table; graph remains re-optimizable. let db = Omnigraph::open(&uri).await.unwrap(); assert_eq!( helpers::count_rows(&db, "node:Person").await, - 5, - "concurrent insert must not be lost", + 4, + "Person compaction must preserve every seed row", + ); + assert_eq!( + helpers::count_rows(&db, "node:Company").await, + 1, + "queued table-disjoint Company insert must persist", ); db.optimize() .await diff --git a/docs/dev/testing.md b/docs/dev/testing.md index 1e173222..cb8db5fe 100644 --- a/docs/dev/testing.md +++ b/docs/dev/testing.md @@ -52,7 +52,7 @@ The engine's `tests/` is the principal coverage surface; most graph-shaped behav | `merge_cost.rs` | Cost-budget tests for branch MERGE on the shared `helpers::cost` harness: `merge_validation_is_delta_scoped` (a 1-row-delta merge opens ≤3 data tables — Δ-scoped, not the whole catalog; was ~6 pre-#5) and `merge_manifest_cost_grows_with_history` (the cross-branch `__manifest` open amplification still grows with commit depth — a separate, not-yet-addressed term — while validation `data_open_count` stays flat) | | `policy_engine_chassis.rs` | Engine-layer Cedar enforcement (MR-722): allow + deny through every `_as` writer via the SDK directly — no HTTP — proving embedded and CLI callers hit the same gate as the server, with action × scope shapes matching `authorize_request` | | `maintenance.rs` | `optimize` (compaction), `repair` (explicit uncovered-drift publish), and `cleanup` (version GC): empty/idempotent/no-op edges, policy validation, head preservation; cleanup pins exact keep-count behavior (including keep larger than history), count/time retention of a live lazy branch, the oldest of multiple lazy pins, graph-wide fail-closed ordering on an unopenable pin, and refusal of uncovered main HEAD drift before any GC; `optimize` publishes its own compaction (`optimize_publishes_compaction_to_manifest_so_schema_apply_succeeds`), skips pre-existing uncovered drift (`optimize_skips_preexisting_manifest_head_drift`), refuses to run while a `__recovery` sidecar is pending (`optimize_defers_when_recovery_sidecar_is_pending`), and compacts blob-v2 tables through the normal path (`optimize_compacts_blob_table_alongside_plain_table`); `repair` previews/heals verified maintenance drift, refuses raw semantic drift without `--force`, and forced repair publishes only by explicit operator choice; the index reconciler (iss-848): `index_build_tolerates_null_vector_rows` (logical load succeeds without inline index work; an untrainable Vector column then defers during reconciliation) and `optimize_materializes_index_declared_but_unbuilt` (optimize creates a declared-but-deferred index) | -| `failpoints.rs` | Failure-injection coverage (gated on `failpoints` feature). RFC-022 includes deterministic post-stage/pre-effect races for mutation/load uniqueness and strict disjoint-head changes, plus the cross-handle post-effect `RecoveryRequired` → read-write-open rollback cell. Branch merge adds the captured-source advance cell; post-confirm target-winner compensation; mixed physical + pointer-only delta recovery with fixed commit id/actor/parents; and both sidecar-before-first-ref and ambiguous-ref-create recovery. Native controls are pinned by `native_branch_controls_reclassify_lost_acknowledgements` (matching create and absent-ref delete, with no version/lineage movement); `armed_first_touch_recovery_accepts_missing_target_ref` additionally forges and reclaims the clone-only/no-`BranchContents` table state. Legacy path overlap has both sides pinned: `armed_first_touch_recovery_defers_legacy_path_overlap_until_leaf_delete` permits open only for a proven no-effect intent, while `partial_first_touch_recovery_fails_closed_on_legacy_path_overlap` leaves one exact multi-table effect and verifies open fails closed until offline leaf cleanup lets rollback converge. Other control/recovery race cells include `first_touch_post_create_open_error_keeps_recovery_ownership`, `branch_delete_orphans_sidecar_armed_after_initial_barrier`, `branch_merge_fences_target_delete_recreate_aba`, `branch_merge_fences_concurrent_sync_on_same_handle`, `branch_merge_rejects_fresh_target_manifest_change_before_effects`, `branch_merge_rechecks_late_sidecar_after_table_gates`, `cleanup_rechecks_sidecars_under_gc_gates`, and `full_recovery_rereads_sidecar_body_after_discovery`. The suite also includes the five per-writer effect → manifest-CAS recovery tests, write-entry in-process heal contract, storage-fault matrix, S3 recovery twin, and convergence-idempotent roll-forward regression. | +| `failpoints.rs` | Failure-injection coverage (gated on `failpoints` feature). RFC-022 includes deterministic post-stage/pre-effect races for mutation/load uniqueness and strict disjoint-head changes, plus the cross-handle post-effect `RecoveryRequired` → read-write-open rollback cell. Branch merge adds the captured-source advance cell; post-confirm target-winner compensation; mixed physical + pointer-only delta recovery with fixed commit id/actor/parents; and both sidecar-before-first-ref and ambiguous-ref-create recovery. Native controls are pinned by `native_branch_controls_reclassify_lost_acknowledgements` (matching create and absent-ref delete, with no version/lineage movement); `armed_first_touch_recovery_accepts_missing_target_ref` additionally forges and reclaims the clone-only/no-`BranchContents` table state. Legacy path overlap has both sides pinned: `armed_first_touch_recovery_defers_legacy_path_overlap_until_leaf_delete` permits open only for a proven no-effect intent, while `partial_first_touch_recovery_fails_closed_on_legacy_path_overlap` leaves one exact multi-table effect and verifies open fails closed until offline leaf cleanup lets rollback converge. Other control/recovery race cells include `first_touch_post_create_open_error_keeps_recovery_ownership`, `branch_delete_orphans_sidecar_armed_after_initial_barrier`, `branch_merge_fences_target_delete_recreate_aba`, `branch_merge_fences_concurrent_sync_on_same_handle`, `branch_merge_rejects_fresh_target_manifest_change_before_effects`, `branch_merge_rechecks_late_sidecar_after_table_gates`, `optimize_rechecks_late_schema_apply_sidecar_after_main_gate` (late zero-pin graph-global intent), `optimize_rechecks_late_disjoint_main_sidecar_after_main_gate` (table-disjoint intent sharing `graph_head:main`), `optimize_holds_main_gate_through_disjoint_table_effects` (post-relist branch-gate lifetime), `cleanup_rechecks_sidecars_under_gc_gates`, and `full_recovery_rereads_sidecar_body_after_discovery`. The suite also includes the five per-writer effect → manifest-CAS recovery tests, write-entry in-process heal contract, storage-fault matrix, S3 recovery twin, and convergence-idempotent roll-forward regression. | | `failpoint_names_guard.rs` | Source-walk guard: every `maybe_fail(...)` call site (engine + cluster) must reference the compile-checked `failpoints::names` consts, never a bare string literal — a typo'd literal compiles but silently never fires; same defense-in-depth shape as `forbidden_apis.rs` | | `recovery.rs` | Open-time recovery sweep — legacy sidecars; schema-v3 Mutation/Load exact transaction identity; schema-v4 BranchMerge exact multi-commit chains/ref-only effects plus complete logical delta; restartable compensation recognition; fixed logical/rollback outcome IDs; branch-token comparison; fresh under-gate sidecar reread/reparse; all-or-nothing roll-forward/rollback/refusal; audit row in `_graph_commit_recoveries.lance`; and `OpenMode::ReadOnly` skip path | | `composite_flow.rs` | Compositional/narrative end-to-end stories — multi-step flows that compose mechanics covered by other test files. Catches integration regressions where individual operations all pass their unit tests but their composition breaks (sequential merges, post-merge main writes, time-travel through merge DAG, reopen consistency over multi-merge histories, post-optimize and post-cleanup strict writes). | diff --git a/docs/dev/writes.md b/docs/dev/writes.md index bfe1de1f..21227191 100644 --- a/docs/dev/writes.md +++ b/docs/dev/writes.md @@ -494,8 +494,18 @@ outcome ambiguity while retaining loose table classification. EnsureIndices' schema-v6 payload also retains loose table-effect classification, but gives rollback a fixed commit id and a durable pre-restore audit plan so recovery re-entry cannot flip the outcome. Both close the pre-arm ownership boundary while their full exact -adapters remain future work. Optimize retains its legacy adapter. The -manager is shared by every +adapters remain future work. Optimize retains its legacy effect adapter and a +branch-authority two-stage recovery barrier: the graph-wide entry probe is a fast +path; after acquiring main's process-local branch-writer gate, Optimize relists and +rejects every main-target sidecar plus graph-global SchemaApply before it reads table +HEADs, classifies drift, or arms its own sidecars. The gate is retained through the +internally parallel per-table effects/publishes because each table-pointer publish +advances shared `graph_head:main`. It remains held through final physical-only +`__manifest` compaction so a new main recovery intent cannot arm before raw manifest +movement finishes. This coarse legacy-adapter fence makes same-process sidecar-enrolled +main writers wait for the entire graph-wide Optimize, while Optimize's own table tasks +remain parallel. +The manager is shared by every `Omnigraph` handle for one canonical local root identity (relative, absolute, and symlink aliases converge; object-store/custom schemes stay opaque), so this also serializes a refresh or separately-opened handle against a live writer instead of rolling its @@ -506,7 +516,8 @@ schema → branch → sorted tables → coordinator, matching the writer effect Mutation/load, branch merge, SchemaApply, and EnsureIndices perform one additional `list_dir` after acquiring their authority gates; that final check closes the pre-gate recovery TOCTOU without moving validation or reclaimable staged-file -construction under the gate. +construction under the gate. Optimize's separate final `list_dir` runs under the +main branch gate because even table-disjoint main intents share graph-head authority. Pinned by the four `tests/failpoints.rs::*_after_finalize_publisher_failure_heals_without_reopen` tests (load, mutation, schema apply, branch merge). The maintenance diff --git a/docs/rfcs/rfc-022-unified-write-path.md b/docs/rfcs/rfc-022-unified-write-path.md index 3a098a6a..5f384cc0 100644 --- a/docs/rfcs/rfc-022-unified-write-path.md +++ b/docs/rfcs/rfc-022-unified-write-path.md @@ -508,6 +508,18 @@ able to enumerate every adapter and every entry point that invokes it. - Schema-v6 does not prove ownership of an index commit and does not add a fixed forward outcome. Exact transaction chains, fixed original lineage, and exact first-touch ref identity remain requirements of the full adapter. +- Optimize retains its legacy loose effect adapter. Its synchronous overlap + barrier is two-stage: a broad entry probe, then a relist under main's + process-local branch-writer gate that rejects every main-target sidecar plus + graph-global SchemaApply before HEAD inspection, drift classification, or its + own recovery arm. The gate remains held through internally parallel table + effects/publishes because every published table pointer advances shared + `graph_head:main`, and through final physical `__manifest` compaction so a new + main intent cannot arm before raw manifest movement finishes. This coarse + legacy-adapter safety cost makes same-process sidecar-enrolled main writers + wait for the entire graph-wide Optimize, while Optimize's own table tasks stay + parallel. This is not the future exact Optimize adapter, a universal mutex for + every main publisher, or a distributed fence. - Logical operations never fail because a derived index is absent or behind. - Physical-only internal-table maintenance remains the exception in Section 8. @@ -791,6 +803,13 @@ writers. after manifest CAS, and during sidecar deletion for every adapter. - A later overlapping writer blocks, heals, or returns recovery-required; it never advances around the sidecar. +- If Optimize's entry probe passes and a SchemaApply or table-disjoint main intent + arms before its branch gate is acquired, Optimize returns recovery-required with + that intent's id; it does not create an Optimize sidecar, move another table, + advance `graph_head:main`, or compact `__manifest` afterward. +- After Optimize's under-gate relist passes, a table-disjoint main writer started + while productive table compaction is paused remains queued until Optimize + releases its branch-wide effect envelope, then completes normally. - A live foreign writer's sidecar is not destructively recovered without fencing. - Recovery proves exact effect identity; a numerically newer unrelated version is not accepted as proof of ancestry.