From d1889804267fac8b7d7d852eff0c19a148cd9452 Mon Sep 17 00:00:00 2001 From: "Moises Lopez - https://www.vauxoo.com/" Date: Tue, 1 Sep 2026 12:50:29 -0600 Subject: [PATCH 1/5] [IMP] codebase-memory-deployv: bound the CPU, memory and time of an indexing 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. --- README.md | 66 ++++- src/codebase_memory_deployv/cli.py | 42 ++- src/codebase_memory_deployv/indexer.py | 20 +- src/codebase_memory_deployv/limits.py | 383 +++++++++++++++++++++++++ tests/test_cli.py | 50 +++- tests/test_indexer.py | 45 ++- tests/test_limits.py | 199 +++++++++++++ 7 files changed, 795 insertions(+), 10 deletions(-) create mode 100644 src/codebase_memory_deployv/limits.py create mode 100644 tests/test_limits.py diff --git a/README.md b/README.md index 9a7f111..0824490 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,8 @@ That single command: 4. Renders `.cbmignore` (Odoo core `odoo/odoo` complete + selected modules, dropping `static/lib`, `static/tests`, minified JS, caches and `.git`) in **cumulative batches of 25 modules**, running `codebase-memory-mcp cli index_repository` after each batch so memory-killed - one-shot indexing is never a problem. + one-shot indexing is never a problem. Every pass is bounded to a quarter of the machine + (see [Resource limits](#resource-limits)) so indexing stays a background job. 5. Validates the graph scope: no `test_*` addon modules indexed, no missing and no extra module roots compared to the expected module list, and **every indexed file carrying one of the configured extensions** (`SOURCE_GLOBS`: `.py .xml .js .rst .md .css .scss .csv`) — anything @@ -84,10 +85,73 @@ That single command: --batch-size N modules per cumulative indexing pass; 0 indexes one-shot (default: 25) --mode MODE auto (default), installed, or all --modules-file PATH ir.module.module export listing the modules to index +--workers N indexing workers; default is a quarter of the usable CPUs +--max-memory-mb N memory one pass may use; default is a quarter of the usable memory +--index-timeout N seconds one index worker may run (default: 7200) +--no-enforce-memory do not kill a pass that grows past --max-memory-mb --skip-install do not install codebase-memory-mcp when missing --skip-validate do not validate graph scope after indexing ``` +## Resource limits + +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): + +```text +codebase-memory-mcp cli --index-worker 428% CPU 9.0 GB RSS (73.9% of the VM) +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 this tool. Every pass now runs with a quarter of the machine: + +```text +workers=1 source=default usable_cpus=5 +memory_mb=2816 source=default total_ram_mb=11264 enforce=yes +index_worker_timeout_s=7200 source=default +``` + +| Limit | Default | Handed to the child as | +|---|---|---| +| Workers | a quarter of the usable CPUs, never below 1 | `CBM_WORKERS` | +| Memory | a quarter of the usable memory, never below 512 MB | `CBM_MEM_BUDGET_MB` **and the enforced cap** | +| Index worker timeout | 7200 s (two hours) | `CBM_INDEX_WORKER_TIMEOUT_S` | + +Each value is resolved from the command line first, then from an already exported `CBM_*` +variable, then from the machine — so exporting `CBM_WORKERS` in a container still works and the +resolved value and its source are logged. + +"Usable" means 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()` would keep reporting 10) and +the cgroup memory limit, falling back to the CPU affinity mask and `/proc/meminfo`. + +### Why the memory cap is enforced here + +`CBM_MEM_BUDGET_MB` is **not** a cap. It only tells codebase-memory-mcp when to log +`mem.pressure` and purge its allocator: the run measured 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 ever +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 +this tool started would miss it) and the tree is killed when it crosses the limit: + +```text +memory_cap_exceeded rss_mb=3012 limit_mb=2816 processes=2; killing the pass +Batch 3: the indexing pass reached 3012 MB, over the 2816 MB cap. Lower --batch-size so each +pass parses fewer modules, or raise --max-memory-mb when the machine can afford it +``` + +Recovery is the mechanism this tool already ships: the batches are cumulative, so everything +indexed so far stays in the graph and a smaller `--batch-size` picks up from there. Use +`--no-enforce-memory` to go back to measuring only — the budget is still exported, it just stops +being a cap. + ## Indexing the modules installed in production A local docker container only installs the modules of the main app, while production usually has diff --git a/src/codebase_memory_deployv/cli.py b/src/codebase_memory_deployv/cli.py index e55c683..20b21ff 100644 --- a/src/codebase_memory_deployv/cli.py +++ b/src/codebase_memory_deployv/cli.py @@ -22,6 +22,7 @@ validate_scope, write_cbmignore, ) +from .limits import DEFAULT_INDEX_TIMEOUT_S, MemoryLimitExceeded, resolve_limits _logger = logging.getLogger(__name__) PACKAGE_LOGGER = __name__.rsplit(".", 1)[0] @@ -77,6 +78,35 @@ def build_parser(): "Without a state column the file is assumed to list installed modules only. Ignored by --mode=all" ), ) + parser.add_argument( + "--workers", + type=int, + default=None, + help="indexing workers (CBM_WORKERS); default is a quarter of the usable CPUs", + ) + parser.add_argument( + "--max-memory-mb", + type=int, + default=None, + help=( + "memory one indexing pass may use; exported as CBM_MEM_BUDGET_MB and enforced on the " + "process tree. Default is a quarter of the usable memory" + ), + ) + parser.add_argument( + "--index-timeout", + type=int, + default=None, + help="seconds one index worker may run (CBM_INDEX_WORKER_TIMEOUT_S) (default: %d)" % DEFAULT_INDEX_TIMEOUT_S, + ) + parser.add_argument( + "--no-enforce-memory", + action="store_true", + help=( + "do not kill a pass that grows past --max-memory-mb; the value is still exported as " + "CBM_MEM_BUDGET_MB, which codebase-memory-mcp treats as advisory" + ), + ) parser.add_argument("--skip-install", action="store_true", help="do not install codebase-memory-mcp when missing") parser.add_argument("--skip-validate", action="store_true", help="do not validate graph scope after indexing") parser.add_argument("-v", "--verbose", action="store_true", help="log debug messages too") @@ -128,12 +158,22 @@ def main(argv=None): modules = discover_modules(root) _logger.info("mode=%s modules=%d", mode, len(modules)) + limits = resolve_limits( + workers=args.workers, + memory_mb=args.max_memory_mb, + timeout_s=args.index_timeout, + enforce_memory=not args.no_enforce_memory, + ) + for number, selected in enumerate(cumulative_batches(modules, args.batch_size), 1): _logger.info( "=== batch=%d modules_rendered=%d/%d project=%s ===", number, len(selected), len(modules), project ) write_cbmignore(root, render_cbmignore(selected)) - index_repository(root, project) + try: + index_repository(root, project, limits) + except MemoryLimitExceeded as error: + raise SystemExit("Batch %d: %s" % (number, error)) if not args.skip_validate: if not validate_scope(project, modules): diff --git a/src/codebase_memory_deployv/indexer.py b/src/codebase_memory_deployv/indexer.py index 3449564..0f1429c 100644 --- a/src/codebase_memory_deployv/indexer.py +++ b/src/codebase_memory_deployv/indexer.py @@ -18,6 +18,8 @@ import subprocess import urllib.request +from .limits import MB, limits_env, run_capped + _logger = logging.getLogger(__name__) CBM_BIN = "codebase-memory-mcp" @@ -440,11 +442,19 @@ def write_cbmignore(root, content): return path -def index_repository(root, project): - """Run one indexing pass reusing the same project name for every pass.""" - subprocess.check_call( - [find_cbm() or CBM_BIN, "cli", "index_repository", "--repo_path", root, "--name=%s" % project] - ) +def index_repository(root, project, limits=None): + """Run one indexing pass reusing the same project name for every pass. + + Without limits the pass behaves the way it always did: every core the container sees + and as much memory as it wants. Callers that care (the CLI does) hand over a resolved + limits.Limits so the pass stays a background job instead of taking over the machine. + """ + command = [find_cbm() or CBM_BIN, "cli", "index_repository", "--repo_path", root, "--name=%s" % project] + if limits is None: + subprocess.check_call(command) + return + max_rss = limits.memory_mb * MB if limits.enforce_memory and limits.memory_mb else 0 + run_capped(command, env=limits_env(limits), max_rss_bytes=max_rss) def query_graph(project, query): diff --git a/src/codebase_memory_deployv/limits.py b/src/codebase_memory_deployv/limits.py new file mode 100644 index 0000000..e09c05a --- /dev/null +++ b/src/codebase_memory_deployv/limits.py @@ -0,0 +1,383 @@ +"""Bound the CPU, memory and wall time one codebase-memory-mcp indexing pass may take. + +Left alone the indexer sizes itself against the whole machine. Measured inside a Vauxoo +container on a 18 GB MacBook (Docker Desktop VM of 11.9 GiB):: + + codebase-memory-mcp cli --index-worker 428% CPU 9.0 GB RSS (73.9% of the VM) + 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 nothing +below this tool stops it either: the contention has to come from here. + +Three knobs are handed to the child process: + + CBM_WORKERS parsing workers; a quarter of the usable CPUs by default + CBM_INDEX_WORKER_TIMEOUT_S how long one index worker may run; two hours by default + CBM_MEM_BUDGET_MB the indexer's *internal* budget; a quarter of RAM by default + +The third one is not a cap and must not be mistaken for one. It only tells +codebase-memory-mcp when to log ``mem.pressure`` and purge its allocator: the measured 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 environment variable that bounds RSS +(it tracks it, it never enforces it), and the portable OS mechanisms do not fit either: +``RLIMIT_AS``/``RLIMIT_DATA`` count the address space mimalloc reserves without touching, +``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 here, in the parent: the RSS of the child process tree is sampled +while the pass runs and the tree is killed when it crosses the limit. Recovery is the +mechanism this tool already ships and documents — lower ``--batch-size`` and run again, +since the batches are cumulative and everything indexed so far stays in the graph. +""" + +import collections +import errno +import logging +import os +import signal +import subprocess +import time + +_logger = logging.getLogger(__name__) + +# A quarter of the machine each, so the indexer stays a background job on the developer's +# laptop instead of taking it over. Both are overridable on the command line. +DEFAULT_CPU_FRACTION = 0.25 +DEFAULT_MEMORY_FRACTION = 0.25 +# Two hours. The default of codebase-memory-mcp is far shorter than a full Odoo instance +# pass, and a batch killed by the timeout looks exactly like a batch killed by the OOM +# killer, which sends the reader hunting for a memory problem that is not there. +DEFAULT_INDEX_TIMEOUT_S = 7200 +# Below this a cap is not worth enforcing: the indexer cannot even load the Odoo core tree +# and every pass would die on a limit the machine, not the tool, is responsible for. +MIN_MEMORY_MB = 512 +MEMORY_POLL_INTERVAL_S = 5.0 +# Grace given to the process tree to exit on SIGTERM before SIGKILL. The worker flushes its +# sqlite store on the way out; killing it outright leaves the store to be recovered. +KILL_GRACE_S = 10.0 + +WORKERS_ENV = "CBM_WORKERS" +MEMORY_BUDGET_ENV = "CBM_MEM_BUDGET_MB" +TIMEOUT_ENV = "CBM_INDEX_WORKER_TIMEOUT_S" + +MB = 1024 * 1024 +CGROUP_V2_CPU = "/sys/fs/cgroup/cpu.max" +CGROUP_V1_CPU_QUOTA = "/sys/fs/cgroup/cpu/cpu.cfs_quota_us" +CGROUP_V1_CPU_PERIOD = "/sys/fs/cgroup/cpu/cpu.cfs_period_us" +CGROUP_V2_MEMORY = "/sys/fs/cgroup/memory.max" +CGROUP_V1_MEMORY = "/sys/fs/cgroup/memory/memory.limit_in_bytes" +PROC_MEMINFO = "/proc/meminfo" +# cgroup v1 spells "no limit" as a number close to the whole address space; anything at or +# above this is the kernel saying unlimited, not a real 8 EB container. +CGROUP_V1_UNLIMITED = 1 << 62 + +Limits = collections.namedtuple("Limits", "workers memory_mb timeout_s enforce_memory") + + +class MemoryLimitExceeded(RuntimeError): + """Raised when an indexing pass grew past the configured memory cap and was killed.""" + + +def _read_file(path): + """Return the contents of path, or "" when it cannot be read (not Linux, no cgroup).""" + try: + with open(path) as handler: + return handler.read() + except (OSError, ValueError): + return "" + + +def cgroup_cpu_quota(): + """CPUs granted by the cgroup (e.g. 5.0 for "docker update --cpus=5"), or None. + + os.cpu_count() reports the CPUs of the machine, not of the container: a quarter of it + is the wrong number the moment anybody bounds the container from the outside. + """ + raw = _read_file(CGROUP_V2_CPU).split() + if len(raw) == 2 and raw[0] != "max": + try: + quota, period = int(raw[0]), int(raw[1]) + except ValueError: + return None + if quota > 0 and period > 0: + return float(quota) / period + return None + try: + quota = int(_read_file(CGROUP_V1_CPU_QUOTA).strip() or -1) + period = int(_read_file(CGROUP_V1_CPU_PERIOD).strip() or 0) + except ValueError: + return None + if quota > 0 and period > 0: + return float(quota) / period + return None + + +def usable_cpus(): + """CPUs this process may actually use: cgroup quota, else affinity, else the machine.""" + quota = cgroup_cpu_quota() + if quota: + return max(1, int(quota)) + affinity = getattr(os, "sched_getaffinity", None) + if affinity is not None: + try: + return max(1, len(affinity(0))) + except OSError: + pass + return max(1, os.cpu_count() or 1) + + +def cgroup_memory_limit(): + """Memory granted by the cgroup in bytes, or None when the container is unbounded.""" + raw = _read_file(CGROUP_V2_MEMORY).strip() + if raw and raw != "max": + try: + return int(raw) + except ValueError: + return None + raw = _read_file(CGROUP_V1_MEMORY).strip() + if raw: + try: + value = int(raw) + except ValueError: + return None + if 0 < value < CGROUP_V1_UNLIMITED: + return value + return None + + +def total_memory_bytes(): + """Memory the machine offers: cgroup limit when bounded, else the OS total, else 0. + + 0 means "unknown", and every caller treats it as "no default cap can be computed" + rather than guessing a number that would silently kill legitimate passes. + """ + limit = cgroup_memory_limit() + if limit: + return limit + for line in _read_file(PROC_MEMINFO).splitlines(): + if line.startswith("MemTotal:"): + parts = line.split() + if len(parts) >= 2 and parts[1].isdigit(): + return int(parts[1]) * 1024 + try: + pages = os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE") + except (AttributeError, ValueError, OSError): + return 0 + return pages if pages > 0 else 0 + + +def default_workers(fraction=DEFAULT_CPU_FRACTION): + """A quarter of the usable CPUs, never below one.""" + return max(1, int(usable_cpus() * fraction)) + + +def default_memory_mb(fraction=DEFAULT_MEMORY_FRACTION): + """A quarter of the usable memory in MB, or 0 when the total cannot be read.""" + total = total_memory_bytes() + if not total: + return 0 + return max(MIN_MEMORY_MB, int(total * fraction) // MB) + + +def _from_environment(environ, name): + """Positive integer exported as name, or None. An exported value is a deliberate choice.""" + try: + value = int(environ.get(name, "").strip()) + except (AttributeError, ValueError): + return None + return value if value > 0 else None + + +def _resolve(explicit, environ, name, computed): + """Pick a value and say where it came from: command line, environment, then default.""" + if explicit is not None: + return explicit, "argument" + exported = _from_environment(environ, name) + if exported is not None: + return exported, name + return computed, "default" + + +def resolve_limits(workers=None, memory_mb=None, timeout_s=None, enforce_memory=True, environ=None): + """Resolve the three limits, logging each value and its source. + + Precedence is command line, then an already exported CBM_* variable, then a quarter of + the machine. The environment is honoured on purpose: the caller who exported CBM_WORKERS + knows something about that container, and this tool is not the only thing reading it. + """ + environ = os.environ if environ is None else environ + workers, workers_source = _resolve(workers, environ, WORKERS_ENV, default_workers()) + memory_mb, memory_source = _resolve(memory_mb, environ, MEMORY_BUDGET_ENV, default_memory_mb()) + timeout_s, timeout_source = _resolve(timeout_s, environ, TIMEOUT_ENV, DEFAULT_INDEX_TIMEOUT_S) + _logger.info("workers=%d source=%s usable_cpus=%d", workers, workers_source, usable_cpus()) + _logger.info( + "memory_mb=%s source=%s total_ram_mb=%d enforce=%s", + memory_mb or "unknown", + memory_source, + total_memory_bytes() // MB, + "yes" if enforce_memory and memory_mb else "no", + ) + _logger.info("index_worker_timeout_s=%d source=%s", timeout_s, timeout_source) + if enforce_memory and not memory_mb: + _logger.warning("memory_cap=none (total memory unreadable); the pass runs unbounded") + return Limits(workers=workers, memory_mb=memory_mb, timeout_s=timeout_s, enforce_memory=enforce_memory) + + +def limits_env(limits, environ=None): + """A copy of environ carrying the resolved limits, ready for subprocess.""" + env = dict(os.environ if environ is None else environ) + env[WORKERS_ENV] = str(limits.workers) + env[TIMEOUT_ENV] = str(limits.timeout_s) + if limits.memory_mb: + env[MEMORY_BUDGET_ENV] = str(limits.memory_mb) + return env + + +def _linux_process_table(): + """{pid: (ppid, rss_bytes)} read from /proc, or {} when /proc is not there.""" + page_size = os.sysconf("SC_PAGE_SIZE") if hasattr(os, "sysconf") else 4096 + table = {} + try: + entries = os.listdir("/proc") + except OSError: + return table + for entry in entries: + if not entry.isdigit(): + continue + stat = _read_file("/proc/%s/stat" % entry) + # The command name sits in parentheses and may contain spaces: everything before + # the last ")" is pid + comm, so ppid is the second field of what follows. + _, _, rest = stat.rpartition(")") + fields = rest.split() + if len(fields) < 2 or not fields[1].lstrip("-").isdigit(): + continue + statm = _read_file("/proc/%s/statm" % entry).split() + if len(statm) < 2 or not statm[1].isdigit(): + continue + table[int(entry)] = (int(fields[1]), int(statm[1]) * page_size) + return table + + +def _ps_process_table(): + """{pid: (ppid, rss_bytes)} from ps, for the platforms without /proc (macOS).""" + try: + raw = subprocess.check_output(["ps", "-Ao", "pid=,ppid=,rss="], universal_newlines=True) + except (OSError, subprocess.CalledProcessError): + return {} + table = {} + for line in raw.splitlines(): + fields = line.split() + if len(fields) < 3 or not all(field.lstrip("-").isdigit() for field in fields[:3]): + continue + table[int(fields[0])] = (int(fields[1]), int(fields[2]) * 1024) + return table + + +def process_table(): + """{pid: (ppid, rss_bytes)} for every process on the machine.""" + return _linux_process_table() if os.path.isdir("/proc") else _ps_process_table() + + +def process_tree(pid, table): + """Return pid plus every descendant of it present in table. + + The whole tree matters, not just the process this tool started: "codebase-memory-mcp + cli index_repository" forks the ``--index-worker`` child that does the parsing, and + that child is the one that reached 9.0 GB. + """ + children = {} + for child, (parent, _rss) in table.items(): + children.setdefault(parent, []).append(child) + found = [] + pending = [pid] + seen = set() + while pending: + current = pending.pop() + if current in seen or current not in table: + continue + seen.add(current) + found.append(current) + pending.extend(children.get(current, ())) + return found + + +def tree_rss_bytes(pid, table=None): + """RSS of pid and its descendants, in bytes.""" + table = process_table() if table is None else table + return sum(table[member][1] for member in process_tree(pid, table)) + + +def _signal_tree(members, sig): + """Send sig to every member still alive, ignoring the ones that already left.""" + for member in members: + try: + os.kill(member, sig) + except OSError as error: + if error.errno != errno.ESRCH: + raise + + +def _kill_tree(process, members): + """SIGTERM the process tree, then SIGKILL whatever is still alive after the grace. + + The members are signalled one by one instead of through the process group so the pass + keeps sharing the terminal group: starting it in a session of its own would stop Ctrl-C + from reaching an indexing run that takes hours. + + Reaping goes through the Popen object only. A bare os.waitpid() here would steal the + status Popen is waiting for and turn its next wait() into ECHILD. + """ + _signal_tree(members, signal.SIGTERM) + deadline = time.time() + KILL_GRACE_S + while time.time() < deadline: + if process.poll() is not None: + break + time.sleep(0.1) + if process.poll() is None: + _signal_tree(members, signal.SIGKILL) + process.wait() + # Descendants outlive the parent they were forked from, so sweep them once more. + _signal_tree([member for member in members if member != process.pid], signal.SIGKILL) + + +def run_capped(cmd, env=None, max_rss_bytes=0, poll_interval=MEMORY_POLL_INTERVAL_S): + """Run cmd, killing its whole process tree if the tree grows past max_rss_bytes. + + Returns the peak RSS observed, in bytes. A non-positive max_rss_bytes only measures. + Raises MemoryLimitExceeded when the cap is crossed and CalledProcessError on a plain + non-zero exit, so the caller can tell "too big" apart from "it failed". + """ + process = subprocess.Popen(cmd, env=env) + peak = 0 + try: + while True: + table = process_table() + members = process_tree(process.pid, table) + rss = sum(table[member][1] for member in members) + peak = max(peak, rss) + if max_rss_bytes > 0 and rss > max_rss_bytes: + _logger.error( + "memory_cap_exceeded rss_mb=%d limit_mb=%d processes=%d; killing the pass", + rss // MB, + max_rss_bytes // MB, + len(members), + ) + _kill_tree(process, members) + raise MemoryLimitExceeded( + "the indexing pass reached %d MB, over the %d MB cap. Lower --batch-size so each " + "pass parses fewer modules, or raise --max-memory-mb when the machine can afford it" + % (rss // MB, max_rss_bytes // MB) + ) + if process.poll() is not None: + break + time.sleep(poll_interval) + finally: + if process.poll() is None: + # Ctrl-C or any other exception on the way out: take the descendants down too, + # otherwise the index worker keeps eating the machine with nobody watching it. + _kill_tree(process, process_tree(process.pid, process_table())) + _logger.info("peak_rss_mb=%d limit_mb=%s", peak // MB, max_rss_bytes // MB if max_rss_bytes > 0 else "none") + if process.returncode: + raise subprocess.CalledProcessError(process.returncode, cmd) + return peak diff --git a/tests/test_cli.py b/tests/test_cli.py index 02123c0..95c609e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,6 +4,7 @@ from codebase_memory_deployv import cli from codebase_memory_deployv.indexer import DEFAULT_BATCH_SIZE, DEFAULT_ROOT +from codebase_memory_deployv.limits import MemoryLimitExceeded @pytest.fixture(autouse=True) @@ -56,7 +57,7 @@ def test_main_modules_file_wins_over_the_database(tmp_path, monkeypatch, capsys) modules_file.write_text("name,state\nsale_extended,installed\nnever_installed_here,installed\nweb,installed\n") indexed = [] monkeypatch.setattr(cli, "ensure_cbm_installed", lambda: "cbm") - monkeypatch.setattr(cli, "index_repository", lambda root_, project: indexed.append(project)) + monkeypatch.setattr(cli, "index_repository", lambda root_, project, limits_: indexed.append(project)) monkeypatch.setattr(cli, "installed_modules", lambda root_: pytest.fail("the database must not be queried")) monkeypatch.setattr(cli, "validate_scope", lambda project, modules: True) monkeypatch.setenv("CBM_PROJECT", "prod_18.0") @@ -75,7 +76,7 @@ def test_main_modules_file_ignored_with_mode_all(tmp_path, monkeypatch, capsys): modules_file = tmp_path / "modules.csv" modules_file.write_text("name\nsale_extended\n") monkeypatch.setattr(cli, "ensure_cbm_installed", lambda: "cbm") - monkeypatch.setattr(cli, "index_repository", lambda root_, project: None) + monkeypatch.setattr(cli, "index_repository", lambda root_, project, limits_: None) monkeypatch.setenv("CBM_PROJECT", "prod_18.0") assert cli.main(["--repo-path", root, "--mode=all", "--modules-file", str(modules_file), "--skip-validate"]) == 0 assert "modules_file=ignored" in capsys.readouterr().out @@ -92,3 +93,48 @@ def test_main_modules_file_without_modules_on_disk(tmp_path, monkeypatch): assert "does not list any module" in str(error) else: raise AssertionError("main must fail when the modules file matches nothing on disk") + + +def test_parser_resource_defaults_are_resolved_at_runtime(): + """The flags default to None so the environment still has a say; limits resolve them.""" + args = cli.build_parser().parse_args([]) + assert args.workers is None + assert args.max_memory_mb is None + assert args.index_timeout is None + assert not args.no_enforce_memory + + +def test_parser_accepts_the_resource_flags(): + args = cli.build_parser().parse_args(["--workers=2", "--max-memory-mb=1500", "--index-timeout=600"]) + assert (args.workers, args.max_memory_mb, args.index_timeout) == (2, 1500, 600) + + +def test_main_passes_the_resolved_limits_to_every_pass(tmp_path, monkeypatch): + root = _instance(tmp_path, "extra_addons/vauxoo/sale_extended") + seen = [] + monkeypatch.setattr(cli, "ensure_cbm_installed", lambda: "cbm") + monkeypatch.setattr(cli, "index_repository", lambda root_, project, limits_: seen.append(limits_)) + monkeypatch.setattr(cli, "installed_modules", lambda root_: None) + assert cli.main(["--repo-path", root, "--workers=2", "--max-memory-mb=1500", "--skip-validate"]) == 0 + assert [(one.workers, one.memory_mb, one.enforce_memory) for one in seen] == [(2, 1500, True)] + + +def test_main_reports_a_pass_killed_by_the_memory_cap(tmp_path, monkeypatch): + """A killed pass must name its batch and the way out, not just die.""" + root = _instance(tmp_path, "extra_addons/vauxoo/sale_extended") + + def boom(root_, project, limits_): + raise MemoryLimitExceeded("the indexing pass reached 3000 MB, over the 1500 MB cap. Lower --batch-size") + + monkeypatch.setattr(cli, "ensure_cbm_installed", lambda: "cbm") + monkeypatch.setattr(cli, "index_repository", boom) + monkeypatch.setattr(cli, "installed_modules", lambda root_: None) + with pytest.raises(SystemExit) as error: + cli.main(["--repo-path", root, "--max-memory-mb=1500", "--skip-validate"]) + assert "Batch 1" in str(error.value) + assert "--batch-size" in str(error.value) + + +def test_help_renders(): + """argparse re-expands "%" in a help string: a literal one there breaks --help only.""" + assert "--max-memory-mb" in cli.build_parser().format_help() diff --git a/tests/test_indexer.py b/tests/test_indexer.py index 82f88dc..b23a5a0 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -6,7 +6,7 @@ import pytest -from codebase_memory_deployv import indexer +from codebase_memory_deployv import indexer, limits def test_cumulative_batches_are_cumulative(): @@ -480,3 +480,46 @@ def test_check_layout_requires_odoo_bin(tmp_path): odoo_dir.mkdir() (odoo_dir / "odoo-bin").write_text("") assert indexer.check_layout(str(tmp_path)) == os.path.join(str(tmp_path), "odoo", "odoo-bin") + + +def test_index_repository_without_limits_keeps_the_old_behaviour(monkeypatch): + calls = [] + monkeypatch.setattr(indexer, "find_cbm", lambda: "/usr/bin/cbm") + monkeypatch.setattr(indexer.subprocess, "check_call", lambda cmd: calls.append(cmd)) + indexer.index_repository("/home/odoo/instance", "vauxoo_12.0") + assert calls == [ + ["/usr/bin/cbm", "cli", "index_repository", "--repo_path", "/home/odoo/instance", "--name=vauxoo_12.0"] + ] + + +def test_index_repository_caps_the_pass_when_given_limits(monkeypatch): + """The env carries the three CBM_* knobs and the watchdog gets the cap in bytes.""" + captured = {} + + def fake_run_capped(cmd, env=None, max_rss_bytes=0): + captured.update(cmd=cmd, env=env, max_rss_bytes=max_rss_bytes) + + monkeypatch.setattr(indexer, "find_cbm", lambda: "/usr/bin/cbm") + monkeypatch.setattr(indexer, "run_capped", fake_run_capped) + monkeypatch.setattr(indexer.subprocess, "check_call", lambda cmd: pytest.fail("must not bypass the cap")) + resolved = limits.Limits(workers=3, memory_mb=1500, timeout_s=7200, enforce_memory=True) + indexer.index_repository("/home/odoo/instance", "vauxoo_12.0", resolved) + assert captured["max_rss_bytes"] == 1500 * limits.MB + assert captured["env"]["CBM_WORKERS"] == "3" + assert captured["env"]["CBM_MEM_BUDGET_MB"] == "1500" + assert captured["env"]["CBM_INDEX_WORKER_TIMEOUT_S"] == "7200" + + +def test_index_repository_only_advises_when_enforcement_is_off(monkeypatch): + """--no-enforce-memory still exports the budget; it just stops the watchdog killing.""" + captured = {} + + def fake_run_capped(cmd, env=None, max_rss_bytes=0): + captured.update(env=env, max_rss_bytes=max_rss_bytes) + + monkeypatch.setattr(indexer, "find_cbm", lambda: "/usr/bin/cbm") + monkeypatch.setattr(indexer, "run_capped", fake_run_capped) + resolved = limits.Limits(workers=3, memory_mb=1500, timeout_s=7200, enforce_memory=False) + indexer.index_repository("/home/odoo/instance", "vauxoo_12.0", resolved) + assert captured["max_rss_bytes"] == 0 + assert captured["env"]["CBM_MEM_BUDGET_MB"] == "1500" diff --git a/tests/test_limits.py b/tests/test_limits.py new file mode 100644 index 0000000..35ca831 --- /dev/null +++ b/tests/test_limits.py @@ -0,0 +1,199 @@ +import os +import subprocess +import sys +import time + +import pytest + +from codebase_memory_deployv import limits + + +def test_default_workers_is_a_quarter_of_the_cpus(monkeypatch): + monkeypatch.setattr(limits, "usable_cpus", lambda: 12) + assert limits.default_workers() == 3 + + +def test_default_workers_never_reaches_zero(monkeypatch): + """A machine with one or two cores still has to index; it just does it with one worker.""" + monkeypatch.setattr(limits, "usable_cpus", lambda: 2) + assert limits.default_workers() == 1 + + +def test_default_memory_is_a_quarter_of_the_ram(monkeypatch): + monkeypatch.setattr(limits, "total_memory_bytes", lambda: 11946 * limits.MB) + assert limits.default_memory_mb() == 2986 + + +def test_default_memory_has_a_floor(monkeypatch): + monkeypatch.setattr(limits, "total_memory_bytes", lambda: 1024 * limits.MB) + assert limits.default_memory_mb() == limits.MIN_MEMORY_MB + + +def test_default_memory_unknown_total(monkeypatch): + monkeypatch.setattr(limits, "total_memory_bytes", lambda: 0) + assert limits.default_memory_mb() == 0 + + +def test_cgroup_cpu_quota_v2(tmp_path, monkeypatch): + """docker update --cpus=5 must be seen; os.cpu_count() would keep reporting the host.""" + quota = tmp_path / "cpu.max" + quota.write_text("500000 100000\n") + monkeypatch.setattr(limits, "CGROUP_V2_CPU", str(quota)) + assert limits.cgroup_cpu_quota() == 5.0 + assert limits.usable_cpus() == 5 + + +def test_cgroup_cpu_quota_v2_unlimited(tmp_path, monkeypatch): + quota = tmp_path / "cpu.max" + quota.write_text("max 100000\n") + monkeypatch.setattr(limits, "CGROUP_V2_CPU", str(quota)) + monkeypatch.setattr(limits, "CGROUP_V1_CPU_QUOTA", str(tmp_path / "missing")) + assert limits.cgroup_cpu_quota() is None + + +def test_cgroup_cpu_quota_v1(tmp_path, monkeypatch): + (tmp_path / "cpu.cfs_quota_us").write_text("200000\n") + (tmp_path / "cpu.cfs_period_us").write_text("100000\n") + monkeypatch.setattr(limits, "CGROUP_V2_CPU", str(tmp_path / "missing")) + monkeypatch.setattr(limits, "CGROUP_V1_CPU_QUOTA", str(tmp_path / "cpu.cfs_quota_us")) + monkeypatch.setattr(limits, "CGROUP_V1_CPU_PERIOD", str(tmp_path / "cpu.cfs_period_us")) + assert limits.cgroup_cpu_quota() == 2.0 + + +def test_cgroup_memory_limit_v2(tmp_path, monkeypatch): + limit = tmp_path / "memory.max" + limit.write_text("%d\n" % (4 * limits.MB)) + monkeypatch.setattr(limits, "CGROUP_V2_MEMORY", str(limit)) + assert limits.cgroup_memory_limit() == 4 * limits.MB + + +def test_cgroup_memory_limit_v1_unlimited_is_not_a_limit(tmp_path, monkeypatch): + """cgroup v1 spells "no limit" as a huge number; taking a quarter of it caps nothing.""" + limit = tmp_path / "memory.limit_in_bytes" + limit.write_text("9223372036854771712\n") + monkeypatch.setattr(limits, "CGROUP_V2_MEMORY", str(tmp_path / "missing")) + monkeypatch.setattr(limits, "CGROUP_V1_MEMORY", str(limit)) + assert limits.cgroup_memory_limit() is None + + +def test_total_memory_prefers_the_cgroup_over_the_host(monkeypatch, tmp_path): + meminfo = tmp_path / "meminfo" + meminfo.write_text("MemTotal: 12234752 kB\n") + monkeypatch.setattr(limits, "PROC_MEMINFO", str(meminfo)) + monkeypatch.setattr(limits, "cgroup_memory_limit", lambda: 2048 * limits.MB) + assert limits.total_memory_bytes() == 2048 * limits.MB + monkeypatch.setattr(limits, "cgroup_memory_limit", lambda: None) + assert limits.total_memory_bytes() == 12234752 * 1024 + + +def test_resolve_limits_defaults(monkeypatch): + monkeypatch.setattr(limits, "usable_cpus", lambda: 8) + monkeypatch.setattr(limits, "total_memory_bytes", lambda: 8192 * limits.MB) + resolved = limits.resolve_limits(environ={}) + assert resolved == limits.Limits(workers=2, memory_mb=2048, timeout_s=7200, enforce_memory=True) + + +def test_resolve_limits_arguments_win_over_the_environment(monkeypatch): + monkeypatch.setattr(limits, "usable_cpus", lambda: 8) + monkeypatch.setattr(limits, "total_memory_bytes", lambda: 8192 * limits.MB) + environ = {"CBM_WORKERS": "7", "CBM_MEM_BUDGET_MB": "9000", "CBM_INDEX_WORKER_TIMEOUT_S": "60"} + resolved = limits.resolve_limits(workers=1, memory_mb=500, timeout_s=30, environ=environ) + assert resolved == limits.Limits(workers=1, memory_mb=500, timeout_s=30, enforce_memory=True) + + +def test_resolve_limits_environment_wins_over_the_default(monkeypatch): + """Whoever exported CBM_WORKERS knows something about that container; do not overrule it.""" + monkeypatch.setattr(limits, "usable_cpus", lambda: 8) + monkeypatch.setattr(limits, "total_memory_bytes", lambda: 8192 * limits.MB) + environ = {"CBM_WORKERS": "7", "CBM_MEM_BUDGET_MB": "9000", "CBM_INDEX_WORKER_TIMEOUT_S": "60"} + resolved = limits.resolve_limits(environ=environ) + assert resolved == limits.Limits(workers=7, memory_mb=9000, timeout_s=60, enforce_memory=True) + + +@pytest.mark.parametrize("value", ["", "0", "-4", "many", " "]) +def test_resolve_limits_ignores_unusable_environment_values(monkeypatch, value): + monkeypatch.setattr(limits, "usable_cpus", lambda: 8) + monkeypatch.setattr(limits, "total_memory_bytes", lambda: 8192 * limits.MB) + resolved = limits.resolve_limits(environ={"CBM_WORKERS": value}) + assert resolved.workers == 2 + + +def test_limits_env_exports_the_three_variables(): + resolved = limits.Limits(workers=3, memory_mb=2986, timeout_s=7200, enforce_memory=True) + env = limits.limits_env(resolved, environ={"PATH": "/usr/bin"}) + assert env["CBM_WORKERS"] == "3" + assert env["CBM_MEM_BUDGET_MB"] == "2986" + assert env["CBM_INDEX_WORKER_TIMEOUT_S"] == "7200" + assert env["PATH"] == "/usr/bin" + + +def test_limits_env_omits_an_unknown_memory_budget(): + """Exporting CBM_MEM_BUDGET_MB=0 would be read as invalid; leave the default in place.""" + resolved = limits.Limits(workers=1, memory_mb=0, timeout_s=7200, enforce_memory=True) + assert "CBM_MEM_BUDGET_MB" not in limits.limits_env(resolved, environ={}) + + +def test_process_tree_collects_the_descendants(): + # 10 -> 20 -> 30, plus an unrelated 40 + table = {10: (1, 100), 20: (10, 200), 30: (20, 400), 40: (1, 800)} + assert sorted(limits.process_tree(10, table)) == [10, 20, 30] + assert limits.tree_rss_bytes(10, table) == 700 + + +def test_process_tree_survives_a_cycle(): + """A pid table read while processes come and go can look self-parented; do not hang.""" + table = {10: (10, 100)} + assert limits.process_tree(10, table) == [10] + + +def test_process_table_sees_this_process(): + table = limits.process_table() + assert os.getpid() in table + assert table[os.getpid()][1] > 0 + + +def test_run_capped_returns_the_peak_and_the_exit_status(): + peak = limits.run_capped([sys.executable, "-c", "import time; time.sleep(0.5)"], poll_interval=0.01) + assert peak > 0 + with pytest.raises(subprocess.CalledProcessError): + limits.run_capped([sys.executable, "-c", "raise SystemExit(3)"], poll_interval=0.01) + + +def test_run_capped_reports_zero_for_a_pass_shorter_than_one_sample(): + """Sampling cannot see a process that already exited; that is a 0, never a failure.""" + assert limits.run_capped([sys.executable, "-c", "pass"], poll_interval=0.01) >= 0 + + +def test_run_capped_kills_a_process_over_the_cap(): + """The whole point: the pass dies instead of the machine. + + CBM_MEM_BUDGET_MB cannot do this — the measured worker reported budget_mb=2986 and + still grew to 9.0 GB — so the cap has to be enforced from the parent. + """ + grow = "buffer = bytearray()\nwhile True:\n buffer += bytearray(8 * 1024 * 1024)\n" + with pytest.raises(limits.MemoryLimitExceeded) as error: + limits.run_capped([sys.executable, "-c", grow], max_rss_bytes=64 * limits.MB, poll_interval=0.01) + assert "--batch-size" in str(error.value) + + +def test_run_capped_kills_the_descendants_too(tmp_path): + """The parsing happens in the "--index-worker" child, so killing only the parent leaks it.""" + marker = tmp_path / "child.pid" + # The grandchild waits before allocating so the pid file is always on disk by the time + # the cap is crossed; otherwise the assertion races the watchdog. + grow = "import time\ntime.sleep(1)\nbuffer = bytearray()\nwhile True:\n buffer += bytearray(8 * 1024 * 1024)" + script = ( + "import subprocess, sys, time\n" + "child = subprocess.Popen([sys.executable, '-c', %r])\n" + "open(%r, 'w').write(str(child.pid))\n" + "time.sleep(120)\n" % (grow, str(marker)) + ) + with pytest.raises(limits.MemoryLimitExceeded): + limits.run_capped([sys.executable, "-c", script], max_rss_bytes=128 * limits.MB, poll_interval=0.05) + child_pid = int(marker.read_text()) + # Gone, or a zombie nobody reaped yet: either way it stopped holding memory. Asking for + # the pid to disappear would depend on whether PID 1 of the container reaps orphans. + deadline = time.time() + 15 + while time.time() < deadline and limits.tree_rss_bytes(child_pid): + time.sleep(0.1) + assert limits.tree_rss_bytes(child_pid) == 0 From fa5e3f4b3ac6f6b5f7b4503199d0dfe6d875f52c Mon Sep 17 00:00:00 2001 From: "Moises Lopez - https://www.vauxoo.com/" Date: Tue, 1 Sep 2026 12:53:41 -0600 Subject: [PATCH 2/5] [FIX] limits: degrade the memory cap gracefully where no process table 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. --- src/codebase_memory_deployv/limits.py | 22 ++++++++++++++++++++-- tests/test_limits.py | 18 ++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/codebase_memory_deployv/limits.py b/src/codebase_memory_deployv/limits.py index e09c05a..c75573b 100644 --- a/src/codebase_memory_deployv/limits.py +++ b/src/codebase_memory_deployv/limits.py @@ -308,6 +308,11 @@ def tree_rss_bytes(pid, table=None): return sum(table[member][1] for member in process_tree(pid, table)) +# Windows has no SIGKILL; there os.kill(SIGTERM) already calls TerminateProcess, so the +# escalation collapses into a single stage instead of failing with an AttributeError. +SIGKILL = getattr(signal, "SIGKILL", signal.SIGTERM) + + def _signal_tree(members, sig): """Send sig to every member still alive, ignoring the ones that already left.""" for member in members: @@ -335,10 +340,20 @@ def _kill_tree(process, members): break time.sleep(0.1) if process.poll() is None: - _signal_tree(members, signal.SIGKILL) + _signal_tree(members, SIGKILL) process.wait() # Descendants outlive the parent they were forked from, so sweep them once more. - _signal_tree([member for member in members if member != process.pid], signal.SIGKILL) + _signal_tree([member for member in members if member != process.pid], SIGKILL) + + +def can_measure_rss(): + """True when this platform exposes a process table (Linux /proc, or ps elsewhere). + + Only Linux containers ever run an indexing pass, but the package is imported and tested + on macOS and Windows too, and a cap that cannot be measured must say so rather than + look enforced. + """ + return bool(process_table()) def run_capped(cmd, env=None, max_rss_bytes=0, poll_interval=MEMORY_POLL_INTERVAL_S): @@ -348,6 +363,9 @@ def run_capped(cmd, env=None, max_rss_bytes=0, poll_interval=MEMORY_POLL_INTERVA Raises MemoryLimitExceeded when the cap is crossed and CalledProcessError on a plain non-zero exit, so the caller can tell "too big" apart from "it failed". """ + if max_rss_bytes > 0 and not can_measure_rss(): + _logger.warning("memory_cap=unenforceable (no readable process table on this platform)") + max_rss_bytes = 0 process = subprocess.Popen(cmd, env=env) peak = 0 try: diff --git a/tests/test_limits.py b/tests/test_limits.py index 35ca831..5922a46 100644 --- a/tests/test_limits.py +++ b/tests/test_limits.py @@ -7,6 +7,12 @@ from codebase_memory_deployv import limits +# Only Linux containers ever run an indexing pass, but the suite also runs on macOS and +# Windows: there the process table is unreadable and the cap degrades to advisory. +needs_process_table = pytest.mark.skipif( + not limits.can_measure_rss(), reason="no readable process table on this platform" +) + def test_default_workers_is_a_quarter_of_the_cpus(monkeypatch): monkeypatch.setattr(limits, "usable_cpus", lambda: 12) @@ -146,12 +152,14 @@ def test_process_tree_survives_a_cycle(): assert limits.process_tree(10, table) == [10] +@needs_process_table def test_process_table_sees_this_process(): table = limits.process_table() assert os.getpid() in table assert table[os.getpid()][1] > 0 +@needs_process_table def test_run_capped_returns_the_peak_and_the_exit_status(): peak = limits.run_capped([sys.executable, "-c", "import time; time.sleep(0.5)"], poll_interval=0.01) assert peak > 0 @@ -164,6 +172,7 @@ def test_run_capped_reports_zero_for_a_pass_shorter_than_one_sample(): assert limits.run_capped([sys.executable, "-c", "pass"], poll_interval=0.01) >= 0 +@needs_process_table def test_run_capped_kills_a_process_over_the_cap(): """The whole point: the pass dies instead of the machine. @@ -176,6 +185,7 @@ def test_run_capped_kills_a_process_over_the_cap(): assert "--batch-size" in str(error.value) +@needs_process_table def test_run_capped_kills_the_descendants_too(tmp_path): """The parsing happens in the "--index-worker" child, so killing only the parent leaks it.""" marker = tmp_path / "child.pid" @@ -197,3 +207,11 @@ def test_run_capped_kills_the_descendants_too(tmp_path): while time.time() < deadline and limits.tree_rss_bytes(child_pid): time.sleep(0.1) assert limits.tree_rss_bytes(child_pid) == 0 + + +def test_run_capped_does_not_pretend_to_enforce_without_a_process_table(monkeypatch, caplog): + """A cap that cannot be measured must say so, not silently look enforced.""" + monkeypatch.setattr(limits, "process_table", dict) + with caplog.at_level("WARNING"): + assert limits.run_capped([sys.executable, "-c", "pass"], max_rss_bytes=1, poll_interval=0.01) == 0 + assert "memory_cap=unenforceable" in caplog.text From 0939849b1dfcf68cd21c063a7f9106add10136c1 Mon Sep 17 00:00:00 2001 From: "Moises Lopez - https://www.vauxoo.com/" Date: Tue, 1 Sep 2026 16:39:21 -0600 Subject: [PATCH 3/5] [REF] limits: only resolve the worker count, drop the RSS watchdog 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. --- README.md | 91 +++--- src/codebase_memory_deployv/cli.py | 32 +-- src/codebase_memory_deployv/indexer.py | 14 +- src/codebase_memory_deployv/limits.py | 371 ++++--------------------- tests/test_cli.py | 29 +- tests/test_indexer.py | 51 ++-- tests/test_limits.py | 178 ++---------- 7 files changed, 161 insertions(+), 605 deletions(-) diff --git a/README.md b/README.md index 0824490..c6b7c88 100644 --- a/README.md +++ b/README.md @@ -55,8 +55,8 @@ That single command: 4. Renders `.cbmignore` (Odoo core `odoo/odoo` complete + selected modules, dropping `static/lib`, `static/tests`, minified JS, caches and `.git`) in **cumulative batches of 25 modules**, running `codebase-memory-mcp cli index_repository` after each batch so memory-killed - one-shot indexing is never a problem. Every pass is bounded to a quarter of the machine - (see [Resource limits](#resource-limits)) so indexing stays a background job. + one-shot indexing is never a problem. Every pass runs with a quarter of the CPUs the + container may use (see [Keeping the indexer off the whole machine](#keeping-the-indexer-off-the-whole-machine)). 5. Validates the graph scope: no `test_*` addon modules indexed, no missing and no extra module roots compared to the expected module list, and **every indexed file carrying one of the configured extensions** (`SOURCE_GLOBS`: `.py .xml .js .rst .md .css .scss .csv`) — anything @@ -86,71 +86,72 @@ That single command: --mode MODE auto (default), installed, or all --modules-file PATH ir.module.module export listing the modules to index --workers N indexing workers; default is a quarter of the usable CPUs ---max-memory-mb N memory one pass may use; default is a quarter of the usable memory ---index-timeout N seconds one index worker may run (default: 7200) ---no-enforce-memory do not kill a pass that grows past --max-memory-mb +--index-timeout N CBM_INDEX_WORKER_TIMEOUT_S; a no-progress window, not a time budget --skip-install do not install codebase-memory-mcp when missing --skip-validate do not validate graph scope after indexing ``` -## Resource limits +## Keeping the indexer off the whole machine -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): +Left alone the indexer takes every core it can see. Measured inside a Vauxoo container on an +18 GB MacBook (Docker Desktop VM of 11.9 GiB): ```text -codebase-memory-mcp cli --index-worker 428% CPU 9.0 GB RSS (73.9% of the VM) +codebase-memory-mcp cli --index-worker 428% CPU 9.0 GB RSS 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 this tool. Every pass now runs with a quarter of the machine: +### CPU: the one thing this tool has to resolve + +That 428% is deliberate on the indexer's side — `cbm_default_worker_count(initial=true)` +returns `total_cores`, commented *"Use all cores for initial indexing — user is waiting"*. +Inside a container that count comes from `sysconf(_SC_NPROCESSORS_ONLN)`, which reports the +**host** CPUs and not the cgroup quota. codebase-memory-mcp says so itself, right where it reads +the override: ```text -workers=1 source=default usable_cpus=5 -memory_mb=2816 source=default total_ram_mb=11264 enforce=yes -index_worker_timeout_s=7200 source=default +CBM_WORKERS env override (clamped to [1, CBM_WORKERS_MAX]). +Useful inside containers where sysconf(_SC_NPROCESSORS_ONLN) +reports host CPUs rather than the cgroup's effective CPU quota. ``` -| Limit | Default | Handed to the child as | -|---|---|---| -| Workers | a quarter of the usable CPUs, never below 1 | `CBM_WORKERS` | -| Memory | a quarter of the usable memory, never below 512 MB | `CBM_MEM_BUDGET_MB` **and the enforced cap** | -| Index worker timeout | 7200 s (two hours) | `CBM_INDEX_WORKER_TIMEOUT_S` | +So resolving that quota is what it delegates to its caller, and it is all this tool does: every +pass runs with a quarter of the CPUs the **container** may use — the cgroup v1/v2 quota +(`docker update --cpus=5` reads back as 5, where `os.cpu_count()` keeps reporting 10), falling +back to the affinity mask. The value and its source are logged: -Each value is resolved from the command line first, then from an already exported `CBM_*` -variable, then from the machine — so exporting `CBM_WORKERS` in a container still works and the -resolved value and its source are logged. +```text +workers=1 source=default usable_cpus=5 +``` -"Usable" means 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()` would keep reporting 10) and -the cgroup memory limit, falling back to the CPU affinity mask and `/proc/meminfo`. +`--workers N` overrides it, and an already exported `CBM_WORKERS` is honoured before the default. -### Why the memory cap is enforced here +### Memory: already handled, and the real ceiling is the kernel's -`CBM_MEM_BUDGET_MB` is **not** a cap. It only tells codebase-memory-mcp when to log -`mem.pressure` and purge its allocator: the run measured 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 ever -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. +Nothing here touches the memory budget, on purpose. codebase-memory-mcp already sizes it as a +fraction of RAM that scales with the machine (`mem.c`: 25% at or below 16 GB, 35% at or below +32 GB, 50% above). On the 11.9 GiB VM that is 2986 MB — the same number any "a quarter of RAM" +rule would produce, so exporting `CBM_MEM_BUDGET_MB` there changes nothing. -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 -this tool started would miss it) and the tree is killed when it crosses the limit: +That budget is **not a ceiling** either: it drives `mem.pressure` logging and allocator purges, +which is why the measured worker reported `mem.init budget_mb=2986 source=ram_fraction` and +still reached 9.0 GB RSS. A real ceiling belongs to the kernel, so bound the container: -```text -memory_cap_exceeded rss_mb=3012 limit_mb=2816 processes=2; killing the pass -Batch 3: the indexing pass reached 3012 MB, over the 2816 MB cap. Lower --batch-size so each -pass parses fewer modules, or raise --max-memory-mb when the machine can afford it +```bash +docker run --memory=3g --cpus=2 ... # or: docker update --memory=3g --cpus=2 ``` -Recovery is the mechanism this tool already ships: the batches are cumulative, so everything -indexed so far stays in the graph and a smaller `--batch-size` picks up from there. Use -`--no-enforce-memory` to go back to measuring only — the budget is still exported, it just stops -being a cap. +The cgroup enforces that for everything inside the container, with none of the overshoot a +userspace sampler has — and `--cpus` is then picked up automatically by the worker count above. +Export `CBM_MEM_BUDGET_MB` yourself if the indexer's own fraction needs adjusting. + +### Timeout + +`--index-timeout N` sets `CBM_INDEX_WORKER_TIMEOUT_S`, which is a **no-progress** window, not a +time budget: codebase-memory-mcp kills a worker that logs nothing for 15 minutes, and every +progress line resets the clock, so a long pass that keeps working is never killed by it. Raising +it does not buy a slow pass more time — it only makes the hang detector slower to fire. Left +unset by default for that reason. ## Indexing the modules installed in production diff --git a/src/codebase_memory_deployv/cli.py b/src/codebase_memory_deployv/cli.py index 20b21ff..d81434c 100644 --- a/src/codebase_memory_deployv/cli.py +++ b/src/codebase_memory_deployv/cli.py @@ -22,7 +22,7 @@ validate_scope, write_cbmignore, ) -from .limits import DEFAULT_INDEX_TIMEOUT_S, MemoryLimitExceeded, resolve_limits +from .limits import resolve_limits _logger = logging.getLogger(__name__) PACKAGE_LOGGER = __name__.rsplit(".", 1)[0] @@ -84,27 +84,13 @@ def build_parser(): default=None, help="indexing workers (CBM_WORKERS); default is a quarter of the usable CPUs", ) - parser.add_argument( - "--max-memory-mb", - type=int, - default=None, - help=( - "memory one indexing pass may use; exported as CBM_MEM_BUDGET_MB and enforced on the " - "process tree. Default is a quarter of the usable memory" - ), - ) parser.add_argument( "--index-timeout", type=int, default=None, - help="seconds one index worker may run (CBM_INDEX_WORKER_TIMEOUT_S) (default: %d)" % DEFAULT_INDEX_TIMEOUT_S, - ) - parser.add_argument( - "--no-enforce-memory", - action="store_true", help=( - "do not kill a pass that grows past --max-memory-mb; the value is still exported as " - "CBM_MEM_BUDGET_MB, which codebase-memory-mcp treats as advisory" + "CBM_INDEX_WORKER_TIMEOUT_S: seconds an index worker may log nothing before " + "codebase-memory-mcp treats it as hung. Not a time budget; unset by default" ), ) parser.add_argument("--skip-install", action="store_true", help="do not install codebase-memory-mcp when missing") @@ -158,22 +144,14 @@ def main(argv=None): modules = discover_modules(root) _logger.info("mode=%s modules=%d", mode, len(modules)) - limits = resolve_limits( - workers=args.workers, - memory_mb=args.max_memory_mb, - timeout_s=args.index_timeout, - enforce_memory=not args.no_enforce_memory, - ) + limits = resolve_limits(workers=args.workers, timeout_s=args.index_timeout) for number, selected in enumerate(cumulative_batches(modules, args.batch_size), 1): _logger.info( "=== batch=%d modules_rendered=%d/%d project=%s ===", number, len(selected), len(modules), project ) write_cbmignore(root, render_cbmignore(selected)) - try: - index_repository(root, project, limits) - except MemoryLimitExceeded as error: - raise SystemExit("Batch %d: %s" % (number, error)) + index_repository(root, project, limits) if not args.skip_validate: if not validate_scope(project, modules): diff --git a/src/codebase_memory_deployv/indexer.py b/src/codebase_memory_deployv/indexer.py index 0f1429c..7cca092 100644 --- a/src/codebase_memory_deployv/indexer.py +++ b/src/codebase_memory_deployv/indexer.py @@ -18,7 +18,7 @@ import subprocess import urllib.request -from .limits import MB, limits_env, run_capped +from .limits import limits_env _logger = logging.getLogger(__name__) @@ -445,16 +445,12 @@ def write_cbmignore(root, content): def index_repository(root, project, limits=None): """Run one indexing pass reusing the same project name for every pass. - Without limits the pass behaves the way it always did: every core the container sees - and as much memory as it wants. Callers that care (the CLI does) hand over a resolved - limits.Limits so the pass stays a background job instead of taking over the machine. + Without limits the pass behaves the way it always did: every core the container sees. + Callers that care (the CLI does) hand over a resolved limits.Limits, whose values reach + codebase-memory-mcp as CBM_* variables it already knows how to read. """ command = [find_cbm() or CBM_BIN, "cli", "index_repository", "--repo_path", root, "--name=%s" % project] - if limits is None: - subprocess.check_call(command) - return - max_rss = limits.memory_mb * MB if limits.enforce_memory and limits.memory_mb else 0 - run_capped(command, env=limits_env(limits), max_rss_bytes=max_rss) + subprocess.check_call(command, env=limits_env(limits) if limits is not None else None) def query_graph(project, query): diff --git a/src/codebase_memory_deployv/limits.py b/src/codebase_memory_deployv/limits.py index c75573b..0d30ee8 100644 --- a/src/codebase_memory_deployv/limits.py +++ b/src/codebase_memory_deployv/limits.py @@ -1,82 +1,53 @@ -"""Bound the CPU, memory and wall time one codebase-memory-mcp indexing pass may take. +"""Decide how many indexing workers codebase-memory-mcp may use inside a container. -Left alone the indexer sizes itself against the whole machine. Measured inside a Vauxoo -container on a 18 GB MacBook (Docker Desktop VM of 11.9 GiB):: +Left alone the indexer takes every core it can see. 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% of the VM) + codebase-memory-mcp cli --index-worker 428% CPU 9.0 GB RSS 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 nothing -below this tool stops it either: the contention has to come from here. - -Three knobs are handed to the child process: - - CBM_WORKERS parsing workers; a quarter of the usable CPUs by default - CBM_INDEX_WORKER_TIMEOUT_S how long one index worker may run; two hours by default - CBM_MEM_BUDGET_MB the indexer's *internal* budget; a quarter of RAM by default - -The third one is not a cap and must not be mistaken for one. It only tells -codebase-memory-mcp when to log ``mem.pressure`` and purge its allocator: the measured 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 environment variable that bounds RSS -(it tracks it, it never enforces it), and the portable OS mechanisms do not fit either: -``RLIMIT_AS``/``RLIMIT_DATA`` count the address space mimalloc reserves without touching, -``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 here, in the parent: the RSS of the child process tree is sampled -while the pass runs and the tree is killed when it crosses the limit. Recovery is the -mechanism this tool already ships and documents — lower ``--batch-size`` and run again, -since the batches are cumulative and everything indexed so far stays in the graph. +That is deliberate on its side: ``cbm_default_worker_count(initial=true)`` returns +``total_cores`` with the comment "Use all cores for initial indexing — user is waiting". +Inside a container that count comes from ``sysconf(_SC_NPROCESSORS_ONLN)``, which reports +the host CPUs and not the cgroup quota — codebase-memory-mcp says so itself, right where +it reads the override:: + + CBM_WORKERS env override (clamped to [1, CBM_WORKERS_MAX]). + Useful inside containers where sysconf(_SC_NPROCESSORS_ONLN) + reports host CPUs rather than the cgroup's effective CPU quota. + +Resolving that quota is the one thing it delegates to its caller, so it is the one thing +this module does. + +Memory is deliberately *not* handled here. codebase-memory-mcp already sizes its budget as +a fraction of RAM that scales with the machine (``mem.c``: 25% at or below 16 GB, 35% at or +below 32 GB, 50% above), which on the measured 11.9 GiB VM is the same 2986 MB any +quarter-of-RAM rule would produce — exporting CBM_MEM_BUDGET_MB there changes nothing. And +that budget is not a ceiling either: it drives ``mem.pressure`` logging and allocator +purges, which is why the measured worker reported ``budget_mb=2986`` and still reached +9.0 GB RSS. A real ceiling belongs to the kernel, not to a userspace sampler that can only +notice the overshoot after it happened: bound the container itself with +``docker run --memory=3g --cpus=2`` and the cgroup enforces it for everything inside. """ import collections -import errno import logging import os -import signal -import subprocess -import time _logger = logging.getLogger(__name__) -# A quarter of the machine each, so the indexer stays a background job on the developer's -# laptop instead of taking it over. Both are overridable on the command line. +# A quarter of the cores, so indexing stays a background job on the developer's laptop +# instead of taking it over. Overridable with --workers. DEFAULT_CPU_FRACTION = 0.25 -DEFAULT_MEMORY_FRACTION = 0.25 -# Two hours. The default of codebase-memory-mcp is far shorter than a full Odoo instance -# pass, and a batch killed by the timeout looks exactly like a batch killed by the OOM -# killer, which sends the reader hunting for a memory problem that is not there. -DEFAULT_INDEX_TIMEOUT_S = 7200 -# Below this a cap is not worth enforcing: the indexer cannot even load the Odoo core tree -# and every pass would die on a limit the machine, not the tool, is responsible for. -MIN_MEMORY_MB = 512 -MEMORY_POLL_INTERVAL_S = 5.0 -# Grace given to the process tree to exit on SIGTERM before SIGKILL. The worker flushes its -# sqlite store on the way out; killing it outright leaves the store to be recovered. -KILL_GRACE_S = 10.0 WORKERS_ENV = "CBM_WORKERS" -MEMORY_BUDGET_ENV = "CBM_MEM_BUDGET_MB" TIMEOUT_ENV = "CBM_INDEX_WORKER_TIMEOUT_S" -MB = 1024 * 1024 CGROUP_V2_CPU = "/sys/fs/cgroup/cpu.max" CGROUP_V1_CPU_QUOTA = "/sys/fs/cgroup/cpu/cpu.cfs_quota_us" CGROUP_V1_CPU_PERIOD = "/sys/fs/cgroup/cpu/cpu.cfs_period_us" -CGROUP_V2_MEMORY = "/sys/fs/cgroup/memory.max" -CGROUP_V1_MEMORY = "/sys/fs/cgroup/memory/memory.limit_in_bytes" -PROC_MEMINFO = "/proc/meminfo" -# cgroup v1 spells "no limit" as a number close to the whole address space; anything at or -# above this is the kernel saying unlimited, not a real 8 EB container. -CGROUP_V1_UNLIMITED = 1 << 62 -Limits = collections.namedtuple("Limits", "workers memory_mb timeout_s enforce_memory") - - -class MemoryLimitExceeded(RuntimeError): - """Raised when an indexing pass grew past the configured memory cap and was killed.""" +Limits = collections.namedtuple("Limits", "workers timeout_s") def _read_file(path): @@ -89,28 +60,20 @@ def _read_file(path): def cgroup_cpu_quota(): - """CPUs granted by the cgroup (e.g. 5.0 for "docker update --cpus=5"), or None. - - os.cpu_count() reports the CPUs of the machine, not of the container: a quarter of it - is the wrong number the moment anybody bounds the container from the outside. - """ + """CPUs granted by the cgroup (5.0 for "docker update --cpus=5"), or None when unbounded.""" raw = _read_file(CGROUP_V2_CPU).split() if len(raw) == 2 and raw[0] != "max": try: quota, period = int(raw[0]), int(raw[1]) except ValueError: return None - if quota > 0 and period > 0: - return float(quota) / period - return None + return float(quota) / period if quota > 0 and period > 0 else None try: quota = int(_read_file(CGROUP_V1_CPU_QUOTA).strip() or -1) period = int(_read_file(CGROUP_V1_CPU_PERIOD).strip() or 0) except ValueError: return None - if quota > 0 and period > 0: - return float(quota) / period - return None + return float(quota) / period if quota > 0 and period > 0 else None def usable_cpus(): @@ -127,59 +90,11 @@ def usable_cpus(): return max(1, os.cpu_count() or 1) -def cgroup_memory_limit(): - """Memory granted by the cgroup in bytes, or None when the container is unbounded.""" - raw = _read_file(CGROUP_V2_MEMORY).strip() - if raw and raw != "max": - try: - return int(raw) - except ValueError: - return None - raw = _read_file(CGROUP_V1_MEMORY).strip() - if raw: - try: - value = int(raw) - except ValueError: - return None - if 0 < value < CGROUP_V1_UNLIMITED: - return value - return None - - -def total_memory_bytes(): - """Memory the machine offers: cgroup limit when bounded, else the OS total, else 0. - - 0 means "unknown", and every caller treats it as "no default cap can be computed" - rather than guessing a number that would silently kill legitimate passes. - """ - limit = cgroup_memory_limit() - if limit: - return limit - for line in _read_file(PROC_MEMINFO).splitlines(): - if line.startswith("MemTotal:"): - parts = line.split() - if len(parts) >= 2 and parts[1].isdigit(): - return int(parts[1]) * 1024 - try: - pages = os.sysconf("SC_PHYS_PAGES") * os.sysconf("SC_PAGE_SIZE") - except (AttributeError, ValueError, OSError): - return 0 - return pages if pages > 0 else 0 - - def default_workers(fraction=DEFAULT_CPU_FRACTION): """A quarter of the usable CPUs, never below one.""" return max(1, int(usable_cpus() * fraction)) -def default_memory_mb(fraction=DEFAULT_MEMORY_FRACTION): - """A quarter of the usable memory in MB, or 0 when the total cannot be read.""" - total = total_memory_bytes() - if not total: - return 0 - return max(MIN_MEMORY_MB, int(total * fraction) // MB) - - def _from_environment(environ, name): """Positive integer exported as name, or None. An exported value is a deliberate choice.""" try: @@ -189,213 +104,37 @@ def _from_environment(environ, name): return value if value > 0 else None -def _resolve(explicit, environ, name, computed): - """Pick a value and say where it came from: command line, environment, then default.""" - if explicit is not None: - return explicit, "argument" - exported = _from_environment(environ, name) - if exported is not None: - return exported, name - return computed, "default" +def resolve_limits(workers=None, timeout_s=None, environ=None): + """Resolve the worker count and the optional worker timeout, logging each source. + Precedence is the command line, then an already exported CBM_* variable, then the + default. The environment is honoured on purpose: whoever exported CBM_WORKERS in that + container knows something about it, and this tool is not the only thing reading it. -def resolve_limits(workers=None, memory_mb=None, timeout_s=None, enforce_memory=True, environ=None): - """Resolve the three limits, logging each value and its source. - - Precedence is command line, then an already exported CBM_* variable, then a quarter of - the machine. The environment is honoured on purpose: the caller who exported CBM_WORKERS - knows something about that container, and this tool is not the only thing reading it. + timeout_s has no default. CBM_INDEX_WORKER_TIMEOUT_S is a *no-progress* window, not a + time budget -- codebase-memory-mcp kills a worker that logs nothing for 15 minutes, and + every progress line resets it -- so raising it does not buy a long pass more time, it + only makes the hang detector slower to fire. It stays available for whoever needs that. """ environ = os.environ if environ is None else environ - workers, workers_source = _resolve(workers, environ, WORKERS_ENV, default_workers()) - memory_mb, memory_source = _resolve(memory_mb, environ, MEMORY_BUDGET_ENV, default_memory_mb()) - timeout_s, timeout_source = _resolve(timeout_s, environ, TIMEOUT_ENV, DEFAULT_INDEX_TIMEOUT_S) - _logger.info("workers=%d source=%s usable_cpus=%d", workers, workers_source, usable_cpus()) - _logger.info( - "memory_mb=%s source=%s total_ram_mb=%d enforce=%s", - memory_mb or "unknown", - memory_source, - total_memory_bytes() // MB, - "yes" if enforce_memory and memory_mb else "no", - ) - _logger.info("index_worker_timeout_s=%d source=%s", timeout_s, timeout_source) - if enforce_memory and not memory_mb: - _logger.warning("memory_cap=none (total memory unreadable); the pass runs unbounded") - return Limits(workers=workers, memory_mb=memory_mb, timeout_s=timeout_s, enforce_memory=enforce_memory) + if workers is None: + workers = _from_environment(environ, WORKERS_ENV) + source = WORKERS_ENV if workers else "default" + workers = workers or default_workers() + else: + source = "argument" + _logger.info("workers=%d source=%s usable_cpus=%d", workers, source, usable_cpus()) + if timeout_s is None: + timeout_s = _from_environment(environ, TIMEOUT_ENV) + if timeout_s: + _logger.info("index_worker_no_progress_timeout_s=%d", timeout_s) + return Limits(workers=workers, timeout_s=timeout_s) def limits_env(limits, environ=None): """A copy of environ carrying the resolved limits, ready for subprocess.""" env = dict(os.environ if environ is None else environ) env[WORKERS_ENV] = str(limits.workers) - env[TIMEOUT_ENV] = str(limits.timeout_s) - if limits.memory_mb: - env[MEMORY_BUDGET_ENV] = str(limits.memory_mb) + if limits.timeout_s: + env[TIMEOUT_ENV] = str(limits.timeout_s) return env - - -def _linux_process_table(): - """{pid: (ppid, rss_bytes)} read from /proc, or {} when /proc is not there.""" - page_size = os.sysconf("SC_PAGE_SIZE") if hasattr(os, "sysconf") else 4096 - table = {} - try: - entries = os.listdir("/proc") - except OSError: - return table - for entry in entries: - if not entry.isdigit(): - continue - stat = _read_file("/proc/%s/stat" % entry) - # The command name sits in parentheses and may contain spaces: everything before - # the last ")" is pid + comm, so ppid is the second field of what follows. - _, _, rest = stat.rpartition(")") - fields = rest.split() - if len(fields) < 2 or not fields[1].lstrip("-").isdigit(): - continue - statm = _read_file("/proc/%s/statm" % entry).split() - if len(statm) < 2 or not statm[1].isdigit(): - continue - table[int(entry)] = (int(fields[1]), int(statm[1]) * page_size) - return table - - -def _ps_process_table(): - """{pid: (ppid, rss_bytes)} from ps, for the platforms without /proc (macOS).""" - try: - raw = subprocess.check_output(["ps", "-Ao", "pid=,ppid=,rss="], universal_newlines=True) - except (OSError, subprocess.CalledProcessError): - return {} - table = {} - for line in raw.splitlines(): - fields = line.split() - if len(fields) < 3 or not all(field.lstrip("-").isdigit() for field in fields[:3]): - continue - table[int(fields[0])] = (int(fields[1]), int(fields[2]) * 1024) - return table - - -def process_table(): - """{pid: (ppid, rss_bytes)} for every process on the machine.""" - return _linux_process_table() if os.path.isdir("/proc") else _ps_process_table() - - -def process_tree(pid, table): - """Return pid plus every descendant of it present in table. - - The whole tree matters, not just the process this tool started: "codebase-memory-mcp - cli index_repository" forks the ``--index-worker`` child that does the parsing, and - that child is the one that reached 9.0 GB. - """ - children = {} - for child, (parent, _rss) in table.items(): - children.setdefault(parent, []).append(child) - found = [] - pending = [pid] - seen = set() - while pending: - current = pending.pop() - if current in seen or current not in table: - continue - seen.add(current) - found.append(current) - pending.extend(children.get(current, ())) - return found - - -def tree_rss_bytes(pid, table=None): - """RSS of pid and its descendants, in bytes.""" - table = process_table() if table is None else table - return sum(table[member][1] for member in process_tree(pid, table)) - - -# Windows has no SIGKILL; there os.kill(SIGTERM) already calls TerminateProcess, so the -# escalation collapses into a single stage instead of failing with an AttributeError. -SIGKILL = getattr(signal, "SIGKILL", signal.SIGTERM) - - -def _signal_tree(members, sig): - """Send sig to every member still alive, ignoring the ones that already left.""" - for member in members: - try: - os.kill(member, sig) - except OSError as error: - if error.errno != errno.ESRCH: - raise - - -def _kill_tree(process, members): - """SIGTERM the process tree, then SIGKILL whatever is still alive after the grace. - - The members are signalled one by one instead of through the process group so the pass - keeps sharing the terminal group: starting it in a session of its own would stop Ctrl-C - from reaching an indexing run that takes hours. - - Reaping goes through the Popen object only. A bare os.waitpid() here would steal the - status Popen is waiting for and turn its next wait() into ECHILD. - """ - _signal_tree(members, signal.SIGTERM) - deadline = time.time() + KILL_GRACE_S - while time.time() < deadline: - if process.poll() is not None: - break - time.sleep(0.1) - if process.poll() is None: - _signal_tree(members, SIGKILL) - process.wait() - # Descendants outlive the parent they were forked from, so sweep them once more. - _signal_tree([member for member in members if member != process.pid], SIGKILL) - - -def can_measure_rss(): - """True when this platform exposes a process table (Linux /proc, or ps elsewhere). - - Only Linux containers ever run an indexing pass, but the package is imported and tested - on macOS and Windows too, and a cap that cannot be measured must say so rather than - look enforced. - """ - return bool(process_table()) - - -def run_capped(cmd, env=None, max_rss_bytes=0, poll_interval=MEMORY_POLL_INTERVAL_S): - """Run cmd, killing its whole process tree if the tree grows past max_rss_bytes. - - Returns the peak RSS observed, in bytes. A non-positive max_rss_bytes only measures. - Raises MemoryLimitExceeded when the cap is crossed and CalledProcessError on a plain - non-zero exit, so the caller can tell "too big" apart from "it failed". - """ - if max_rss_bytes > 0 and not can_measure_rss(): - _logger.warning("memory_cap=unenforceable (no readable process table on this platform)") - max_rss_bytes = 0 - process = subprocess.Popen(cmd, env=env) - peak = 0 - try: - while True: - table = process_table() - members = process_tree(process.pid, table) - rss = sum(table[member][1] for member in members) - peak = max(peak, rss) - if max_rss_bytes > 0 and rss > max_rss_bytes: - _logger.error( - "memory_cap_exceeded rss_mb=%d limit_mb=%d processes=%d; killing the pass", - rss // MB, - max_rss_bytes // MB, - len(members), - ) - _kill_tree(process, members) - raise MemoryLimitExceeded( - "the indexing pass reached %d MB, over the %d MB cap. Lower --batch-size so each " - "pass parses fewer modules, or raise --max-memory-mb when the machine can afford it" - % (rss // MB, max_rss_bytes // MB) - ) - if process.poll() is not None: - break - time.sleep(poll_interval) - finally: - if process.poll() is None: - # Ctrl-C or any other exception on the way out: take the descendants down too, - # otherwise the index worker keeps eating the machine with nobody watching it. - _kill_tree(process, process_tree(process.pid, process_table())) - _logger.info("peak_rss_mb=%d limit_mb=%s", peak // MB, max_rss_bytes // MB if max_rss_bytes > 0 else "none") - if process.returncode: - raise subprocess.CalledProcessError(process.returncode, cmd) - return peak diff --git a/tests/test_cli.py b/tests/test_cli.py index 95c609e..0dd1155 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4,7 +4,6 @@ from codebase_memory_deployv import cli from codebase_memory_deployv.indexer import DEFAULT_BATCH_SIZE, DEFAULT_ROOT -from codebase_memory_deployv.limits import MemoryLimitExceeded @pytest.fixture(autouse=True) @@ -99,14 +98,12 @@ def test_parser_resource_defaults_are_resolved_at_runtime(): """The flags default to None so the environment still has a say; limits resolve them.""" args = cli.build_parser().parse_args([]) assert args.workers is None - assert args.max_memory_mb is None assert args.index_timeout is None - assert not args.no_enforce_memory def test_parser_accepts_the_resource_flags(): - args = cli.build_parser().parse_args(["--workers=2", "--max-memory-mb=1500", "--index-timeout=600"]) - assert (args.workers, args.max_memory_mb, args.index_timeout) == (2, 1500, 600) + args = cli.build_parser().parse_args(["--workers=2", "--index-timeout=600"]) + assert (args.workers, args.index_timeout) == (2, 600) def test_main_passes_the_resolved_limits_to_every_pass(tmp_path, monkeypatch): @@ -115,26 +112,10 @@ def test_main_passes_the_resolved_limits_to_every_pass(tmp_path, monkeypatch): monkeypatch.setattr(cli, "ensure_cbm_installed", lambda: "cbm") monkeypatch.setattr(cli, "index_repository", lambda root_, project, limits_: seen.append(limits_)) monkeypatch.setattr(cli, "installed_modules", lambda root_: None) - assert cli.main(["--repo-path", root, "--workers=2", "--max-memory-mb=1500", "--skip-validate"]) == 0 - assert [(one.workers, one.memory_mb, one.enforce_memory) for one in seen] == [(2, 1500, True)] - - -def test_main_reports_a_pass_killed_by_the_memory_cap(tmp_path, monkeypatch): - """A killed pass must name its batch and the way out, not just die.""" - root = _instance(tmp_path, "extra_addons/vauxoo/sale_extended") - - def boom(root_, project, limits_): - raise MemoryLimitExceeded("the indexing pass reached 3000 MB, over the 1500 MB cap. Lower --batch-size") - - monkeypatch.setattr(cli, "ensure_cbm_installed", lambda: "cbm") - monkeypatch.setattr(cli, "index_repository", boom) - monkeypatch.setattr(cli, "installed_modules", lambda root_: None) - with pytest.raises(SystemExit) as error: - cli.main(["--repo-path", root, "--max-memory-mb=1500", "--skip-validate"]) - assert "Batch 1" in str(error.value) - assert "--batch-size" in str(error.value) + assert cli.main(["--repo-path", root, "--workers=2", "--skip-validate"]) == 0 + assert [one.workers for one in seen] == [2] def test_help_renders(): """argparse re-expands "%" in a help string: a literal one there breaks --help only.""" - assert "--max-memory-mb" in cli.build_parser().format_help() + assert "--workers" in cli.build_parser().format_help() diff --git a/tests/test_indexer.py b/tests/test_indexer.py index b23a5a0..7af416f 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -485,41 +485,26 @@ def test_check_layout_requires_odoo_bin(tmp_path): def test_index_repository_without_limits_keeps_the_old_behaviour(monkeypatch): calls = [] monkeypatch.setattr(indexer, "find_cbm", lambda: "/usr/bin/cbm") - monkeypatch.setattr(indexer.subprocess, "check_call", lambda cmd: calls.append(cmd)) + monkeypatch.setattr(indexer.subprocess, "check_call", lambda cmd, env=None: calls.append((cmd, env))) indexer.index_repository("/home/odoo/instance", "vauxoo_12.0") - assert calls == [ - ["/usr/bin/cbm", "cli", "index_repository", "--repo_path", "/home/odoo/instance", "--name=vauxoo_12.0"] + command, env = calls[0] + assert command == [ + "/usr/bin/cbm", + "cli", + "index_repository", + "--repo_path", + "/home/odoo/instance", + "--name=vauxoo_12.0", ] + assert env is None -def test_index_repository_caps_the_pass_when_given_limits(monkeypatch): - """The env carries the three CBM_* knobs and the watchdog gets the cap in bytes.""" - captured = {} - - def fake_run_capped(cmd, env=None, max_rss_bytes=0): - captured.update(cmd=cmd, env=env, max_rss_bytes=max_rss_bytes) - - monkeypatch.setattr(indexer, "find_cbm", lambda: "/usr/bin/cbm") - monkeypatch.setattr(indexer, "run_capped", fake_run_capped) - monkeypatch.setattr(indexer.subprocess, "check_call", lambda cmd: pytest.fail("must not bypass the cap")) - resolved = limits.Limits(workers=3, memory_mb=1500, timeout_s=7200, enforce_memory=True) - indexer.index_repository("/home/odoo/instance", "vauxoo_12.0", resolved) - assert captured["max_rss_bytes"] == 1500 * limits.MB - assert captured["env"]["CBM_WORKERS"] == "3" - assert captured["env"]["CBM_MEM_BUDGET_MB"] == "1500" - assert captured["env"]["CBM_INDEX_WORKER_TIMEOUT_S"] == "7200" - - -def test_index_repository_only_advises_when_enforcement_is_off(monkeypatch): - """--no-enforce-memory still exports the budget; it just stops the watchdog killing.""" - captured = {} - - def fake_run_capped(cmd, env=None, max_rss_bytes=0): - captured.update(env=env, max_rss_bytes=max_rss_bytes) - +def test_index_repository_hands_the_worker_count_to_the_child(monkeypatch): + """The pass is bounded by what codebase-memory-mcp already reads, not by a new mechanism.""" + calls = [] monkeypatch.setattr(indexer, "find_cbm", lambda: "/usr/bin/cbm") - monkeypatch.setattr(indexer, "run_capped", fake_run_capped) - resolved = limits.Limits(workers=3, memory_mb=1500, timeout_s=7200, enforce_memory=False) - indexer.index_repository("/home/odoo/instance", "vauxoo_12.0", resolved) - assert captured["max_rss_bytes"] == 0 - assert captured["env"]["CBM_MEM_BUDGET_MB"] == "1500" + monkeypatch.setattr(indexer.subprocess, "check_call", lambda cmd, env=None: calls.append((cmd, env))) + indexer.index_repository("/home/odoo/instance", "vauxoo_12.0", limits.Limits(workers=3, timeout_s=None)) + _command, env = calls[0] + assert env["CBM_WORKERS"] == "3" + assert "CBM_MEM_BUDGET_MB" not in env diff --git a/tests/test_limits.py b/tests/test_limits.py index 5922a46..f7cbcca 100644 --- a/tests/test_limits.py +++ b/tests/test_limits.py @@ -1,18 +1,7 @@ -import os -import subprocess -import sys -import time - import pytest from codebase_memory_deployv import limits -# Only Linux containers ever run an indexing pass, but the suite also runs on macOS and -# Windows: there the process table is unreadable and the cap degrades to advisory. -needs_process_table = pytest.mark.skipif( - not limits.can_measure_rss(), reason="no readable process table on this platform" -) - def test_default_workers_is_a_quarter_of_the_cpus(monkeypatch): monkeypatch.setattr(limits, "usable_cpus", lambda: 12) @@ -25,21 +14,6 @@ def test_default_workers_never_reaches_zero(monkeypatch): assert limits.default_workers() == 1 -def test_default_memory_is_a_quarter_of_the_ram(monkeypatch): - monkeypatch.setattr(limits, "total_memory_bytes", lambda: 11946 * limits.MB) - assert limits.default_memory_mb() == 2986 - - -def test_default_memory_has_a_floor(monkeypatch): - monkeypatch.setattr(limits, "total_memory_bytes", lambda: 1024 * limits.MB) - assert limits.default_memory_mb() == limits.MIN_MEMORY_MB - - -def test_default_memory_unknown_total(monkeypatch): - monkeypatch.setattr(limits, "total_memory_bytes", lambda: 0) - assert limits.default_memory_mb() == 0 - - def test_cgroup_cpu_quota_v2(tmp_path, monkeypatch): """docker update --cpus=5 must be seen; os.cpu_count() would keep reporting the host.""" quota = tmp_path / "cpu.max" @@ -66,152 +40,54 @@ def test_cgroup_cpu_quota_v1(tmp_path, monkeypatch): assert limits.cgroup_cpu_quota() == 2.0 -def test_cgroup_memory_limit_v2(tmp_path, monkeypatch): - limit = tmp_path / "memory.max" - limit.write_text("%d\n" % (4 * limits.MB)) - monkeypatch.setattr(limits, "CGROUP_V2_MEMORY", str(limit)) - assert limits.cgroup_memory_limit() == 4 * limits.MB - - -def test_cgroup_memory_limit_v1_unlimited_is_not_a_limit(tmp_path, monkeypatch): - """cgroup v1 spells "no limit" as a huge number; taking a quarter of it caps nothing.""" - limit = tmp_path / "memory.limit_in_bytes" - limit.write_text("9223372036854771712\n") - monkeypatch.setattr(limits, "CGROUP_V2_MEMORY", str(tmp_path / "missing")) - monkeypatch.setattr(limits, "CGROUP_V1_MEMORY", str(limit)) - assert limits.cgroup_memory_limit() is None - - -def test_total_memory_prefers_the_cgroup_over_the_host(monkeypatch, tmp_path): - meminfo = tmp_path / "meminfo" - meminfo.write_text("MemTotal: 12234752 kB\n") - monkeypatch.setattr(limits, "PROC_MEMINFO", str(meminfo)) - monkeypatch.setattr(limits, "cgroup_memory_limit", lambda: 2048 * limits.MB) - assert limits.total_memory_bytes() == 2048 * limits.MB - monkeypatch.setattr(limits, "cgroup_memory_limit", lambda: None) - assert limits.total_memory_bytes() == 12234752 * 1024 +def test_cgroup_cpu_quota_malformed(tmp_path, monkeypatch): + """A cpu.max that does not parse must read as "unbounded", not raise mid-run.""" + quota = tmp_path / "cpu.max" + quota.write_text("weird\n") + monkeypatch.setattr(limits, "CGROUP_V2_CPU", str(quota)) + monkeypatch.setattr(limits, "CGROUP_V1_CPU_QUOTA", str(tmp_path / "missing")) + assert limits.cgroup_cpu_quota() is None def test_resolve_limits_defaults(monkeypatch): monkeypatch.setattr(limits, "usable_cpus", lambda: 8) - monkeypatch.setattr(limits, "total_memory_bytes", lambda: 8192 * limits.MB) - resolved = limits.resolve_limits(environ={}) - assert resolved == limits.Limits(workers=2, memory_mb=2048, timeout_s=7200, enforce_memory=True) + assert limits.resolve_limits(environ={}) == limits.Limits(workers=2, timeout_s=None) def test_resolve_limits_arguments_win_over_the_environment(monkeypatch): monkeypatch.setattr(limits, "usable_cpus", lambda: 8) - monkeypatch.setattr(limits, "total_memory_bytes", lambda: 8192 * limits.MB) - environ = {"CBM_WORKERS": "7", "CBM_MEM_BUDGET_MB": "9000", "CBM_INDEX_WORKER_TIMEOUT_S": "60"} - resolved = limits.resolve_limits(workers=1, memory_mb=500, timeout_s=30, environ=environ) - assert resolved == limits.Limits(workers=1, memory_mb=500, timeout_s=30, enforce_memory=True) + environ = {"CBM_WORKERS": "7", "CBM_INDEX_WORKER_TIMEOUT_S": "60"} + assert limits.resolve_limits(workers=1, timeout_s=30, environ=environ) == limits.Limits(workers=1, timeout_s=30) def test_resolve_limits_environment_wins_over_the_default(monkeypatch): """Whoever exported CBM_WORKERS knows something about that container; do not overrule it.""" monkeypatch.setattr(limits, "usable_cpus", lambda: 8) - monkeypatch.setattr(limits, "total_memory_bytes", lambda: 8192 * limits.MB) - environ = {"CBM_WORKERS": "7", "CBM_MEM_BUDGET_MB": "9000", "CBM_INDEX_WORKER_TIMEOUT_S": "60"} - resolved = limits.resolve_limits(environ=environ) - assert resolved == limits.Limits(workers=7, memory_mb=9000, timeout_s=60, enforce_memory=True) + environ = {"CBM_WORKERS": "7", "CBM_INDEX_WORKER_TIMEOUT_S": "60"} + assert limits.resolve_limits(environ=environ) == limits.Limits(workers=7, timeout_s=60) @pytest.mark.parametrize("value", ["", "0", "-4", "many", " "]) def test_resolve_limits_ignores_unusable_environment_values(monkeypatch, value): monkeypatch.setattr(limits, "usable_cpus", lambda: 8) - monkeypatch.setattr(limits, "total_memory_bytes", lambda: 8192 * limits.MB) - resolved = limits.resolve_limits(environ={"CBM_WORKERS": value}) - assert resolved.workers == 2 + resolved = limits.resolve_limits(environ={"CBM_WORKERS": value, "CBM_INDEX_WORKER_TIMEOUT_S": value}) + assert resolved == limits.Limits(workers=2, timeout_s=None) -def test_limits_env_exports_the_three_variables(): - resolved = limits.Limits(workers=3, memory_mb=2986, timeout_s=7200, enforce_memory=True) - env = limits.limits_env(resolved, environ={"PATH": "/usr/bin"}) +def test_limits_env_exports_the_worker_count(): + env = limits.limits_env(limits.Limits(workers=3, timeout_s=None), environ={"PATH": "/usr/bin"}) assert env["CBM_WORKERS"] == "3" - assert env["CBM_MEM_BUDGET_MB"] == "2986" - assert env["CBM_INDEX_WORKER_TIMEOUT_S"] == "7200" assert env["PATH"] == "/usr/bin" -def test_limits_env_omits_an_unknown_memory_budget(): - """Exporting CBM_MEM_BUDGET_MB=0 would be read as invalid; leave the default in place.""" - resolved = limits.Limits(workers=1, memory_mb=0, timeout_s=7200, enforce_memory=True) - assert "CBM_MEM_BUDGET_MB" not in limits.limits_env(resolved, environ={}) - - -def test_process_tree_collects_the_descendants(): - # 10 -> 20 -> 30, plus an unrelated 40 - table = {10: (1, 100), 20: (10, 200), 30: (20, 400), 40: (1, 800)} - assert sorted(limits.process_tree(10, table)) == [10, 20, 30] - assert limits.tree_rss_bytes(10, table) == 700 - - -def test_process_tree_survives_a_cycle(): - """A pid table read while processes come and go can look self-parented; do not hang.""" - table = {10: (10, 100)} - assert limits.process_tree(10, table) == [10] - - -@needs_process_table -def test_process_table_sees_this_process(): - table = limits.process_table() - assert os.getpid() in table - assert table[os.getpid()][1] > 0 - - -@needs_process_table -def test_run_capped_returns_the_peak_and_the_exit_status(): - peak = limits.run_capped([sys.executable, "-c", "import time; time.sleep(0.5)"], poll_interval=0.01) - assert peak > 0 - with pytest.raises(subprocess.CalledProcessError): - limits.run_capped([sys.executable, "-c", "raise SystemExit(3)"], poll_interval=0.01) - - -def test_run_capped_reports_zero_for_a_pass_shorter_than_one_sample(): - """Sampling cannot see a process that already exited; that is a 0, never a failure.""" - assert limits.run_capped([sys.executable, "-c", "pass"], poll_interval=0.01) >= 0 - - -@needs_process_table -def test_run_capped_kills_a_process_over_the_cap(): - """The whole point: the pass dies instead of the machine. - - CBM_MEM_BUDGET_MB cannot do this — the measured worker reported budget_mb=2986 and - still grew to 9.0 GB — so the cap has to be enforced from the parent. - """ - grow = "buffer = bytearray()\nwhile True:\n buffer += bytearray(8 * 1024 * 1024)\n" - with pytest.raises(limits.MemoryLimitExceeded) as error: - limits.run_capped([sys.executable, "-c", grow], max_rss_bytes=64 * limits.MB, poll_interval=0.01) - assert "--batch-size" in str(error.value) - - -@needs_process_table -def test_run_capped_kills_the_descendants_too(tmp_path): - """The parsing happens in the "--index-worker" child, so killing only the parent leaks it.""" - marker = tmp_path / "child.pid" - # The grandchild waits before allocating so the pid file is always on disk by the time - # the cap is crossed; otherwise the assertion races the watchdog. - grow = "import time\ntime.sleep(1)\nbuffer = bytearray()\nwhile True:\n buffer += bytearray(8 * 1024 * 1024)" - script = ( - "import subprocess, sys, time\n" - "child = subprocess.Popen([sys.executable, '-c', %r])\n" - "open(%r, 'w').write(str(child.pid))\n" - "time.sleep(120)\n" % (grow, str(marker)) - ) - with pytest.raises(limits.MemoryLimitExceeded): - limits.run_capped([sys.executable, "-c", script], max_rss_bytes=128 * limits.MB, poll_interval=0.05) - child_pid = int(marker.read_text()) - # Gone, or a zombie nobody reaped yet: either way it stopped holding memory. Asking for - # the pid to disappear would depend on whether PID 1 of the container reaps orphans. - deadline = time.time() + 15 - while time.time() < deadline and limits.tree_rss_bytes(child_pid): - time.sleep(0.1) - assert limits.tree_rss_bytes(child_pid) == 0 - - -def test_run_capped_does_not_pretend_to_enforce_without_a_process_table(monkeypatch, caplog): - """A cap that cannot be measured must say so, not silently look enforced.""" - monkeypatch.setattr(limits, "process_table", dict) - with caplog.at_level("WARNING"): - assert limits.run_capped([sys.executable, "-c", "pass"], max_rss_bytes=1, poll_interval=0.01) == 0 - assert "memory_cap=unenforceable" in caplog.text +def test_limits_env_leaves_the_timeout_alone_unless_asked(): + """CBM_INDEX_WORKER_TIMEOUT_S is a no-progress window; its 15 min default is fine.""" + assert limits.TIMEOUT_ENV not in limits.limits_env(limits.Limits(workers=1, timeout_s=None), environ={}) + env = limits.limits_env(limits.Limits(workers=1, timeout_s=600), environ={}) + assert env[limits.TIMEOUT_ENV] == "600" + + +def test_limits_module_does_not_touch_the_memory_budget(): + """codebase-memory-mcp already scales its budget with the RAM (25/35/50%); do not duplicate it.""" + env = limits.limits_env(limits.Limits(workers=2, timeout_s=None), environ={}) + assert "CBM_MEM_BUDGET_MB" not in env From 5621c6802424f4df740c8ea0dc070fadaf7b02cc Mon Sep 17 00:00:00 2001 From: "Moises Lopez - https://www.vauxoo.com/" Date: Tue, 1 Sep 2026 16:55:38 -0600 Subject: [PATCH 4/5] [IMP] Make the indexed file extensions configurable with --extensions --- src/codebase_memory_deployv/cli.py | 24 +++++++++++++++-- src/codebase_memory_deployv/indexer.py | 37 +++++++++++++++++++------- tests/test_cli.py | 2 +- tests/test_indexer.py | 20 ++++++++++++++ 4 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/codebase_memory_deployv/cli.py b/src/codebase_memory_deployv/cli.py index d81434c..7d5921c 100644 --- a/src/codebase_memory_deployv/cli.py +++ b/src/codebase_memory_deployv/cli.py @@ -9,6 +9,8 @@ from .indexer import ( DEFAULT_BATCH_SIZE, DEFAULT_ROOT, + MB_RSS_PER_SOURCE_FILE, + SOURCE_EXTENSIONS, check_layout, cumulative_batches, discover_modules, @@ -17,6 +19,8 @@ installed_modules, modules_from_names, read_modules_file, + extensions_for, + globs_for, render_cbmignore, resolve_project, validate_scope, @@ -78,6 +82,16 @@ def build_parser(): "Without a state column the file is assumed to list installed modules only. Ignored by --mode=all" ), ) + parser.add_argument( + "--extensions", + default=None, + help=( + "comma-separated file extensions to index, e.g. 'py,xml'. Fewer extensions means a " + "smaller graph and much less memory: codebase-memory-mcp holds the whole extraction in " + "RAM at roughly %s MB per file, so an Odoo instance costs ~14 GB with the default " + "%s and ~5 GB with 'py' alone" % (MB_RSS_PER_SOURCE_FILE, ",".join(SOURCE_EXTENSIONS)) + ), + ) parser.add_argument( "--workers", type=int, @@ -145,16 +159,22 @@ def main(argv=None): _logger.info("mode=%s modules=%d", mode, len(modules)) limits = resolve_limits(workers=args.workers, timeout_s=args.index_timeout) + source_globs = globs_for(args.extensions.split(",") if args.extensions else None) + _logger.info( + "extensions=%s estimated_peak_rss_mb_per_file=%s", + ",".join(extensions_for(source_globs)), + MB_RSS_PER_SOURCE_FILE, + ) for number, selected in enumerate(cumulative_batches(modules, args.batch_size), 1): _logger.info( "=== batch=%d modules_rendered=%d/%d project=%s ===", number, len(selected), len(modules), project ) - write_cbmignore(root, render_cbmignore(selected)) + write_cbmignore(root, render_cbmignore(selected, source_globs)) index_repository(root, project, limits) if not args.skip_validate: - if not validate_scope(project, modules): + if not validate_scope(project, modules, source_globs): raise SystemExit("Validation failed: fix .cbmignore, re-index and re-run validation") _logger.info("validation=ok") return 0 diff --git a/src/codebase_memory_deployv/indexer.py b/src/codebase_memory_deployv/indexer.py index 7cca092..ac7b2bd 100644 --- a/src/codebase_memory_deployv/indexer.py +++ b/src/codebase_memory_deployv/indexer.py @@ -44,6 +44,24 @@ SOURCE_GLOBS = ("*.py", "*.xml", "*.js", "*.rst", "*.md", "*.css", "*.scss", "*.csv", "*.sql") # What .cbmignore lets through, so what the graph is expected to hold: ".py", ".xml", ... SOURCE_EXTENSIONS = tuple(sorted(pattern[1:] for pattern in SOURCE_GLOBS)) +# Measured on a real instance: codebase-memory-mcp holds the whole extraction in memory and +# costs about this much RSS per source file it indexes (5248 files -> 4880 MB). It is the +# number that decides whether a scope fits, so narrowing SOURCE_GLOBS is the one lever the +# caller has: dropping .js/.scss/.csv/.md/.rst/.css takes an Odoo instance from 15860 files +# to 6042, i.e. from roughly 14 GB down to 5 GB. +MB_RSS_PER_SOURCE_FILE = 0.93 + + +def globs_for(extensions): + """Turn ("py", ".xml") into ("*.py", "*.xml"); None keeps the full SOURCE_GLOBS.""" + if not extensions: + return SOURCE_GLOBS + return tuple("*." + str(name).strip().lstrip("*.") for name in extensions if str(name).strip()) + + +def extensions_for(source_globs): + """The extensions a .cbmignore built from source_globs lets through.""" + return tuple(sorted(pattern[1:] for pattern in source_globs)) MANIFEST_NAMES = ("__manifest__.py", "__openerp__.py") PRUNE_DIRS = {".git", ".github", "__pycache__", "node_modules", ".cache", ".tx", "dist", "build", "setup"} CBMIGNORE_BACKUP_SUFFIX = ".before-codebase-memory-deployv" @@ -360,7 +378,7 @@ def cumulative_batches(module_paths, batch_size): return [ordered[:end] for end in range(batch_size, len(ordered) + batch_size, batch_size)] -def render_cbmignore(module_rel_paths): +def render_cbmignore(module_rel_paths, source_globs=SOURCE_GLOBS): """Render a .cbmignore keeping only the Odoo core tree plus the given modules.""" patterns = [] seen = set() @@ -377,7 +395,7 @@ def include_tree(rel): cur = part if not cur else cur + "/" + part add("!%s/" % cur) add("!%s/**/" % rel) - for glob_pattern in SOURCE_GLOBS: + for glob_pattern in source_globs: add("!%s/**/%s" % (rel, glob_pattern)) add("# Generated by codebase-memory-deployv from Odoo modules.") @@ -538,21 +556,22 @@ def indexed_extensions(paths): return counts -def validate_extensions(paths): +def validate_extensions(paths, source_globs=SOURCE_GLOBS): """Prove the graph only holds the extensions .cbmignore lets through. Anything else means .cbmignore did not apply (stale file, pattern typo, indexing run from another root), which silently bloats the graph with vendored/generated content. """ + expected = extensions_for(source_globs) counts = indexed_extensions(paths) - unexpected = {ext: total for ext, total in counts.items() if ext not in SOURCE_EXTENSIONS} - _logger.info("extensions_configured=%s", ",".join(SOURCE_EXTENSIONS)) + unexpected = {ext: total for ext, total in counts.items() if ext not in expected} + _logger.info("extensions_configured=%s", ",".join(expected)) _logger.info("extensions_indexed=%d files=%d", len(counts), sum(counts.values())) for extension, total in sorted(counts.items(), key=lambda item: -item[1]): - _logger.info("extension %-8s files=%d%s", extension, total, "" if extension in SOURCE_EXTENSIONS else " (!)") + _logger.info("extension %-8s files=%d%s", extension, total, "" if extension in expected else " (!)") if unexpected: _logger.error( - "unexpected_extensions=%d files=%d (not in SOURCE_GLOBS: %s)", + "unexpected_extensions=%d files=%d (not in the configured extensions: %s)", len(unexpected), sum(unexpected.values()), ",".join(sorted(unexpected)), @@ -584,7 +603,7 @@ def _is_test_addon(module_rel_path): return name.startswith("test_") and (parent == "addons" or parent.endswith("/addons")) -def validate_scope(project, expected_module_paths): +def validate_scope(project, expected_module_paths, source_globs=SOURCE_GLOBS): """Prove the graph scope matches the expected modules; return True when clean. Addons are enumerated through their manifests: every addon has exactly one, so the @@ -614,4 +633,4 @@ def validate_scope(project, expected_module_paths): _logger.error("extra %s", rel) # Extensions are checked last: the module comparison is the headline, this one tells # whether .cbmignore really applied to whatever did get indexed. - return validate_extensions(paths) and ok and not missing and not extra + return validate_extensions(paths, source_globs) and ok and not missing and not extra diff --git a/tests/test_cli.py b/tests/test_cli.py index 0dd1155..a80df0e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -58,7 +58,7 @@ def test_main_modules_file_wins_over_the_database(tmp_path, monkeypatch, capsys) monkeypatch.setattr(cli, "ensure_cbm_installed", lambda: "cbm") monkeypatch.setattr(cli, "index_repository", lambda root_, project, limits_: indexed.append(project)) monkeypatch.setattr(cli, "installed_modules", lambda root_: pytest.fail("the database must not be queried")) - monkeypatch.setattr(cli, "validate_scope", lambda project, modules: True) + monkeypatch.setattr(cli, "validate_scope", lambda project, modules, globs_=None: True) monkeypatch.setenv("CBM_PROJECT", "prod_18.0") assert cli.main(["--repo-path", root, "--modules-file", str(modules_file)]) == 0 out = capsys.readouterr().out diff --git a/tests/test_indexer.py b/tests/test_indexer.py index 7af416f..694902a 100644 --- a/tests/test_indexer.py +++ b/tests/test_indexer.py @@ -508,3 +508,23 @@ def test_index_repository_hands_the_worker_count_to_the_child(monkeypatch): _command, env = calls[0] assert env["CBM_WORKERS"] == "3" assert "CBM_MEM_BUDGET_MB" not in env + + +def test_globs_for_normalises_extensions(): + assert indexer.globs_for(["py", ".xml", " js "]) == ("*.py", "*.xml", "*.js") + assert indexer.globs_for(None) == indexer.SOURCE_GLOBS + assert indexer.globs_for([]) == indexer.SOURCE_GLOBS + + +def test_render_cbmignore_honours_a_narrowed_extension_set(): + """Fewer extensions is the one lever on memory: it is what shrinks the file count.""" + content = indexer.render_cbmignore(["extra_addons/vauxoo/sale_extended"], ("*.py",)) + assert "!extra_addons/vauxoo/sale_extended/**/*.py" in content + assert "*.js" not in content + assert "*.scss" not in content + + +def test_validate_extensions_follows_the_configured_set(caplog): + paths = ["a/b.py", "a/c.js"] + assert indexer.validate_extensions(paths, indexer.SOURCE_GLOBS) + assert not indexer.validate_extensions(paths, ("*.py",)) From 28a2485520631bb6b8ff6a79d9cadcb2848bf888 Mon Sep 17 00:00:00 2001 From: "Moises Lopez - https://www.vauxoo.com/" Date: Tue, 1 Sep 2026 16:57:53 -0600 Subject: [PATCH 5/5] [FIX] indexer: state what --extensions really buys 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. --- src/codebase_memory_deployv/cli.py | 14 ++++---------- src/codebase_memory_deployv/indexer.py | 15 ++++++++++----- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/codebase_memory_deployv/cli.py b/src/codebase_memory_deployv/cli.py index 7d5921c..9f96908 100644 --- a/src/codebase_memory_deployv/cli.py +++ b/src/codebase_memory_deployv/cli.py @@ -9,7 +9,6 @@ from .indexer import ( DEFAULT_BATCH_SIZE, DEFAULT_ROOT, - MB_RSS_PER_SOURCE_FILE, SOURCE_EXTENSIONS, check_layout, cumulative_batches, @@ -86,10 +85,9 @@ def build_parser(): "--extensions", default=None, help=( - "comma-separated file extensions to index, e.g. 'py,xml'. Fewer extensions means a " - "smaller graph and much less memory: codebase-memory-mcp holds the whole extraction in " - "RAM at roughly %s MB per file, so an Odoo instance costs ~14 GB with the default " - "%s and ~5 GB with 'py' alone" % (MB_RSS_PER_SOURCE_FILE, ",".join(SOURCE_EXTENSIONS)) + "comma-separated file extensions to index, e.g. 'py,xml' (default: %s). A narrower set " + "means a smaller, faster graph, but it is a weak lever on memory: the peak tracks the " + "nodes extracted, which come almost entirely from .py" % ",".join(SOURCE_EXTENSIONS) ), ) parser.add_argument( @@ -160,11 +158,7 @@ def main(argv=None): limits = resolve_limits(workers=args.workers, timeout_s=args.index_timeout) source_globs = globs_for(args.extensions.split(",") if args.extensions else None) - _logger.info( - "extensions=%s estimated_peak_rss_mb_per_file=%s", - ",".join(extensions_for(source_globs)), - MB_RSS_PER_SOURCE_FILE, - ) + _logger.info("extensions=%s", ",".join(extensions_for(source_globs))) for number, selected in enumerate(cumulative_batches(modules, args.batch_size), 1): _logger.info( diff --git a/src/codebase_memory_deployv/indexer.py b/src/codebase_memory_deployv/indexer.py index ac7b2bd..7b5755e 100644 --- a/src/codebase_memory_deployv/indexer.py +++ b/src/codebase_memory_deployv/indexer.py @@ -44,11 +44,14 @@ SOURCE_GLOBS = ("*.py", "*.xml", "*.js", "*.rst", "*.md", "*.css", "*.scss", "*.csv", "*.sql") # What .cbmignore lets through, so what the graph is expected to hold: ".py", ".xml", ... SOURCE_EXTENSIONS = tuple(sorted(pattern[1:] for pattern in SOURCE_GLOBS)) -# Measured on a real instance: codebase-memory-mcp holds the whole extraction in memory and -# costs about this much RSS per source file it indexes (5248 files -> 4880 MB). It is the -# number that decides whether a scope fits, so narrowing SOURCE_GLOBS is the one lever the -# caller has: dropping .js/.scss/.csv/.md/.rst/.css takes an Odoo instance from 15860 files -# to 6042, i.e. from roughly 14 GB down to 5 GB. +# Measured on a real Odoo instance: codebase-memory-mcp holds the whole extraction in memory, +# and that peak tracks the *nodes* it extracts, not the file count. A mixed scope of 5248 +# files reached 4880 MB RSS with 123023 nodes, i.e. about 40 KB per node. +# +# Narrowing the extensions is therefore a much weaker lever than the file counts suggest: +# .js/.scss/.csv/.md/.rst/.css are two thirds of the files of an Odoo instance (15860 -> 5681) +# but hardly any of the nodes, and indexing only .py still ran a 5 GiB container out of memory. +# What actually decides whether a scope fits is how much *Python* it contains. MB_RSS_PER_SOURCE_FILE = 0.93 @@ -62,6 +65,8 @@ def globs_for(extensions): def extensions_for(source_globs): """The extensions a .cbmignore built from source_globs lets through.""" return tuple(sorted(pattern[1:] for pattern in source_globs)) + + MANIFEST_NAMES = ("__manifest__.py", "__openerp__.py") PRUNE_DIRS = {".git", ".github", "__pycache__", "node_modules", ".cache", ".tx", "dist", "build", "setup"} CBMIGNORE_BACKUP_SUFFIX = ".before-codebase-memory-deployv"