fix(code-index): serve the first index without the whole decode - #2332
Conversation
A first index at a 6 GB cap parked forever: after the text build, the sealed graph build (charged the decode's 2.82 GB retained bytes) and then the serving decode were measured against VmRSS, which counts clean mapped container pages, and ready waited on a decoded seat. - Admission reads RssAnon + RssShmem through the pressure cell's sampler (injectable), and a refused decode or text build returns the pool workers' freed allocator pages before re-measuring, whether or not an owner was shed. - A publication seats its graph head from the text owner's mapped store (verified-head recovery), so ready no longer needs the decode. With no decoded seat, search serves the text owner, so an empty seat is terminal; the released-seat bookkeeping is gone. - Project-open owners (query authority, advisory, TypeScript producer), the LSP census and feedback identity read the sealed manifest and no longer demand the whole decode. - A graph build refused while its own text build holds the memory retries at the text join instead of parking; a refused decode after the graph serves does not park convergence. - A decode records its measured peak growth and its next decode is charged that, not the retained figure. Cross-file edges resolve before per-file copies into one exact-capacity vector.
|
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: 20efb3f130
ℹ️ 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".
| if (generation_ready || text_graph_catalog_warm.is_some()) | ||
| && text_graph_catalog_warm != Some(false) |
There was a problem hiding this comment.
Keep the readiness probe decode-free
When ProductionProjectCompositionHarnessV1 waits here, the generation_ready probe immediately above calls latest_complete_ready_for_scope, whose AwaitDecode path sets complete_generation_requested. That flag makes registry/mount.rs decline defer_serving_decode, so every default harness open still rehydrates the whole generation before this new text-graph readiness branch can hand over the composition, reproducing the memory pressure this change is intended to avoid. Use a decode-free readiness probe in this path.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
| .is_some_and(|text| text.interactive_graph_store().is_ok()); | ||
| if (generation_ready || recovered_text_ready) | ||
| .and_then(|text| text.interactive_graph_store().ok()) | ||
| .map(|store| store.interactive_catalog_is_warm().unwrap_or(false)); |
There was a problem hiding this comment.
Propagate catalog warm failures
If the background catalog warm enters InteractiveCatalogState::Failed, the catalog lock is poisoned, or the warm-state check otherwise returns a typed error, interactive_catalog_is_warm() returns Err; converting every error to false makes this loop treat a permanent failure as ordinary warming until the outer 20-second timeout and then report the misleading “did not publish” error. Preserve the typed failure here, retrying only explicitly transient contention if appropriate.
AGENTS.md reference: AGENTS.md:L189-L190
Useful? React with 👍 / 👎.
Refs #2123. A first index of this repository under
MemoryMax=6G MemorySwapMax=1Gnow reachesready(fresh, graph serving) without an OOM kill and without the serving decode. On the final build, 3 of 4 runs had no park. The fourth still parked for 435 s at the sealed graph build's admission before it recovered, so this PR does not close the issue. The remaining gap is at the end.What was wrong
VmRSS. That includes the mapped sealed container and graph store pages, which the kernel drops on demand before the cgroup kill line.What changed
ResidentMemoryPressureV1samples through an injectableProcessResidentSamplerV1. The production sampler parses/proc/self/statusintoVmRSSandRssAnon + RssShmem. Admission, the text-artifact budget and the pressure latch use the unreclaimable bytes.daemon.process.resident_bytesstaysVmRSS. A newdaemon.process.unreclaimable_resident_bytesgauge carries the admission figure.publish_sealed_graph, the published pass seats the head on the text owner withrecover_verified_head, the same path a restart uses. The whole decode runs only when a reader demanded it (code_index_serving_decode_deferred), and a stale predecessor seat is released.released_seatis deleted.metadata().snapshot().files.latest_feedback_generation_for_scopeno longer awaits a decode.request_complete_generation/latest_complete_ready*demands are removed.serving_generation_changedwhen its projection finishes, so owners that waited on the seat still wake.DecodePeakProbeV1) and recordsdecode_peak_growth_bytes. The next decode of that generation is chargedmax(retained, measured peak)(daemon.code_index.generation.decode.peak_growth_bytes).Admission view at the post-text-build retry (6 GB)
Why #2295 parks where master completed (measured)
Both binaries ran on the same clone at 6 GB.
Per-phase peaks, 6 GB (cgroup RSS+swap / anon, GB)
No run had
oom_kill. The earlier builds of this branch reachedreadyin all 5 runs (322–455 s). acc6a, acc6b, acc6c and dec6 ran on the rebased build before the last guard (no rebuild or re-recovery of an already-serving graph). That guard changes only retained passes under decode demand, not init.12 GB time to ready (same clone, same host)
Time to ready is within noise: master averaged 281 s with a 67 s spread, this PR 283 s. Peak memory dropped by about 3.8 GB because init no longer decodes.
Decode transient
decode.peak_growth_bytesreads 5.19 GB. The process-wide probe also counts concurrent work.resident_accounting): the transient is 2,407,636 B before and after.Retrieval identity
The #2185 query set (search, context, callers, callees per symbol) was compared against master at 697755b. Per-run identity is normalized: profile cursor keys, handles, timestamps and the time-sealed generation id.
Tests
memory_tests::a_decode_is_admitted_against_unreclaimable_bytes_not_clean_file_pagessrc/lib.rs::retained_generation.resident_bytes: "clean file pages do not refuse a decode that fits in anonymous headroom".daemon::code_index_runtime_graph_activation_tests::first_index_serves_graph_reads_without_decoding_the_generationwait_for_readiness(Ready)reaches, withsealed_decode_count == 0.["alpha"]withCurrentfreshness. The census reports 28 source bytes.sealed_decode_countis 1.resident_memory::tests::process_status_splits_clean_file_pages_from_unreclaimable_bytes: the/proc/self/statussplit, with literals.ProductionProjectCompositionHarnessV1hands a composition over once its graph catalog is warm.Proof
daemon_suite::sealed_generation_crash_test, against the built CLI): 2 passed.code_index_suite172,resident_accounting2.runtime_core_suite14,git_repository_authority16, and the smaller binaries.canonical_execution_equivalence5,retrieval_contract_spine2,search_quality_suite70 of 71.reader_refuses_historical_v10_writer_artifact_as_incompatiblealso fails on master (test(query): artifact reader tests fail on master after bounded restore #2313).graph_db_suite153 (2 ignored).application_suite64,pr_tracking7.concurrent_same_identity_worktrees_keep_exact_server_and_scheduler_bindings, fails on master as well.move_symbol_testraced the background catalog warm. With the guard and the harness change it passed 15/15 in 6 of 6 runs.cargo clippy -p tracedecay-runtime-core -p tracedecay-code-index -p tracedecay-code-index-runtime -p tracedecay -p tracedecay-graph-db --all-targets -- -D warningsis clean with and withouttracedecay/test-transport.cargo fmt --all -- --checkis clean.Open
symbol_search,qualified_name,signature_search,source_metadata,facets,timeline, workspace diagnostics, doctor and branch publication still take the whole decode on demand.madviseof mapped pages is not added. With admission counting only anon and shmem, clean mapped pages no longer affect admission.MADV_DONTNEEDon a shared file mapping would not uncharge the page cache from the cgroup anyway.