perf(graph-db): store relation identities once, on the locator - #2268
Conversation
Every relation stored its identity, source and target as 71-byte strings twice: on its locator node and again on its native edge. The locator now owns them. The edge keeps only its owner scalars and payload, and edge reads resolve the locator through the existing RELATION_EDGE unique index and read single columns from it. A `<kind>:<64 lowercase hex>` identity is stored as U+0001, its kind and its digest in unpadded base64url (50 bytes for a symbol id); any other identity is stored verbatim, and graph identifiers may no longer start with U+0001. Unique keys become base64url text of their binary form. Both stay strings because the sealed compact store keeps `Bytes` values in its string dictionary as marked hex, so the binary keys from #2257 took 102 bytes on disk, not 47; they now take 63. Graph format 3 -> 4 with the same typed `FormatSuperseded` rebuild. Paging and proofs still order by the decoded identity strings. Refs #1103
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97de4c3efd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut locators = store | ||
| .find_nodes_by_property(RELATION_EDGE_PROPERTY, &value) | ||
| .into_iter() | ||
| .filter(|node| store.get_node_property(*node, &key).as_ref() == Some(&value)); |
There was a problem hiding this comment.
If a persisted locator's namespace or projection scalar is corrupted while its RELATION_EDGE value and endpoint identities remain intact, this lookup accepts it solely by edge ID. Traversal and fan-out validate ownership on the native edge but then return the identity from this unchecked locator, so they can successfully expose a foreign or mismatched relation instead of reporting GraphDbError::Corrupt. Verify the locator label and ensure its namespace, projection, and kind agree with the edge before using its identity.
AGENTS.md reference: AGENTS.md:L189-L191
Useful? React with 👍 / 👎.
1 participant
Refs #1103. Continues #2257. Relation identities are now stored once, on the locator node, in a compact form. The sealed graph store shrinks by 15–27% at the same base, and retrieval results are unchanged.
What was wrong
Value::Bytes, and grafeo's compact dictionary storesBytesas a marked hex string (dict_value.rs:"\0gfo1:b:" + hex). So those keys took 102 bytes on disk, not the 47 that perf(graph-db): store unique keys as binary namespace-id keys #2257 reported. A first attempt at this slice withBytesidentities made locator identities larger (84 bytes) than the strings they replaced. The code-graph projector already works around this for JSON records (record_property).Encoding
RELATION_ID/_FROM/_TOsymbol:<64 hex>U+0001 ‖ kind ‖ base64url(digest), 50 bytes for a symbol id and 48 for an edge id; any other identity stays verbatimRELATION_ID/_FROM/_TORELATION_EDGEunique index and reads those columns from itBytes(47 B), stored as 102 B of marked hextargetrelation (edge entity → symbol)get_node_property, a single-column read with no node materialization. Owner scalars and payload stay on the edge. Relation lookups keep their endpoint checks: source and target come from the locator and are verified against native adjacency, as before.validate_opaquenow rejects graph identifiers that start with U+0001, so a compact identity can never be confused with a verbatim one. Only the canonical lowercase spelling compacts, so decoding restores the exact identity string.14caed2d7d).Format revision: graph-db 3 → 4, with the same typed
FormatSupersededrebuild: the old container and its sealed generations are deleted and republished from source. Nothing is converted.Bytes per store per source byte
Base is master
6554ff1901. After is this branch at the same base. Both use the perf CLI, a fresh isolated profile, and one capped daemon: 6 GB, or 12 GB with 4 workers for the clone. Source isgraph_statistics.source_total_bytes.Clone peak memory fell from 12.41 GB to 11.42 GB, and time to
readyfrom 486 s to 412 s.Identical results
This uses the #2185 fixture (
search,context,callers,calleesper symbol), driven through the CLI. Per-request fields and the profile-keyed generation id are normalized. Cost receipts are compared verbatim.contextgraph lane raced the post-activation warm (a known race, see #2257)Upgrade in place: a j768 profile built by the master binary was restarted with this binary. It reached
readyin 15 s, and status showsfresh. The typed rebuild republished from source (log:graph generation has neither a complete staged row set nor a usable sealed artifact; republish from the canonical manifest). The new sealed store is 165,868,823 B, byte-identical in size to a fresh index. The code-index artifacts (segments, text artifact, manifests) are byte-identical, and only graph files differ.#2224 receipt. Every page of starship
src/modules/mod.rs::handlecallees (depth 1, no dispatch) is identical to master: 112 rows over 12 pages, 11graph_sealedpoint reads per page, 2 adjacency queries and 226 rows per page, and the samebytes_hydratedon every page.Tests: fail on master, pass here
To check that these fail on master, this commit's graph-db
srcandCargo.tomlwere checked out atorigin/master, and the tests below were run against them.sealed_generation_bytes_stay_within_the_compact_identity_budget: a 2,000-symbol sealed generation with stable identities, published through the production path, must be at most 1,150,000 B, and it resolves through keys and edges. Master: 1,786,870 B (fails). Here: 1,037,302 B.superseded_format_store_is_rebuilt_fresh_with_its_sealed_generations_discarded: a format-3 store, with a format-3 binary-keyed row next to a sealed generation, is rebuilt fresh. Master: opens it as current (fails).compact_relation_identities_read_back_through_keys_and_edges: a raw format-4 store with a literal base64url key, compact locator identities, and an identity-free native edge.relation,outgoing_relations, andoutgoing_relation_idsall return the literal relation. Master: refuses format 4 (fails).persisted_scalar_identity_mismatch_is_corrupt_on_point_readnow writes a literal base64url key (master fails). A schema unit test pins the compact form ofgraph_stable_identity("edge", "occ")(48 bytes) and its exact round trip, plus verbatim storage of non-canonical identities.Checks (rebased onto
14caed2d7d,--profile perf)cargo clippy -p tracedecay-graph-db --all-targets --features test-helpers -- -D warningsis clean. It is the only crate this slice touches.cargo fmt --all -- --checkis clean.What remains above ~10× source
The clone graph store is still 12.1× source and j768 is 24.4×. These are estimates from row counts, not a column census:
edge:entity carrying its JSONCanonicalRelationEdgeV1record, plus two relations (source,target), each a locator node and a native edge. That is about 1.13M locators for code edges alone, roughly 250 B each (key, identity, endpoints, dictionary codes), plus the edge entity rows.ENTITY_ID(71 B) plus its key (63 B).