indexer: raise semantic worker clamp from 2 to 4 (#109) - #120
Merged
IceRhymers merged 1 commit intoJul 26, 2026
Merged
Conversation
Re-derives the memory model for indexer/repo_config.py's effective_workers clamp: measured per-source-byte materialization coefficients, resident vector cost, #108's per-process RSS, and a measured container memory ceiling all show N=4 clears 0.7x the container budget with margin. Two live-job runs at N=4 confirm it empirically -- peak self+children RSS landed at ~83% of budget, actually more comfortable than N=2's own ~90%, with wall clock down ~11%. index_concurrency's default stays 4: ingest thread-scaling shows no real gain at 4 threads, and the clamp raise alone makes effective_workers == index_concurrency at the default. Both derived byte limits (MAX_EXTRACTED_BYTES, the chunk cap) are documented but left unchanged -- no repo in the measurement corpus approached either the current or derived value, so there is no empirical signal to justify moving a constant whose breach closes the whole run's reconciliation checkpoint. New indexer/job.py peak-RSS instrumentation (RUSAGE_SELF/RUSAGE_CHILDREN) made the live-job comparison possible; a new test pins that the advisory shas_fn connection closes before embedding starts, since pool_size == effective_workers depends on that sequencing. Full derivation, coefficients, and both live-job before/after runs are in docs/perf/issue-109-measurements.md. The semantics tripwire fires on this branch as an expected, pre-documented false positive (indexer/ingest.py is in SEMANTICS_PATHS and postdates origin/master locally) -- not a real extraction-semantics change, and not a stop condition per the tripwire's own escape hatch. Refs #109
4 tasks
5 tasks
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
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.
Summary
indexer/repo_config.py's semanticeffective_workersclamp and raises it from 2 to 4, based on measured per-source-byte materialization coefficients, resident vector cost, indexer: process-pool symbol/edge extraction #108's per-process RSS, and a measured container memory ceiling — all showing N=4 clears 0.7x the container budget with margin.self+childrenRSS at N=4 landed at ~83% of the 0.7×M budget — actually more comfortable than N=2's own ~90–91% — with wall clock down ~11%. No OOMs, no degraded-coverage warnings, no 429s across all 5 semantic-index runs this session (1 priming + 2 Arm A + 2 Arm B).index_concurrency's default stays 4: ingest thread-scaling (scripts/measure_ingest_threads.py) shows no real gain at 4 threads (1.15–1.20x, short of the 2.0x threshold), and the clamp raise alone makeseffective_workers == index_concurrencyat the shipped default — no separate change needed.MAX_EXTRACTED_BYTES, the per-repo chunk cap) are documented in code comments and the runbooks but left unchanged: no repo in the measurement corpus approached either the current or the derived value, so this run gives no empirical signal to justify moving a constant whose breach closes the whole run's reconciliation checkpoint.indexer/job.pypeak-RSS instrumentation (RUSAGE_SELF/RUSAGE_CHILDREN, logged once per run) made the live-job before/after comparison possible — there was no RSS instrumentation anywhere in the codebase before this.tests/unit/test_job.py::test_shas_fn_connection_closes_before_embedding_starts) pins that the advisoryshas_fnconnection closes before embedding starts, sincepool_size == effective_workersdepends on each worker holding at most one connection at a time.docs/perf/issue-109-measurements.md.Notes
config.yamlreverted to the pre-PR live config, the 3 extra repos retired via a normal (non-mass) reconciliation purge in a separate clean run. The trackedconfig.yaml'sconnections:block is untouched (comment-only changes elsewhere in the file).max_concurrent_runs: 1andqueue.enabled: trueare untouched.tests/unit/test_semantics_version_tripwire.py::test_semantics_change_bumps_the_index_semantics_versionfails on this branch. This is an expected, pre-documented false positive (indexer/ingest.pyis inSEMANTICS_PATHSand postdatesorigin/masterlocally, per the test's own module docstring) — not a real extraction-semantics change from this PR, and not a stop condition.Refs #109
Test plan
uv run pytest tests/unit -q --ignore=tests/unit/test_webui_main.py— 1281 passed, 1 expected pre-documented failure (semantics tripwire, see above).test_webui_main.pyis skipped locally for a pre-existing missingfastapidependency, unrelated to this change (confirmed present onmaster/base too).make lint(ruff check, ruff format --check, mypy app indexer webui) clean, apart from the same pre-existing localfastapimypy error (confirmed present on the base branch too, CI installsfastapi).docs/perf/issue-109-measurements.mdfor full numbers.