Skip to content

Fence Optimize against late recovery intents#347

Merged
aaltshuler merged 2 commits into
mainfrom
codex/optimize-late-recovery-barrier
Jul 12, 2026
Merged

Fence Optimize against late recovery intents#347
aaltshuler merged 2 commits into
mainfrom
codex/optimize-late-recovery-barrier

Conversation

@aaltshuler

@aaltshuler aaltshuler commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

What & why

Close the recovery TOCTOU between the initial sidecar probe and Optimize effects. Every main table-pointer publish also advances the shared graph head, so a late recovery intent on another main table must fence Optimize too.

Optimize now acquires the process-local main branch gate, relists recovery state under that gate, rejects main-target intents plus graph-global SchemaApply, and retains the gate through the internally parallel table phase and final physical manifest compaction.

Backing issue / RFC

  • Fixes an accepted issue: Closes #
  • Implements / is an accepted RFC: RFC-022 unified write path
  • Trivial fast-lane — no issue/RFC required

Checklist

  • Change is focused (one logical change)
  • Tests added/updated for behavior changes
  • Public and developer docs updated
  • Reviewed against docs/dev/invariants.md — no Hard Invariant weakened and no deny-list item hit

Notes for reviewers

The intentionally coarse legacy-adapter tradeoff is that same-process sidecar-enrolled main writers wait for the graph-wide Optimize run. Optimize table tasks remain parallel, and reads do not take this gate. This is process-local coordination, not a distributed lock; cross-process safety remains Lance OCC plus recovery classification.

Deterministic failpoint coverage proves:

  • late zero-pin SchemaApply is rejected with its exact recovery operation id;
  • a late disjoint Company mutation blocks Person Optimize because both share main graph-head authority;
  • the main branch gate remains held after the final relist during disjoint table effects;
  • refusal does not move data HEAD, graph lineage, logical manifest state, or physical manifest HEAD.

Verification:

  • cargo test -p omnigraph-engine --features failpoints --test failpoints (106 passed)
  • cargo test -p omnigraph-engine --locked
  • cargo test -p omnigraph-engine --test maintenance optimize_
  • cargo test -p omnigraph-engine --test failpoint_names_guard
  • scripts/check-agents-md.sh
  • git diff --check

Open in Devin Review

Greptile Summary

This PR fences Optimize against late recovery intents on the main branch. The main changes are:

  • Adds a second recovery sidecar check after acquiring the main branch gate.
  • Holds the main gate through parallel table optimization and final __manifest compaction.
  • Adds failpoint coverage for late SchemaApply and disjoint main-table recovery races.
  • Updates the developer docs and RFC text for the new Optimize fence.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
crates/omnigraph/src/db/omnigraph/optimize.rs Adds the main-gate recovery relist and keeps the gate held through table work and physical manifest compaction.
crates/omnigraph/tests/failpoints.rs Adds coverage for late recovery sidecars and disjoint main-writer serialization during Optimize.
AGENTS.md Documents the branch-wide Optimize recovery fence in the capability matrix.
docs/dev/writes.md Documents the two-stage Optimize barrier and main graph-head authority.
docs/rfcs/rfc-022-unified-write-path.md Adds the Optimize legacy-adapter fence semantics and expected race coverage.

Reviews (2): Last reviewed commit: "Preserve manifest compaction after table..." | Re-trigger Greptile

Context used:

  • Context used - AGENTS.md (source)
  • Context used - CLAUDE.md (source)

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Comment thread crates/omnigraph/src/db/omnigraph/optimize.rs Outdated
Comment thread crates/omnigraph/tests/failpoints.rs
@aaltshuler aaltshuler merged commit 58defb4 into main Jul 12, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant