4 commits

Author	SHA1	Message	Date
Ragnor Comerford	e62d9166fb	fix: optimize publishes compaction; recovery roll-back converges manifest (#141) ... * test(optimize): cover manifest publish + HEAD-drift reconcile Red against the pre-fix optimize, which ran compact_files without publishing the compacted version to __manifest: - maintenance: optimize must publish so the manifest table_version tracks the compacted Lance HEAD and a later schema apply succeeds; and must reconcile a pre-existing manifest-behind-HEAD drift (forged via raw Lance compaction) so strict writes commit again. - end_to_end + composite_flow: post-optimize query / strict update / reopen in the full lifecycle (the canonical flow previously omitted post-optimize writes as a documented "known limitation"). - failpoints: a crash between compaction and the manifest publish rolls forward on next open. * fix(optimize): publish compaction to manifest and reconcile HEAD drift optimize ran Lance compact_files without publishing the new version to __manifest, so the manifest table_version lagged the Lance HEAD: reads stayed pinned to the pre-compaction version, and the next schema apply or strict update/delete failed its HEAD-vs-manifest precondition with "stale view ... refresh and retry" (open-time recovery rollback inflated the gap on retry). optimize now publishes each compacted table's version under the per-(table, main) write queue, guarded by a manifest CAS and a SidecarKind::Optimize recovery sidecar (loose-match; roll-forward is safe because compaction is content-preserving). When a table has nothing left to compact but its Lance HEAD is already ahead of the manifest pin (pre-fix drift, or a recovery restore commit), optimize reconciles the manifest forward to HEAD (metadata-only, no sidecar). Caches and the CSR/CSC graph index are invalidated after a publish. Docs updated (maintenance, storage, branches-commits, writes, testing). * test(recovery): rollback convergence + optimize-defer regressions Red against the current code, landed before the fix: - recovery: after the open-time sweep rolls a sidecar back, the manifest must track Lance HEAD (no residual drift) so a follow-up schema apply succeeds — the original "+1 per retry" loop. Today roll-back restores without publishing, so the manifest lags HEAD and the apply fails its HEAD-vs-manifest precondition. - maintenance: optimize must refuse while a recovery sidecar is pending — operating on an unrecovered graph could publish a partial write the sweep would roll back. Also removes optimize_reconciles_preexisting_manifest_head_drift: the ad-hoc drift reconcile it covered is replaced by recovery-side convergence. * fix(recovery): converge manifest on roll-back; optimize defers on pending recovery Root of PR #141's review findings and the original "+1 per retry" loop: a Lance HEAD ahead of the manifest was ambiguous (benign content-preserving drift vs. a partial write a sidecar will roll back), and optimize's reconcile guessed it benign. Close the class instead of guessing: - Recovery roll-back now PUBLISHES the restored version (via a push_table_update_at_head helper shared with roll-forward), so the manifest tracks the Lance HEAD after recovery — symmetric with roll-forward. This fixes the +1 loop (after one roll-back the retry's HEAD-vs-manifest precondition passes) and removes the only remaining source of orphaned drift. The audit still records the logical rolled-back-to version; the manifest is published at the restore commit (identical content). - optimize drops the ad-hoc drift reconcile and instead REFUSES when a __recovery sidecar is pending, so it only ever operates on a recovered graph (manifest == HEAD); its compaction publish can no longer commit a partial write. With the reconcile gone, the blob-skip-vs-reconcile gap is moot. Updates the rollback recovery-test helper (manifest == HEAD after roll-back), the failpoints assertions, and the user/dev docs. * test(recovery): fix rollback assertion for manifest convergence The roll-back-publishes change makes the manifest version advance after a SchemaApply roll-back (to the old-schema content), so the schema_apply_without_schema_staging_rolls_back_on_next_open assertion must be `version > pre`, not `version == pre`. This update was dropped during the commit churn and surfaced as a CI Test Workspace failure; the old-schema-preserved intent stays covered by count_rows + _schema.pg + the RolledBack convergence invariant.	2026-06-08 02:50:12 +03:00
Ragnor Comerford	d54bccb940	fix(optimize): skip blob-bearing tables to avoid Lance compaction crash (#138) ... * test(optimize): pin Lance blob-column compaction failure as a surface guard Lance compact_files mis-decodes blob-v2 columns under its forced BlobHandling::AllBinary read ("more fields in the schema than provided column indices"), failing even a pristine uniform-V2_2 multi-fragment blob table; reads use descriptor handling and are unaffected. Guard 10 reproduces this and is self-retiring: it turns red on the Lance bump that fixes the bug, forcing LANCE_SUPPORTS_BLOB_COMPACTION to flip. * fix(optimize): skip blob-bearing tables instead of crashing compaction omnigraph optimize aborted the whole sweep when any node/edge table had a Blob property: Lance compact_files cannot decode blob-v2 columns under AllBinary (the column-index error pinned by the surface guard). Skip blob-bearing tables behind a LANCE_SUPPORTS_BLOB_COMPACTION gate and report them via TableOptimizeStats.skipped / SkipReason (surfaced in the CLI and a tracing::warn) instead of erroring, which also isolates the failure so the other tables still compact. Reads/writes are unaffected; only fragment/space reclamation on blob tables is deferred until the upstream Lance fix. Adds a maintenance.rs regression test (validated red with the column-index symptom before the fix, green after), a concise v0.6.1 release note, and updates docs (maintenance, cli-reference, AGENTS capability matrix, invariants Known Gaps, lance.md audit, constants). * refactor(optimize): make TableOptimizeStats and SkipReason non_exhaustive Both are returned result types, never built by callers, so #[non_exhaustive] makes this the last field/variant addition that can break downstream literal construction and keeps future ones non-breaking (review feedback on the public-field addition). The v0.6.1 Compatibility Notes call out the source-level change. Also drops the now-stale "RED today / GREEN after the fix lands" narration in the optimize_skips_blob_table_and_reports_skip test (historical regression context now that the fix is in this branch), and folds in the expanded v0.6.1 release note. * chore(release): bump workspace to v0.6.1 Coherent version bump to accompany the v0.6.1 release note: all five crate manifests + path-dependency constraints, Cargo.lock, the AGENTS.md surveyed-version line, and openapi.json info.version move 0.6.0 -> 0.6.1. Matches the established release pattern (#118 landed the v0.6.0 note + bump together) and resolves the Codex/Devin review flag that a v0.6.1 note without a bump leaves CARGO_PKG_VERSION reporting 0.6.0 and mixed package versions.	2026-06-02 17:12:00 +02:00
Ragnor Comerford	353c0c876a	fix(branch): make branch delete correct under partial failure (#137) ... * test(lance): pin force_delete_branch surface guard Pin the Lance 6.0.1 force_delete_branch behavior the branch-delete single-authority redesign relies on: plain delete_branch errors on a missing ref, force_delete_branch removes an existing forked branch, and the local-store quirk where force_delete on a fully-absent branch still errors (worked around by the upcoming TableStore::force_delete_branch). Re-pin the docs/dev/lance.md alignment stanza (9 guards; 4 runtime). * feat(storage): add force branch-delete to TableStore + CommitGraph Add TableStore::force_delete_branch and CommitGraph::force_delete_branch (idempotent: tolerate an already-absent branch via Lance RefNotFound / NotFound), plus CommitGraph::list_branches for the cleanup reconciler to diff against the manifest authority. RefConflict (referencing descendants) is still surfaced. Unused until the branch-delete rewire. * test(maintenance): red — cleanup reconciles orphaned branch forks Forge a Lance branch on the Person table that the manifest never references (a zombie fork from an incomplete prior delete) and assert cleanup reclaims it while leaving main intact. Fails today: cleanup does not yet reconcile orphaned forks. Goes green with the next commit. * fix(maintenance): reconcile orphaned branch forks in cleanup Add reconcile_orphaned_branches: force_delete_branch every per-table and commit-graph Lance branch absent from the manifest branch set (the authority), children-before-parents. Folded into cleanup_all_tables, runs before version GC. Idempotent and authority-derived; no-ops once nothing is orphaned, and would harmlessly find nothing if a future Lance atomic multi-dataset branch op prevented orphans. Adds TableStore::list_branches and exposes graph_commits_uri(pub crate). Turns the maintenance red test green. * test(failpoints): red — branch_delete partial failure converges Add the branch_delete.before_table_cleanup failpoint hook (inert without the feature) and a regression test: a cleanup-step failure after the manifest authority flip must leave branch_delete returning Ok, the branch gone, the orphan stranded, then reclaimed by cleanup, and the name reusable. Fails today: cleanup_deleted_branch_tables propagates the error as a hard failure. Goes green with the next commit. * fix(branch): best-effort fork reclaim after the manifest flip Make branch_delete treat per-table forks and the commit-graph branch as derived state reclaimed best-effort with force_delete_branch after the manifest authority flip. A reclaim failure (transient error, or the branch_delete.before_table_cleanup failpoint) is logged via tracing::warn and swallowed: the branch is already gone and the cleanup reconciler converges the orphan. cleanup_deleted_branch_tables no longer returns an error or blocks the call. Turns the partial-failure recovery test green. * test(failpoints): red — recreate over orphaned fork is actionable After a partial-failure delete leaves a fork orphaned, recreating the branch name and writing to the previously-forked table before cleanup runs currently surfaces the opaque ExpectedVersionMismatch ("stale view ... expected manifest table version N"). Assert instead a clear error pointing the user at cleanup. Goes green with the next commit. * fix(branch): actionable orphan-collision error in fork_branch_from_state When a fork's create_branch collides with an existing target ref, reuse it only if its head matches source_version (a legitimate concurrent first-write). A version mismatch means a zombie fork from an incomplete prior delete: return a manifest_conflict pointing the user at `omnigraph cleanup`, instead of the opaque ExpectedVersionMismatch. Turns the recreate-over-orphan red test green. * docs(invariants): single-authority branch-lifecycle + Lance forward-compat Record branch delete in the Current Truth Matrix: manifest is the single authority flipped atomically first, per-table forks + commit-graph branch are derived state reclaimed best-effort with the cleanup reconciler as backstop, and reusing a name whose reclaim failed surfaces an actionable error. Note the reconciler is authority-derived and degrades to a no-op under a future Lance atomic multi-dataset branch op, the same shape as invariant 7. * test(failpoints): red — cleanup isolates a single-table failure Add the cleanup.table_gc failpoint hook (inert without the feature) and an error: Option<String> field on TableCleanupStats (mechanical, always None for now). Regression test: a one-shot version-GC failure for one table must not abort the whole cleanup — assert cleanup still succeeds, surfaces the failure per-table in stats, and the independent reconcile pass still reclaimed an orphan. Fails today: the version-GC collect aborts on the first table error. Goes green with the next commit. * fix(maintenance): fault-isolate cleanup per table Make the cleanup sweep do as much as it can and converge on re-run instead of aborting wholesale on one table's transient error (invariant 13). The version-GC loop now records a per-table failure on its stats row (error: Some) and logs it rather than collecting into a Result that aborts; reconcile_orphaned_branches isolates per-table and commit-graph failures into BranchReconcileStats.failures. The CLI reports any failed tables and tells the user to rerun cleanup. Addresses the Devin review finding. Turns the single-table-failure test green. * test(failpoints): red — branch_create heals commit-graph zombie + is atomic Add the branch_delete.before_commit_graph_reclaim failpoint hook and two regression tests: (a) recreating a name whose delete left a commit-graph zombie must succeed (today it dies on Lance's internal Clone error), and (b) branch_create must roll back the manifest branch when the derived commit-graph branch fails (today it leaves the manifest branch created while returning Err). Both fail now; green with the next commit. The existing branch_create_failpoint_triggers test still passes. * fix(branch): make branch_create atomic + heal commit-graph zombie branch_create now flips the manifest authority first, then creates the derived commit-graph branch in create_commit_graph_branch, force-dropping any orphaned commit-graph ref left by an incomplete prior delete (the manifest branch is fresh, so a same-named commit-graph branch is provably a zombie). If commit-graph creation fails, the manifest branch is rolled back so the name never half-exists. Addresses the Codex review finding. Turns the two branch_create red tests green; existing tests unaffected. * test(failpoints): red — fork collision misclassifies live concurrent fork Add the fork.before_classify failpoint hook and a concurrency test: when a concurrent first-write legitimately wins the fork race, the loser must get a retryable refresh-and-retry, not the misleading run-cleanup orphan error. Today the version-comparison misclassifies the live fork as an orphan (the Cursor finding). Goes green with the next commit. * fix(branch): manifest-arbitrated fork-collision classification Classify a fork collision by the manifest authority instead of comparing Lance branch versions. Before forking, open_owned_dataset_for_branch_write re-reads the live manifest: if the table is already forked on the active branch, a concurrent first-write won and the loser gets a retryable refresh-and-retry (not a misleading orphan error). fork_branch_from_state no longer guesses from versions — a create collision past that check is an orphan, so it returns the actionable cleanup error. Addresses the Cursor finding; turns the live-concurrent-fork test green, zombie path unchanged. * test(failpoints): close branch-lifecycle test gaps Three coverage additions for the branch-delete work (behavior already correct; these lock it in and catch regressions): - cleanup_isolates_reconcile_failure: inject a force-delete failure into the reconcile loop (new cleanup.reconcile_fork hook) and assert the sweep continues + converges on re-run. Directly covers the reconcile loop the Devin finding was about (previously only version-GC was). - cleanup_reclaims_orphaned_commit_graph_branch: forge a commit-graph orphan via the delete reclaim failpoint and assert cleanup's reconcile_commit_graph_orphans drops it (previously untested). - fork_collision_with_live_concurrent_fork_is_retryable: replace the fixed 300ms sleep with a deterministic readiness signal (cfg_callback + compare_exchange atomics) so the two-writer ordering can't flake. Full failpoints suite 31/0.	2026-06-01 13:28:38 +02:00
Andrew Altshuler	60eee78465	docs: split user and developer docs (#93)	2026-05-15 03:45:22 +03:00

Renamed from docs/maintenance.md (Browse further)