fix(index): supervised-subprocess RSS isolation for background auto-index + watcher#853
Open
DeusData wants to merge 4 commits into
Open
fix(index): supervised-subprocess RSS isolation for background auto-index + watcher#853DeusData wants to merge 4 commits into
DeusData wants to merge 4 commits into
Conversation
…vised subprocess (RSS isolation) The long-lived MCP server ran the full index pipeline in-process on two background paths -- the session auto-index (autoindex_thread, mcp.c) and the watcher re-index (watcher_index_fn, main.c). The parallel pipeline's worker threads abandon their mimalloc pages at thread exit, and mimalloc v3 defaults page_reclaim_on_free=0, so those pages are never reclaimed when the main thread later frees their blocks (mi_collect cannot touch abandoned pages either). RSS then ratchets across re-index cycles and never comes back down (#832). The index_repository tool path already avoided this by running the index in a supervised worker subprocess: on child exit the kernel reclaims 100% of the child's RSS. Route the two background paths through that same supervised runner: - supervisor_invalidate_store() is now NULL-safe (the watcher path has no MCP server / cached store). - New static index_run_supervised_path() builds {"repo_path": root} and reuses the existing spawn + skip-and-continue recovery loop (no duplication). New public cbm_mcp_index_run_supervised_path() is the srv-less entry main.c calls. - autoindex_thread and watcher_index_fn gate on cbm_index_supervisor_should_wrap(): supervised on a marked host with the kill switch off, else fall through to the unchanged in-process pipeline (kill switch / spawn-failure degrade). The watcher keeps its non-blocking try_lock gate + g_shutdown check exactly; the lock still serialises re-indexes. cbm_mem_collect() stays on the in-process branch only. Also fold in the in-process allocator fallback: cbm_mem_init sets mi_option_page_reclaim_on_free=1, restoring v2 behaviour so any path that stays in-process (kill switch, degrade, embedders) also reclaims abandoned worker pages. The subprocess is the primary cure (child exit returns RSS every cycle); this is the fallback. Tests: a deterministic gating guard (test_mcp.c) proves the background paths now spawn a worker child and index the fixture (RED when routed in-process: spawn_count unchanged; GREEN after). The test-runner acts as a real in-process worker when spawned as `cli --index-worker` (public APIs only, no production seam). A non-gating prod-binary RSS reproduction (tests/repro/issue832_rss.py) shows the supervised server flat at ~10 MB vs ~131 MB in-process. Scope: JobObject, the #841 spurious-trigger fix, retention caps, budget tiering and the backpressure busy-spin are follow-ups. Refs #832, #841. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…cess-isolation Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com> # Conflicts: # tests/test_mcp.c
…atch (keystone×#849) The #832 supervisor keystone added a second watcher-registration site in autoindex_thread's supervised-success branch, but wired it as a direct cbm_watcher_watch() guarded only by `if (srv->watcher)`. srv->watcher is set unconditionally (cbm_mcp_server_set_watcher), so that guard does NOT honour the auto_watch config. PR #849 had routed ALL watcher registration through register_watcher_if_enabled() (which checks auto_watch_enabled() and logs watcher.register.skipped reason=auto_watch_off). On a supervisor-marked prod host with a fresh (unindexed) project and `config set auto_watch false`, the supervised path therefore registered a background watcher against the user's explicit opt-out — defeating #849's multi-project containment. The in-process twin ~20 lines below already used register_watcher_if_enabled() correctly. Fix: replace the ungated block in the supervised-success branch with register_watcher_if_enabled(srv), matching the in-process branch. Surrounding supervised-success logging is unchanged. Reproduce-first: mcp_auto_watch_false_skips_supervised_autoindex_issue853 drives the real public entry initialize -> maybe_auto_index -> autoindex_thread on a supervisor-marked host (kill switch off) with a FRESH project and auto_watch=false, in a fork+alarm-bounded child (like the #832/#845 gate tests). cbm_mcp_server_free() joins the autoindex thread so the registration decision has run before the watch count is read. It asserts spawn_count advanced (so the probe cannot pass vacuously by skipping the supervised branch) and watch_count == 0. RED on the unfixed code (watch_count == 1, exit 61); GREEN after the fix (register_watcher_if_enabled skips → watcher.register.skipped reason=auto_watch_off). Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…IN32 The idx832/idx853 fork-harness helpers are used only in the POSIX (#else) branch of their tests; on Windows (SKIP_PLATFORM) they are unused → -Werror,-Wunused-function on the CLANG64 leg. Guard the definitions to match their call sites. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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
1 participant
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.
Keystone memory fix (#832) — DRAFT, stacked on #851
cbm_index_supervisor_mark_host/spawn_count). Until #851 merges to main, this PR's diff includes #851's commits; the moment #851 is in main, the diff self-cleans to only the memory changes below.Root cause
mimalloc v3 flipped the reclaim default (
page_reclaim_on_free0 = only reclaim pages from the current thread's heap). Our worker threads exit, abandon their pages, and the long-lived server frees those blocks from a different heap → they never reclaim → RSS ratchets. Andmi_collect(true)cannot touch abandoned pages (code-confirmed against mimalloc dev3). The two background paths (autoindex_thread,watcher_index_fn) ran the full pipeline in-process, so every cycle ratcheted; theindex_repositorytool path already isolated via the supervisor subprocess.Fix
Route
autoindex_thread+watcher_index_fnthrough the same supervised subprocess (index_run_supervised); the kernel returns 100% of each cycle's RSS on child exit. Fold inmi_option_page_reclaim_on_free=1as the in-process fallback for the kill-switch/degrade path. Atomic: 7 files, +454/−3. Preserves kill-switch, degrade-on-spawn-failure, watcher try_lock; args JSON-escaped (yyjson, argv not shell); gate requires the post-#851 host-mark.Evidence
10 index cycles: supervised peak 10.5 MB vs in-process 131.5 MB (12.5×) — the #832 ratchet cured (RSS repro runs against the prod
MI_OVERRIDE=1binary; a C RSS test on the ASan runner would be vacuous). Deterministic spawn-count gate guard RED→GREEN; crash-containment tool path intact; -Werror + lint clean.Credit
The
page_reclaim_on_free=1lever was first identified by @fxfxfx123 in #833 — that PR's core diagnosis, folded in here. (Its bundledarena_purge_mult/fraction/worker-cap changes were dropped as inert/wrong-layer.) #833 will be closed crediting this once it merges.Scope
Isolation only. Separate follow-up PRs: #841 (spurious Windows watcher trigger — the #832 trigger), Windows Job Object worker lifecycle, #685 (retention), #752 (host-tiered budget), backpressure de-spin. So #832 is not fully closed by this PR — the isolation lands here; the trigger + retention peak follow.
Refs #832.