fix(cockpit-server): wire chains OrdinalIndex into the request-time gather path - #143
Conversation
…ather path main.rs's boot sequence converted the .chains sidecar to Lance but discarded the returned OrdinalIndex (bound to `_index`), so CHAINS_INDEX was never populated. gather_chains()'s `CHAINS_INDEX.get()?.as_ref()?` therefore always short-circuited to None, and every tile/feature request needing chain geometry permanently fell back to the eager open_chains() singleton — a OnceLock populated once on first use and never freed. Live-confirmed on maps.oga.red: cgroup memory sat at ~850MB baseline, jumped to ~5.7GB after the first chains-touching request, and never came back down — matching the reported symptom (memory doesn't go down again after opening the map in browser). Add osm_chains_books_lance::publish_chains_conversion(), the one call site responsible for turning an ensure_chains_lance_local() result into a set_chains_index() call, with its own unit tests covering both the Some (wires the index) and None (fails open, no panic) arms. main.rs now calls it instead of its old broken inline match. Also corrected the now-stale "not yet consumed by the read path" log wording for both chains and books (task #16 already wired the Lance-first-with-eager-fallback read path for both). Verified: cargo check -p cockpit-server --bin q2-cockpit --tests passes clean (0 errors; 33/18 warnings, at or below the pre-existing baseline). Full `cargo test`/nextest for this binary is not runnable in this environment (transitively links most of the workspace, exceeds the container's disk allowance) — documented per prior sessions' practice rather than claimed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6ca046dd-4ec0-48b2-aa3e-bd18ddaede02) |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fe5ca3a3-cc25-4ec1-b49b-a55001fd7bcf) |
Summary
Root-causes and fixes the live memory-growth regression on
maps.oga.red(reported: "after opening the map in browser memory doesn't go down again").
main.rs's boot sequence converted the.chainssidecar to a Lancedataset but discarded the returned
OrdinalIndex(Some((dataset_dir, _index)) => ...), soCHAINS_INDEXwas never populated.gather_chains()'sCHAINS_INDEX.get()?.as_ref()?therefore alwaysshort-circuited to
None, and every tile/feature request needing chaingeometry permanently fell back to
open_chains()— an eager,process-global
OnceLocksingleton that decodes the entire.chainsfile into memory once and never releases it.
/api/osm/status:cgroup_memory.current_bytessat at ~850MB baseline and jumped to ~5.73GB after the first
chains-touching request, then stayed flat forever — matching the
reported symptom exactly.
osm_chains_books_lance::publish_chains_conversion(), the onefunction responsible for turning an
ensure_chains_lance_local()resultinto a
set_chains_index()call, with unit tests for both theSome(wires the index) and
None(fails open, no panic) arms.main.rsnow calls it instead of the old broken inlinematch.for both the chains and books boot-time conversion blocks — an earlier
PR (Claude/add palantir dashboard 3 mn on #16 in this branch's task list) already wired the
Lance-first-with-eager-fallback read path for both.
Test plan
cargo check -p cockpit-server --bin q2-cockpit --tests— 0 errors,33/18 warnings (at/under the pre-existing baseline)
publish_chains_conversion_wires_the_index_into_the_global_gather_path,publish_chains_conversion_stays_silent_on_nonecargo test/nextestfor this binary is not runnable in thiscontainer: it transitively links most of the workspace and exceeds
the environment's disk allowance (documented limitation, same as
prior PRs on this branch)
cgroup_memory.current_bytesreturns toward baseline after achains-touching request instead of staying pinned at ~5.7GB
🤖 Generated with Claude Code
https://claude.ai/code/session_01NMeiLmtDKhomJNSo2ecbJw
Generated by Claude Code