[ragnorc]

Part of #972. A commit that only a branch's __manifest buffers (every commit of an unmerged branch) was found by listing every live branch ref and checking out branch manifests one after another, newest first, on every lookup. A diff side spelled as such a commit looks it up twice, and a commit's own changes up to three times. On a hosted graph with nine live branches, GET /commits/{id}/changes on a one-change branch commit took 13–14 s and a diff of a branch head against itself 13.6 s (measurements in the #972 comment). OmniGraph Studio reads a branch's commits one after another for history and review, so it pays this on every read.

A graph handle's HistoryCache now remembers, per history block, the branch a lookup found a buffered commit on. One native branch writes a block, so the block's other commits stay buffered there until they are released or merged. resolve_commit, commit_state and published_record ask that branch first, before __history and the scan of every live branch: one __manifest open and one branch scan. The guard is that the hint only chooses where to look first. The record still comes from that branch's own __manifest, read the way the scan reads it. A branch that no longer holds the commit, whether released, merged, deleted or recreated under a new native name, drops the hint and the lookup runs as before. The incarnation check that answers 404 for a deleted branch's commit is unchanged. The cache holds at most 4,096 block names per handle, and the new ManifestCoordinator::record_in_hinted_branch is registered as a read-only coordinator surface.

warm_read_cost::by_id_read_of_a_block_found_before_scans_only_its_branch reads two commits of one block from a handle on main with four newer branches. The first lookup scans six branches and the second one; with the hint disabled the second also scans six. The graph as of the hint-found commit reads its rows correctly, and after the branch is deleted the commit still resolves through __history. The neighbouring lookup-order test keeps its counts. The catalog and engine suites pass with the 16 MiB stack CI sets (composite_flow_multi_branch_sequential_merges overflows the default debug stack on main as well), and clippy passes on the pinned 1.97.1 toolchain.

For review: the decision to challenge is answering from the hinted branch's copy of a record before __history. The scan already returns whichever branch holds the commit first, in an order that depends on the handle's own branch, so this relies on nothing the scan did not already rely on. The first lookup of each block still scans every branch; #972's durable address is the fix for that, and this PR leaves it open. I could not run the GQT corpus locally: on this machine under load, every debug case that failed exceeded its 10 s wall-time budget, none with an assertion failure, and one that timed out under load (branch_create_at_a_commit) passed in 8 s alone; CI's GQT job is the check for the corpus. Not measured on the hosted graph.

[ragnorc]

Revise this change before merge.

Why. A by-id read that misses the current branch scans every live __manifest branch. A history list and a review do that read once per commit. The cost grows with the number of live branches.

What. The handle remembers the native branch where it last found a commit of a history block. The next read of a commit in that block opens that branch first. Then it reads __history. Then it scans the live branches.

How. HistoryCache stores the block id and the native name. None means main. record_in_live_branches records the home when a scan finds the commit. resolve_commit, commit_state, and published_record call record_in_hinted_branch before __history.

Impact. The second and later commits of a known block open one branch and scan one branch. The first lookup of a block still scans every live branch. Issue 972 stays open. On the current code a warm handle can also fail a read that __history can answer, and the merge into current main does not compile.

Blocking

1. A missing branch tree stops the lookup

record_in_hinted_branch treats only RefNotFound as a miss. Lance 13.0.0 checkout_branch returns NotFound when the branch tree has no _versions directory. That return is Error::NotFound at lance-table 13.0.0 src/io/commit.rs:653.

Branch delete copies the commits that branch wrote into __history and leaves the tree. Later cleanup removes __manifest/tree/{native}. The maintenance test cleanup_reclaims_retirement_archive_after_the_age_window shows that removal when older_than is Duration::ZERO after the age window.

The next get_commit, snapshot read, or branch-create-from-commit on that handle returns the storage error. The hint stays. Later reads of that block fail the same way. __history can still serve the commit. capture_commit_start_source uses published_record, so branch create from that commit fails on the same path.

The new cost test does not catch this. After delete, checkout of the retired tree still succeeds. The test says the commit is found in __history past a stale hint. That holds only while the tree remains.

Fix the miss in record_in_hinted_branch. Treat NotFound the same as RefNotFound. Forget the home. Return Ok(None). The caller then reads __history and the live branches. A commit that exists only on the missing tree still fails when the later scan meets the same error.

Add a regression in warm_read_cost or the maintenance owner. Learn the hint. Delete the branch. Reclaim the tree the way the age-window test does. get_commit must still return the commit. A .gqt case cannot express a warm handle plus cleanup.

2. The merge into current main does not compile

CI run 40477076158 is a pull_request run of head b5916cbc. It builds merge commit 194ead86 (merge of b5916cbc into 790174c).

Lint, Test Workspace, and Storage Upgrade fail with queries overflow the depth limit in omnigraph-bench. The note names omnigraph-bench.rs:564 and a depth increase of 130.

Azurite fails the same way in omnigraph-cluster test failpoints. The note names async fn body of offline_fixture() and a depth increase of 130.

CI Gate fails because those jobs fail. Main push run 40440899362 at 790174c succeeded. Bench sources match between the merge base and that main commit. The new unboxed async read is what the merge adds inside futures that already meet the compiler depth limit.

refresh already boxes refresh_inner for this hazard. The comment there names the engine stack-depth hazard and keeps that layout out of every caller frame. Box the hinted read the same way. Box it inside record_in_hinted_branch, or at both awaits in record_held_elsewhere and resolve_commit. Do not raise recursion_limit.

Optional

HistoryCache::reset clears settled commits, history objects, and cold registry rows. It does not clear homes.

refresh_inner calls reset when the manifest image changes at the held version. The comment says nothing held from the replaced root describes the new one. The block homes stay.

