[IMP] Bound the CPU, memory and time of an indexing pass - #1
Open
moylop260 wants to merge 5 commits into
Open
Conversation
…ndexing pass
Left alone the indexer sizes itself against the whole machine. Measured inside a
Vauxoo container on an 18 GB MacBook (Docker Desktop VM of 11.9 GiB):
codebase-memory-mcp cli --index-worker 428% CPU 9.0 GB RSS (73.9%)
container irc190_01: NanoCpus=0 CpuShares=0 Memory=0
That is every core the container can see and three quarters of the VM, which
leaves the laptop swapping. Containers are normally started with no cgroup limit
at all, so the contention has to come from here. Every pass now gets a quarter of
the machine, each value overridable on the command line and then by an already
exported CBM_* variable:
--workers N CBM_WORKERS a quarter of the usable CPUs
--max-memory-mb N CBM_MEM_BUDGET_MB a quarter of the usable memory
--index-timeout N CBM_INDEX_WORKER_TIMEOUT_S 7200 (two hours)
"Usable" is what the container may use, not what the host has: the cgroup CPU
quota ("docker update --cpus=5" reads back as 5, where os.cpu_count() keeps
reporting 10) and the cgroup memory limit, falling back to the affinity mask and
/proc/meminfo.
CBM_MEM_BUDGET_MB is not a cap and cannot be used as one. It only tells
codebase-memory-mcp when to log mem.pressure and purge its allocator: the run
above reported "mem.init budget_mb=2986 total_ram_mb=11946 source=ram_fraction"
and still reached 9.0 GB RSS. codebase-memory-mcp exposes no variable that bounds
RSS, and the portable OS mechanisms do not fit either -- RLIMIT_AS/RLIMIT_DATA
count the address space mimalloc reserves without touching it, RLIMIT_RSS is a
no-op on Linux, and writing to the cgroup needs privileges a container running as
odoo does not have.
So the cap is enforced from the parent: the RSS of the child process tree is
sampled while the pass runs -- the parsing happens in the --index-worker child, so
watching only the process we started would miss it -- and the tree is killed when
it crosses the limit. Recovery is the mechanism this tool already ships: the
batches are cumulative, so a smaller --batch-size picks up where the killed pass
left off. --no-enforce-memory goes back to measuring only.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
…e exists The CI matrix runs the suite on Windows too, where signal.SIGKILL does not exist, /proc is absent and "ps" is not a command: the watchdog would have raised AttributeError on the kill path and, worse, sampled an empty process table and reported a cap it was never going to enforce. It now says so once (memory_cap=unenforceable) and runs the pass unbounded, and the tests that need a real process table are skipped there instead of hanging on a process nothing kills.
Reading codebase-memory-mcp shows two of the three knobs were already its job:
* CBM_MEM_BUDGET_MB: mem.c already scales the budget with the machine (25% at
or below 16 GB, 35% at or below 32 GB, 50% above), so on the measured 11.9 GiB
VM it resolves to the same 2986 MB a "quarter of RAM" rule produces. Exporting
it changed nothing.
* CBM_INDEX_WORKER_TIMEOUT_S: it is a NO-PROGRESS window, not a time budget.
index_supervisor.c kills a worker that logs nothing for 15 minutes and every
progress line resets the clock, so setting 7200 did not give a pass two hours,
it made the hang detector four times slower to fire. The flag stays, unset by
default and documented for what it is.
Only CBM_WORKERS was genuinely missing: cbm_default_worker_count(initial=true)
returns total_cores on purpose ("Use all cores for initial indexing -- user is
waiting"), counted with sysconf(_SC_NPROCESSORS_ONLN), which inside a container
reports host CPUs. codebase-memory-mcp documents that gap where it reads the
override and delegates the cgroup quota to its caller. That is what is left here.
The parent-side RSS watchdog is gone, and a real run shows why keeping it would
have been worse than useless: indexing a 325-module instance it reported
"peak_rss_mb=15" while the worker died with signal 9. The pass does not run as a
descendant of the process we spawn ("Preparing one-shot local CBM command..."),
so the sampler was watching the wrong tree and would never have fired. A memory
ceiling belongs to the kernel anyway -- "docker run --memory=3g --cpus=2" costs
one flag, covers everything in the container, and --cpus is picked up by the
worker count above.
Net effect: 400 lines of userspace supervision replaced by the one value
codebase-memory-mcp cannot work out for itself, and no new dependency.
Indexing only .py cut an Odoo instance from 15860 files to 5681 and still ran a 5 GiB container out of memory, so the earlier claim that it takes the peak from ~14 GB to ~5 GB was wrong. The peak tracks extracted nodes (123023 nodes at 4880 MB, about 40 KB each), and nodes come almost entirely from Python, so the extensions are a weak lever and the docs now say so.
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.
Why
Left alone the indexer sizes itself against the whole machine. Measured inside a Vauxoo
container on an 18 GB MacBook (Docker Desktop VM of 11.9 GiB):
That is every core the container can see and three quarters of the VM, which leaves the laptop
swapping (
kernel_taskat 287%, 128 MB free). Containers are normally started with no cgrouplimit at all, so nothing below this tool stops it either:
docker update --cpus=5from theoutside is a patch, the contention has to come from here.
What
Every indexing pass now gets a quarter of the machine. Each value is resolved from the command
line first, then from an already exported
CBM_*variable, then from the machine, and theresolved value and its source are logged:
--workers NCBM_WORKERS--max-memory-mb NCBM_MEM_BUDGET_MBand the enforced cap--index-timeout NCBM_INDEX_WORKER_TIMEOUT_S--no-enforce-memory"Usable" means what the container may use, not what the host has: the cgroup CPU quota and
the cgroup memory limit, falling back to the CPU affinity mask and
/proc/meminfo. On themeasured container
docker update --cpus=5reads back as 5, whereos.cpu_count()keepsreporting 10.
The memory cap is enforced here, not by the environment variable
CBM_MEM_BUDGET_MBis not a cap, and setting it would not have delivered what was asked.It only tells codebase-memory-mcp when to log
mem.pressureand purge its allocator — the runabove reported
mem.init budget_mb=2986 total_ram_mb=11946 source=ram_fractionand stillreached 9.0 GB RSS. Reading
codebase-memory-mcp0.9.1-rc.1 confirms it:src/foundation/mem.ctracks RSS and warns, it never enforces, and none of the 45
CBM_*variables bounds it.The portable OS mechanisms do not fit either:
RLIMIT_AS/RLIMIT_DATAcount the address space mimalloc reserves without ever touchingit, so the pass would die well below its real usage.
RLIMIT_RSSis a no-op on Linux.odoodoes not have.So the cap is enforced from the parent: the RSS of the child process tree is sampled while
the pass runs — the parsing happens in the
--index-workerchild, so watching only the processthis tool started would miss it — and the tree is killed (SIGTERM, then SIGKILL after a grace)
when it crosses the limit:
Recovery is the mechanism this tool already ships and documents: the batches are cumulative, so
everything indexed so far stays in the graph and a smaller
--batch-sizepicks up from there.The tree is signalled member by member instead of through its process group on purpose — putting
the pass in a session of its own would stop Ctrl-C from reaching a run that takes hours.
Verified
pytest),pre-commit run --all-filesclean.irc190_01, not only mocked:cgroup_cpu_quotareturns 5.0 for the
--cpus=5applied to it,cgroup_memory_limit11811160064,default_workers1,default_memory_mb2816.memory_cap_exceeded rss_mb=206 limit_mb=200 processes=2, the grandchild released its memory,and the single-process case returns in 1.13 s.
Worth a second opinion
--cpus=5in place, a quarter isworkers=1. That is what "a quarter" means and it is one flag away, but it is slower than thestatus quo — say so if the fraction should have a floor above 1.
that into a bounded, explainable failure with a documented way forward. It is still a
behaviour change for anyone whose instance legitimately needs more than a quarter of RAM.