Repository navigation
fix(storage): keep a bound handle's re-reads on its own session - #1103
Merged
Merged
Conversation
A graph handle opened on a captured storage binding still built some of its clients from the process-wide Lance session or from the root URI, so an ambient `AWS_ENDPOINT*` setting sent their requests to another endpoint: - the live re-read a write takes before publishing (the `@card` edge count) opened `__manifest` with the session-less `ManifestCoordinator::open`; - a branch delete reopened `__manifest` without a session to compare the branch identifier, then retired the ref and wrote its archive there; - a catalog `Snapshot` without a manifest dataset (merge bases and the snapshots of commit records behind commit-addressed reads, the change feed, proposals, unattached snapshots and collector views) opened its tables on the process-wide session; - a historical CSR read loaded the graph-index artifact through an adapter derived from the root URI; - Lance's `create_branch` probes the new branch location on a default session built from the store parameters it is given, which a bound dataset leaves empty. A catalog `Snapshot` now carries the control session it was read with. The branch-control reopen and the live re-read use the coordinator's and the handle's session, the direct graph-index builder uses the handle's adapter, and a native branch create, at the head or at a commit, passes the binding's captured options. The session-less catalog openers compile only in test builds, so a build of the CLI or server refuses a new production caller. `bound_storage.rs` runs a bound workload in a child process whose ambient endpoint variables name a recording decoy, and requires the decoy to see no request. Before this change 52 requests reached it; each of the five fixes, reverted alone, turns it red. The RustFS job runs it.
The change feed re-proves a named branch's head after its table opens, on a zero-cache session over the store's registry. Lance keys a cached manifest by dataset URI and version, plus the ETag where the store reports one, so the store's data session, warmed by an earlier page, must not be the one that answers. The test warms a data session with the branch's own `__manifest`, runs the re-prove and requires the data session's metadata cache statistics to be unchanged; through the data session it records eight hits.
The testing guide names the decoy workload and the change feed's re-prove test, the Lance guide records `create_branch`'s default-session probe as a compatibility fence, and the architecture guide states that a catalog snapshot keeps the control session it was read with. A release note records the requests that now reach a captured destination.
The decoy forwarders relay plain HTTP, and the test panicked when `OMNIGRAPH_S3_TEST_BUCKET` named AWS itself, with no endpoint override, or an HTTPS-compatible endpoint, so an ordinary engine test run against a supported S3 configuration failed. It now skips there with the reason, under the `skipping s3` prefix the RustFS job's skip guard refuses, so the job's configured run still has to execute it.
4 of 7 tasks
ragnorc
added a commit
to ragnorc/omnigraph
that referenced
this pull request
Oct 21, 2026
Brings in main through ModernRelay#1076: the ModernRelay#1074 squash, ModernRelay#1092 and ModernRelay#1103.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
#1090 binds one storage provider to a graph handle, so its control objects and Lance reads all go to the target's endpoint with its credentials. Five re-reads still opened
__manifest, tables or the graph index without the handle's session. On a process whose ambientAWS_*settings name another endpoint, those requests went there instead.Pointing the ambient endpoints at a recording decoy, unmodified main sent 52 requests to it. With unsigned requests and conditional puts disabled in the ambient environment as well, the workload failed at the first
@cardmutate.What
Each re-read now uses the handle's bound session or adapter:
@cardcardinality check's live re-read before publish__manifest/_versions/; the mutate fails__manifest/_versions/; the delete fails__graph_index/csr-current.bincreate_branch, which probes the new branch on a default session in Lance 13tree/<branch>/_versions/; the branch create failsThe session-less
ManifestCoordinatoropeners now compile only in test builds, so a production caller cannot reach the ambient store again. A check of the CLI and server, as releases build them, fails with E0599 when main's session-less re-read is put back. That is why they leave forbidden_apis' public read-surface registry.A unit test also pins the change feed's branch-head re-prove to a fresh session. Pointed at the data session, its cache hits go from 1 to 9.
Already correct on main, so not changed here: Azure option capture, the S3 option classes and the proxy refusal.
Tests
crates/omnigraph/tests/bound_storage.rs(new) puts two recording forwarders in front of RustFS and points a child process's ambient endpoints at them. It requires zero decoy requests through writes, reads, the cardinality re-read, a racing publish, a three-way merge, branch create and delete, historical and commit-addressed reads, the change feed, export, optimize and cleanup. It is enrolled in the RustFS job as the required cellbound_handle_never_reaches_the_ambient_endpoint.bound_storage,lance_surface_guards,s3_clusterands3_storage::.-D warnings, fmt, check-docs, typos, check-agents-md and check-ci-cells pass.docs/dev/lance.mdgains a "Bound branch create" fence row, checked against Lance 13;testing.mdandarchitecture.mdname the owner. Onechangelog.dnote is included.