The hint is read against the current root, so it does not return stale commit bytes. After the NotFound miss is fixed, a stale name is one failed checkout and then the normal lookup. Clear homes in reset as well. One function then owns the rule that this cache does not describe a replaced root. Five more maps that forget reset is the liability to avoid.

Tradeoff and liability

The change removes a scan of every live branch on the second and later commits of a block this handle already found. That matches a history walk on one branch.

The first lookup of a block still scans every live branch. This pull request does not close issue 972. A durable commit-to-branch address remains the general fix. No other open pull request owns this lookup. Pull request 818 caches a different fact, the version a write committed.

The change adds a process-local hint. The hint is not a second source of truth when a miss continues into __history and the live-branch scan. Today a NotFound checkout does not continue. The hint can then fail a read that __history can answer. That liability is on the error path, not on record identity.

HistoryCache already owns the hint state. record_in_hinted_branch must own the miss policy. reset must own invalidation. Those three owners are enough.

Reading the branch buffer before __history keeps the same record when both copies exist. Catalog test block_placement_preserves_fork_and_parent_records_through_two_releases keeps the fork archive and the parent archive equal. The commit row is stored as written. tables_at uses the earliest table replacement. I checked this order. The records stay equal.

A fork stored as the home of a block the writer later extends causes one miss and then a full scan. The result stays correct.

Validation

Reviewed head b5916cbcf754cf6a3a1c9a133dcd87e8698abcb6. Merge base da1c46368c33876d61615add0ceb6778f9ce2b58. Toolchain rustc 1.97.1.

Same command on both commits, from isolated checkouts, with CARGO_TARGET_DIR=/workspace/target:

cargo test --locked -p omnigraph-engine --test integration -- warm_read_cost::by_id_read_of_a_block_found_before_scans_only_its_branch warm_read_cost::temp_commit_stays_readable_after_hinted_branch_tree_is_reclaimed warm_read_cost::by_id_read_of_a_commit_held_elsewhere_scans_the_likely_branches_first -- --test-threads=1 --nocapture

The third name is a temporary test. It is not in the pull request. I removed it after the runs.

Head result. The cost test passed. The neighbor test by_id_read_of_a_commit_held_elsewhere_scans_the_likely_branches_first passed. The temporary test failed at get_commit after removal of every entry under __manifest/tree. The error was Storage(NotFound) for tree/task.../_versions, from lance-table-13.0.0/src/io/commit.rs:653. Asserts before that removal passed, including equal commit records and people_as_of on the second commit. Result: 2 passed, 1 failed, 640 filtered out.

Merge-base result. The temporary test passed. The cost test failed with left: (1, 6) and right: (1, 1) on the second commit of the block. The second read still scans six branches. Commit equality asserts before that line passed. The neighbor test passed. Result: 2 passed, 1 failed.

The scan count is the performance difference. The lost read after tree removal is the behavior regression. It fails on the head and passes on the merge base.

CI run 40477076158 on merge 194ead86: clippy, Test Workspace, Storage Upgrade, and Azurite fail at compile time. Those jobs stop before tests, so they do not show query behavior. GQT and the DST pinned suite passed on that run. I did not rerun them locally.

Lance inspection. Dataset::checkout_branch uses checkout_by_ref, then find_branch on tree/{name}/, then resolve_latest_location. An empty or missing _versions listing returns Error::NotFound.

Limitations

I did not rerun workspace clippy, the full workspace test graph, GQT, or DST on this machine. I did not measure hosted-graph wall time. The local proof is the open count and the scan count.

The local tree removal was remove_dir_all of every entry under __manifest/tree. It was not a full cleanup(older_than: ZERO) after a backdated retirement. The maintenance test is the evidence that cleanup removes that tree. The observed error is the missing _versions listing that checkout returns when that directory is gone.

Temporary edits were restored. I did not push.

Recommendation: revise.

Inline comment, crates/omnigraph-catalog/src/lib.rs line 2441, side RIGHT:

checkout_branch returns NotFound, not only RefNotFound, when the branch tree has no _versions directory. Lance 13.0.0 builds that as Error::NotFound at lance-table src/io/commit.rs:653.

Branch delete leaves the tree. Cleanup later removes __manifest/tree/{native} (cleanup_reclaims_retirement_archive_after_the_age_window, older_than: Duration::ZERO after the age window). This arm then returns the storage error and keeps the hint. __history can still serve the commit. get_commit, commit_state, and published_record all stop here. capture_commit_start_source stops here too.

Treat NotFound as the RefNotFound arm. Forget the home and return Ok(None).

Local proof on b5916cbc. Two commits on branch task, a second handle reads both ids, branch_delete("task"), then removal of every entry under __manifest/tree. The next get_commit failed with Storage(NotFound) for tree/task.../_versions at commit.rs:653. The same steps passed on merge base da1c463, which has no hint. The new cost test does not remove the tree, so checkout still succeeds after delete.

Inline comment, crates/omnigraph-catalog/src/lib.rs line 2421, side RIGHT:

This unboxed async read is inlined into callers that already meet the compiler depth limit.

CI run 40477076158 builds merge 194ead86 (b5916cbc into 790174c). Lint, Test Workspace, and Storage Upgrade fail with queries overflow the depth limit in omnigraph-bench.rs:564 (depth +130). Azurite fails the same way in omnigraph-cluster test failpoints (offline_fixture, depth +130). Main push run 40440899362 at 790174c succeeded.

Box the await the way refresh boxes refresh_inner in this file. The comment on refresh_inner names the stack-depth hazard. Box it here, or at both awaits in record_held_elsewhere and resolve_commit. Do not raise recursion_limit.