perf(graph-db): stream the sealed store build one column at a time - #2282
Conversation
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a374ac5fc4
ℹ️ 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".
| check, | ||
| ) | ||
| }); | ||
| eprintln!("SEALED BUILD peak {peak}"); |
There was a problem hiding this comment.
Remove the diagnostic eprintln from the test
Every successful graph-db unit-test run executes this ad hoc diagnostic, even though the assertion immediately below already reports the measured peak when the budget fails. Remove it rather than merging investigative stderr scaffolding, as the repository requires Hotpath/tracing for maintained observability and explicitly prohibits merged eprintln! scaffolds.
AGENTS.md reference: AGENTS.md:L151-L160
Useful? React with 👍 / 👎.
Refs #2123. The sealed store build no longer holds every pushed value until
finish. It spools values to disk and writes the container one column at a time, so the build's peak is the row topology plus one column.This does not close #2123. At 6 GB the daemon now builds, verifies, and publishes the graph of this repository, and the graph reaches
ready. The daemon is then OOM-killed in the serving decode and activation that follow the publish. Those phases are measured below.Where grafeo lives
grafeo-*0.5.42 come from a git pin on ScriptedAlchemy/grafeo, our fork of GrafeoDB/grafeo; this account has admin on it.perf/compact-direct-build)..cargo/config.tomlreplaces that git source with pnpm's vendored copy, per build: manage npm packages and Cargo sources with pnpm #2091.What changed in grafeo (ScriptedAlchemy/grafeo#5, merged as
2027d1093c)IncrementalCompactStoreBuilderno longer keepsVec<Value>columns. Each column appends only its present values to a byte spool, as bincode(rows skipped, value)records.spooling_to(file)moves each full 32 KiB column buffer into a caller-owned file.Value::Null, exactly what the null-padded column held.write_sectiondecodes, encodes, writes, and drops one column at a time. A relationship table's CSR is built only while that table is written.SectionStreamthat bothCompactStoreSectionand the builder drive. A streamed section is the finished store's section by construction, and a test compares the bytes.GrafeoDB::write_compact_containertakes the builder and streams it; theCompactStoreis never built on this path.What changed here
2027d1093c.SealedCompactRowsspools totempfile::tempfile_in(staging), an anonymous file the OS removes even after SIGKILL. It hands the builder straight towrite_compact_container. The separatefinishstep and its Hotpath span are gone.direct_build_holds_one_column_not_every_value(graph-db lib, per-thread counting allocator):origin/master: 89,678,855 bytes, so it fails there.Builder peak, daemon (clone of this repository, perf+hotpath CLI, one capped scope)
Anon comes from the scope's
memory.stat. Phase windows come from Hotpath span timings. Both columns are on the same base (#2277), under a 12 GB cap:finish(all columns)Before #2277 halved the code-edge rows, the same comparison was 7.87 GB anon in master's
finish(Hotpath resident 7.81 GB) against 4.24 GB in this PR's container write.Per-phase RSS+swap, this PR on #2277, 6 GB cap
readyat 411 s while the project is stillwarming. The scope is OOM-killed at about 426 s.query.artifact.finalization.canonical_layout, +95 s), which doesn't touch graph-db.The next wall is not the builder. After the graph publish, the serving decode is admitted with a 2.8 GB estimate but grows anon from about 3.5 to 6.2 GB. Activation then maps the sealed container, 1.2–1.3 GB of file pages, and warms the catalog.
Identical store bytes and results
Same method as #2271:
tracedecay.grafeo, its WAL, andtracedecay.sealed/.2aeddd58b843331b7439…2aeddd58b843331b7439…sha256:12ca34bd5ee1f229…005411bf59d269645e51…005411bf59d269645e51…sha256:0ebf2ee223be14b6…The #2185 query set (search/context/callers/callees on 20 symbols), warm:
Seal-time allocator spike (reported, not bounded)
MIMALLOC_SHOW_STATS=1, daemon stopped at 15.8 s with anon at 6.7–6.96 GB):Tests and suites
daemon_suite::sealed_generation_crash_test, against the built CLI):sigkill_after_the_seal_restarts_on_the_identical_sealed_generationandsigkill_during_the_seal_keeps_the_prior_generation_published, 2 passed.tracedecay-graph-db: lib 144 (1 ignored),graph_db_suite153 (2 ignored).tracedecay-code-index: lib 258,code_index_suite171,resident_accounting2.tracedecay-query: lib 263 (1 ignored),search_quality_suite71,canonical_execution_equivalence5,retrieval_contract_spine2.tracedecay-store-runtime: lib 121 (1 ignored).tracedecay-code-index-runtime: lib 516 when run alone. The full suite on a host at load 45–114 fails a varying set of deadline and git-identity tests, and the base commit fails them too. Filed as test(code-index-runtime): lib tests fail under host load #2281; each passes alone.grafeo-core --features compact-storecompact tests 509 passed;compact_container_write5 passed.cargo clippy ... -D warningson the touched crates andcargo fmt --all -- --checkare clean.