perf(engine): reclaim deleted branch forks in the background

[azimafroozeh]

What & why

Follow-up to #540. That PR made branch merge faster; this one makes branch delete faster. Both respond to the same report of merge and delete timeouts.

Deleting a branch does two things. Removing the branch ref from the manifest is the logical deletion: one small write, after which no read can see the branch. Reclaiming the per-dataset Lance forks the branch owned is physical cleanup: a serial chain of object-store round trips that grows with the number of datasets the branch wrote.

Before this PR the response waited for both. Now branch_delete returns at the ref removal, and the fork reclaim continues in a background task. The task holds the request's schema, branch, and dataset gates until it finishes, so every concurrent branch control serializes behind the reclaim exactly as before. Only the caller stops waiting.

• A 600 s watchdog bounds how long a wedged object store can pin the carried gates. On expiry the reclaim is abandoned to its backstops.
• A branch that never forked a dataset skips the dispatch and releases its gates at the response.
• New Omnigraph::wait_for_fork_reclaims() joins pending reclaims. The CLI calls it before process exit, the server after graceful drain, tests for determinism.

Measured with a delete-scenario A/B on the #537 bench harness (release builds, warm local FS, 5-rep medians, T = datasets the branch forked):

T	Response before	Response after	Speedup	Reclaim tail (background)
8	10.2 ms	5.2 ms	2.0x	8.3 ms
50	54.8 ms	17.2 ms	3.2x	40.3 ms
140	134.5 ms	42.4 ms	3.2x	112.6 ms

Response plus convergence time is conserved across the two trees: the reclaim moved, it did not shrink. The effect grows on object storage, where every serial round trip costs real network latency. The delete scenario is the checked-in instrument and follows as its own PR on the #537 harness; none ships here.

Backing issue / RFC

• None filed. This addresses the reported branch delete timeouts; perf(merge): classify branch merges from fragment lineage #540 addressed the merge side of the same report. No RFC: reversible change inside the documented best-effort reclaim contract (docs/dev/invariants.md: derived tree reclaim may converge later).

Checklist

• Focused: one mechanism, branch delete's fork reclaim moves off the request path
• Tests: two new deterministic rendezvous failpoint tests (the response returns while the fork still exists; a same-name recreate serializes behind the carried gates), two adapted
• Docs: docs/user/branching/index.md (delete bullet), docs/user/reference/constants.md (the watchdog), docs/user/operations/server.md (the shutdown join)
• Invariants reviewed: no Hard Invariant weakened, no deny-list hit; this moves the convergence point, not the contract

Local verification

• cargo test --workspace --no-fail-fast (failpoints superset): green except the pre-existing sandbox-only external_blob_file_policy_rejects_special_files
• cargo clippy --workspace --all-targets (CI shape) and cargo fmt --all --check: clean
• Vocabulary guard: all three surfaces clean against the merge base

Notes for reviewers

• Commit point: the response boundary now coincides with the authority flip, which was already the durable deletion. Nothing that can fail the request runs after it.
• Recreate race: the carried gates make a same-name create wait for the reclaim. The write path's first-write self-heal of leftovers is the second line of defense.
• The export-destructive permit is released at the response, not carried. The reclaim touches only fork trees no live export cut can reference, and the export gate is a try-lock, so carrying it would fail a post-response export instead of making it wait.
• Watchdog expiry can leave a dataset with its ref removed but tree residue remaining. That state is invisible to cleanup's ref-listing sweep and converges on the next same-name create, the same state a crash after the ref delete leaves. The warn message names both paths.
• One deliberate semantics change: orphan forks may briefly outlive the response. cleanup remains the backstop either way.
• The server joins pending reclaims on both shutdown exit paths. This is the engine crate's first production tokio::spawn; non-tokio embedders fall back to inline reclaim.

Greptile Summary

The PR moves branch-delete fork reclamation behind the manifest authority flip, allowing deletion responses to return before physical cleanup while preserving serialization through carried write-queue gates.

• Adds tracked background reclaim tasks with a 600-second watchdog and an explicit join API.
• Joins pending reclaims before embedded CLI exit and after server graceful request drain.
• Adds failpoint coverage for early acknowledgement and same-name recreation serialization.
• Documents the asynchronous deletion behavior, shutdown join, and watchdog constant.

Confidence Score: 5/5

The PR appears safe to merge, with background reclamation preserving the existing authority and serialization contracts.

The manifest remains the logical deletion commit point, same-process conflicting operations remain excluded by the carried gates, and production exit paths join registered reclaim tasks after branch-delete dispatch or graceful request drain.

Important Files Changed

Filename	Overview
crates/omnigraph/src/db/omnigraph.rs	Introduces tracked background fork reclamation, carries the existing serialization gates into the task, bounds cleanup with a watchdog, and exposes a deterministic join API.
crates/omnigraph-server/src/lib.rs	Retains the graph registry through serving and joins each graph's pending reclaims after graceful request drain on both server exit paths.
crates/omnigraph-cli/src/client.rs	Joins fork reclaims after embedded branch deletion so process exit cannot cancel newly detached cleanup.
crates/omnigraph/tests/failpoints.rs	Adds deterministic rendezvous tests proving response-before-reclaim behavior and serialization of same-name recreation.
crates/omnigraph/tests/branching.rs	Adapts branch recreation coverage to wait for asynchronous physical cleanup.
crates/omnigraph/tests/forbidden_apis.rs	Classifies the new join method as a non-durable public async surface.
docs/user/branching/index.md	Documents the new logical-delete response boundary and background physical cleanup behavior.
docs/user/operations/server.md	Documents shutdown joining of pending branch-delete reclaims.
docs/user/reference/constants.md	Records the 600-second reclaim watchdog and its convergence behavior.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Engine
    participant Manifest
    participant Reclaim as Background reclaim
    participant Lance
    Client->>Engine: branch_delete(name)
    Engine->>Engine: Acquire schema, branch, and dataset gates
    Engine->>Manifest: Remove authoritative branch ref
    Manifest-->>Engine: Logical deletion committed
    Engine->>Reclaim: Spawn cleanup carrying gates
    Engine-->>Client: Delete succeeded
    Reclaim->>Lance: Remove owned dataset forks
    alt Reclaim completes
        Lance-->>Reclaim: Forks removed
    else 600-second watchdog expires
        Reclaim->>Reclaim: Abandon cleanup and log warning
    end
    Reclaim->>Reclaim: Release carried gates

Loading

_{Reviews (1): Last reviewed commit: "perf(engine): reclaim deleted branch for..." | Re-trigger Greptile}

Context used (3)

• Knowledge Base — Contain Partial Branch Delete Failures
• Knowledge Base — Self-heal orphaned branch forks before writes wedge
• Knowledge Base — Harden branch recovery and cleanup retention

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.