mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-10-22 06:09:35 +08:00
fix: cbm_mem_rss() undercounts on Linux, blinding the ceiling/backpressure
Discovered while calibrating tests/mem_ceiling_abort.sh: a synthetic large-file index genuinely reached ~2.2-2.75GB RSS (confirmed via ps and mi_process_info's own peak_rss field, which read correctly), yet cbm_mem_rss() reported ~4MB at the exact same moments, so neither the new hard ceiling nor the PRE-EXISTING advisory budget/backpressure (pass_parallel.c:559, cbm_mem_over_budget) ever saw real memory pressure during concurrent large-file parsing. Root cause: vendored/mimalloc/src/prim/unix/prim.c's _mi_prim_process_info() never sets pinfo->current_rss on Linux — only peak_rss (via getrusage's ru_maxrss, a monotonic high-water mark). current_rss silently falls back to mimalloc's own internal committed-page counter, which this project deliberately tunes low (mi_option_arena_eager_commit=0 + purge_decommits=1 + purge_delay=0, set in cbm_mem_init to reduce upfront memory) — so on Linux, "current RSS" was never actually current RSS, it was a low-biased mimalloc-internal metric that happened to be nonzero often enough to defeat the existing `current_rss > 0` ASan-only fallback guard. Fix: on Linux, cbm_mem_rss() now prefers os_rss() (/proc/self/statm, already implemented, unaffected by mimalloc's internal accounting) as the PRIMARY source, falling back to mimalloc's value only if /proc is unavailable. macOS/Windows unaffected (mimalloc's primitives set current_rss correctly there via task_info/GetProcessMemoryInfo). Verified via a calibrated 40x8MB synthetic large-file index on IO: before the fix, a 2100MB forced ceiling never tripped despite RSS reaching 2.75GB; after the fix, the same fixture aborts cleanly at rss_mb=2135 against ceiling_mb=2100 with the expected diagnostic dump. Full ASan test suite re-run clean (0 FAIL) after the fix — the pre-existing test_incremental.c:302 RSS-delta flake (a live FastAPI clone that had grown past its 2048MB assumption) did not recur, consistent with more accurate RSS reporting also improving the existing backpressure's effectiveness. tests/mem_ceiling_abort.sh: fixed two harness bugs found while calibrating this — (1) `mode=full` index_repository does not force a re-parse when file hashes are unchanged from a prior run against the same repo_path (routes to incremental.noop), so every run needing a genuine full extraction now resets the target's cached .db first; (2) the forced test ceiling is now calibrated against a directly-probed peak RSS for the exact fixture shape (2100MB, between the 2048MB floor and an observed ~2.2-2.75GB peak) instead of an unverified guess.
This commit is contained in:
Peter Cox
+29
-1
@@ -146,13 +146,41 @@ void cbm_mem_init(double ram_fraction) {
|
||||
}
|
||||
|
||||
size_t cbm_mem_rss(void) {
|
||||
#if defined(__linux__) && !defined(_WIN32)
|
||||
/* Linux: mimalloc's _mi_prim_process_info() (vendored/mimalloc/src/
|
||||
* prim/unix/prim.c) never sets pinfo->current_rss on Linux — only
|
||||
* peak_rss (via getrusage's ru_maxrss, a high-water mark). current_rss
|
||||
* silently falls back to mimalloc's OWN committed-page counter
|
||||
* (mi_process_info()'s pinfo.current_commit default), which this
|
||||
* project deliberately tunes low via mi_option_arena_eager_commit=0 +
|
||||
* mi_option_purge_decommits=1 + mi_option_purge_delay=0 (cbm_mem_init,
|
||||
* above) to reduce upfront memory. Combined, "current_rss" on Linux
|
||||
* can read near-zero (observed: 4MB) while true RSS is multiple GB —
|
||||
* a small-but-NONZERO value that defeated the `current_rss > 0`
|
||||
* ASan-only fallback guard below, silently blinding both this
|
||||
* function's callers (cbm_mem_over_budget backpressure AND the
|
||||
* enforcing ceiling in this same file) to real memory pressure during
|
||||
* concurrent large-file parsing — the exact mechanism the 2026-07-01
|
||||
* incident diagnosed. os_rss() (/proc/self/statm) is unaffected by
|
||||
* mimalloc's internal accounting and is authoritative OS-reported RSS
|
||||
* on every Linux build regardless of allocator tuning, so it is the
|
||||
* PRIMARY source here, not a last-resort fallback. Verified via a
|
||||
* calibrated 40x8MB synthetic large-file index on IO: mi_process_info
|
||||
* current_rss=4MB, actual RSS (ps/proc)=~2.2GB, os_rss()=~2.2GB. */
|
||||
size_t rss = os_rss();
|
||||
if (rss > 0) {
|
||||
return rss;
|
||||
}
|
||||
/* Extremely unlikely (/proc unavailable) — fall through to mimalloc. */
|
||||
#endif
|
||||
size_t current_rss = 0;
|
||||
size_t peak_rss = 0;
|
||||
mi_process_info(NULL, NULL, NULL, ¤t_rss, &peak_rss, NULL, NULL, NULL);
|
||||
if (current_rss > 0) {
|
||||
return current_rss;
|
||||
}
|
||||
/* Fallback for ASan builds (MI_OVERRIDE=0) */
|
||||
/* Fallback for ASan builds (MI_OVERRIDE=0) and any other platform
|
||||
* where mimalloc's current_rss is unavailable/zero. */
|
||||
return os_rss();
|
||||
}
|
||||
|
||||
|
||||
+74
-41
@@ -13,9 +13,10 @@
|
||||
# Row 7 — A normal full index of the real repo does NOT abort (healthy
|
||||
# path preserved; anti-false-positive guard for R3).
|
||||
# Row 9 — CBM_MEM_CEILING_MB override adjusts the abort threshold: forced
|
||||
# low, the HEALTHY real-repo index now aborts (proves the knob
|
||||
# bites); unset uses the default and completes; invalid warns and
|
||||
# falls back to the default (completes).
|
||||
# below a large synthetic index's actual RSS, that index now
|
||||
# aborts (proves the knob bites); unset uses the default and
|
||||
# completes; invalid warns and falls back to the default
|
||||
# (completes).
|
||||
# Row 10 (real-environment half) — both the abort and no-abort verdicts
|
||||
# are read against the REAL RSS of a real process, not an
|
||||
# injected value.
|
||||
@@ -26,6 +27,15 @@
|
||||
# abort during extract precedes any DB write. This script asserts no .db
|
||||
# file (partial or otherwise) is left behind by an aborted run.
|
||||
#
|
||||
# IMPORTANT — incremental indexing gotcha: `index_repository` with
|
||||
# mode=full does NOT force a full re-parse when file hashes are unchanged
|
||||
# from a prior run against the same repo_path (it routes to
|
||||
# incremental.noop). Every run below that needs a genuine full extraction
|
||||
# pass (i.e. any run whose PURPOSE is to exercise cbm_max_file_bytes() /
|
||||
# cbm_mem_abort_if_over_ceiling() on real file reads) deletes the target's
|
||||
# .db first. Skipping that reset is a silent false-pass, not a crash — the
|
||||
# process still exits 0 (it did no real work), so watch for that pattern.
|
||||
#
|
||||
# Usage: bash tests/mem_ceiling_abort.sh
|
||||
# Exit 0 on success, non-zero on failure. SLOW (generates GB-scale fixtures) —
|
||||
# intended for the IO build host, not routine `make test`.
|
||||
@@ -51,6 +61,19 @@ fi
|
||||
|
||||
project_name() { printf '%s' "$1" | sed 's#^/##; s#[^A-Za-z0-9._-]#-#g'; }
|
||||
|
||||
db_path_for() {
|
||||
local repo="$1"
|
||||
printf '%s/%s.db' "$CACHE_DIR" "$(project_name "$repo")"
|
||||
}
|
||||
|
||||
# Force the NEXT index_repository call against $1 to do a genuine full
|
||||
# extraction (not an incremental no-op) by wiping its cached .db/.tmp.
|
||||
reset_index_state() {
|
||||
local repo="$1" db
|
||||
db="$(db_path_for "$repo")"
|
||||
rm -f "$db" "${db}.tmp"
|
||||
}
|
||||
|
||||
# Run `cli index_repository` against $1, with env overrides from the
|
||||
# remaining args (NAME=VALUE pairs), capturing exit code + stderr.
|
||||
# Sets: LAST_EXIT, LAST_LOG (path).
|
||||
@@ -64,13 +87,9 @@ run_index() {
|
||||
LAST_EXIT=$?
|
||||
}
|
||||
|
||||
db_path_for() {
|
||||
local repo="$1"
|
||||
printf '%s/%s.db' "$CACHE_DIR" "$(project_name "$repo")"
|
||||
}
|
||||
|
||||
# ── Row 7 + row 10 (no-abort half): healthy real-repo index, default ceiling ──
|
||||
echo "[mem_ceiling] Row 7/10: healthy real-repo index must NOT abort (default ceiling)..." >&2
|
||||
reset_index_state "$ROOT"
|
||||
run_index "$ROOT"
|
||||
if [[ "$LAST_EXIT" -ne 0 ]]; then
|
||||
echo "[mem_ceiling] FAIL [row7]: healthy real-repo index aborted (exit=$LAST_EXIT) under default ceiling" >&2
|
||||
@@ -83,6 +102,7 @@ rm -f "$LAST_LOG"
|
||||
|
||||
# ── Row 9 (unset leg): explicit unset also completes ─────────────────────
|
||||
echo "[mem_ceiling] Row 9 (unset): explicit CBM_MEM_CEILING_MB unset uses default, completes..." >&2
|
||||
reset_index_state "$ROOT"
|
||||
run_index "$ROOT"
|
||||
if [[ "$LAST_EXIT" -ne 0 ]]; then
|
||||
echo "[mem_ceiling] FAIL [row9-unset]: index aborted with ceiling env unset (exit=$LAST_EXIT)" >&2
|
||||
@@ -94,6 +114,7 @@ rm -f "$LAST_LOG"
|
||||
|
||||
# ── Row 9 (invalid leg): non-numeric override warns + falls back, completes ──
|
||||
echo "[mem_ceiling] Row 9 (invalid): non-numeric CBM_MEM_CEILING_MB falls back to default..." >&2
|
||||
reset_index_state "$ROOT"
|
||||
run_index "$ROOT" CBM_MEM_CEILING_MB="not-a-number"
|
||||
if [[ "$LAST_EXIT" -ne 0 ]]; then
|
||||
echo "[mem_ceiling] FAIL [row9-invalid]: index aborted with invalid ceiling env (exit=$LAST_EXIT)" >&2
|
||||
@@ -108,56 +129,68 @@ else
|
||||
fi
|
||||
rm -f "$LAST_LOG"
|
||||
|
||||
# ── Row 9 (low override leg) + Row 6 + R4: force ceiling low, healthy repo now aborts ──
|
||||
echo "[mem_ceiling] Row 6/9(low)/R4: CBM_MEM_CEILING_MB forced to the floor (2048MB) — healthy repo must now abort with a diagnostic dump..." >&2
|
||||
db_before="$(db_path_for "$ROOT")"
|
||||
rm -f "$db_before" "${db_before}.tmp"
|
||||
run_index "$ROOT" CBM_MEM_CEILING_MB="2048"
|
||||
# ── Row 6 + Row 9(low) + R4: synthetic large-file repro forces cumulative
|
||||
# RSS past a ceiling set BELOW its expected peak but ABOVE the 2048MB
|
||||
# floor (mem_ceiling_floor_applies_on_tiny_ram in test_mem.c already
|
||||
# pins the floor's own arithmetic in-process; this proves the knob bites
|
||||
# on a REAL run — a ceiling below the floor would just get clamped up
|
||||
# to the floor and never trip, so the fixture must genuinely exceed the
|
||||
# CHOSEN ceiling, not the floor). Sized well past what a healthy 587-file
|
||||
# real repo would ever reach (~1-1.3GB peak per the diagnosis), so a
|
||||
# false negative here is a genuine "doesn't bite" signal, not sizing
|
||||
# noise. ──────────────────────────────────────────────────────────────
|
||||
CEILING_TEST_MB=2100
|
||||
echo "[mem_ceiling] Row 6/9(low)/R4: generating a large-file repro sized to exceed a ${CEILING_TEST_MB}MB forced ceiling..." >&2
|
||||
WORKDIR="$(mktemp -d /tmp/cbm_mem_ceiling_repro.XXXXXX)"
|
||||
# 40 tiny + 40 large (~8MB each = ~320MB source) — mirrors the ORIGINAL
|
||||
# diagnosis repro shape (600 tiny + 40 large ~7.6MB files -> 8.8GB on IO)
|
||||
# on file COUNT and per-file size (tiny-file count trimmed 600->40; tiny
|
||||
# files barely contribute RSS, so this doesn't change the mechanism).
|
||||
# CALIBRATED by direct probe on IO/CBM_WORKERS=8 with NO ceiling override:
|
||||
# this exact shape peaks at rss_mb=2198 (parallel.extract.mem log). The
|
||||
# floor (CBM_MEM_CEILING_FLOOR_MB, mem.c) is 2048MB — a ceiling below that
|
||||
# gets clamped UP to the floor and would never trip (this is what silently
|
||||
# broke the first two attempts at 3000MB, and an earlier 20x15MB/3000MB
|
||||
# combination that just didn't reach 3000MB at all). 2100MB sits strictly
|
||||
# between the floor (2048) and the calibrated peak (2198), so it is NOT
|
||||
# floor-clamped and WILL trip against this exact fixture shape on this
|
||||
# exact host. Re-calibrate CEILING_TEST_MB (or the gen call) if run on a
|
||||
# host with materially different allocator/CPU behaviour.
|
||||
bash "$GEN" "$WORKDIR" 40 40 8
|
||||
reset_index_state "$WORKDIR"
|
||||
run_index "$WORKDIR" CBM_MEM_CEILING_MB="$CEILING_TEST_MB" CBM_WORKERS="8"
|
||||
db_synth="$(db_path_for "$WORKDIR")"
|
||||
if [[ "$LAST_EXIT" -eq 0 ]]; then
|
||||
echo "[mem_ceiling] FAIL [row6/row9-low]: index did NOT abort with ceiling forced to the floor (proves the knob doesn't bite, or the real repo genuinely never nears 2GB on this host — re-run with a smaller floor override or bigger synthetic fixture below)" >&2
|
||||
echo "[mem_ceiling] FAIL [row6/row9-low]: synthetic fixture did NOT trip a ${CEILING_TEST_MB}MB forced ceiling — either the knob doesn't bite, or the fixture is undersized for this host's allocator behaviour (IO: 31GB/8 cores). Widen n_large/size_mb in the gen call above if this needs to be load-bearing on a beefier host." >&2
|
||||
FAILURES=$((FAILURES + 1))
|
||||
else
|
||||
echo "[mem_ceiling] index aborted as expected (exit=$LAST_EXIT)" >&2
|
||||
if ! grep -q "mem.ceiling.abort" "$LAST_LOG"; then
|
||||
echo "[mem_ceiling] FAIL [row6]: no mem.ceiling.abort diagnostic dump in output" >&2
|
||||
tail -10 "$LAST_LOG" >&2
|
||||
FAILURES=$((FAILURES + 1))
|
||||
elif ! grep -qE "file=\S+ phase=\S+ rss_mb=[0-9]+ ceiling_mb=[0-9]+" "$LAST_LOG"; then
|
||||
echo "[mem_ceiling] FAIL [row6]: diagnostic dump missing file/phase/rss_mb/ceiling_mb fields" >&2
|
||||
tail -5 "$LAST_LOG" >&2
|
||||
grep "mem.ceiling.abort" "$LAST_LOG" >&2
|
||||
FAILURES=$((FAILURES + 1))
|
||||
else
|
||||
echo "[mem_ceiling] PASS [row6]: diagnostic dump present with file+phase+rss" >&2
|
||||
grep "mem.ceiling.abort" "$LAST_LOG" >&2
|
||||
fi
|
||||
fi
|
||||
# R4: an aborted run must leave no (partial) .db file — the abort happens
|
||||
# strictly before dump_and_persist_hashes()/cbm_gbuf_dump_to_sqlite().
|
||||
if [[ -f "$db_before" || -f "${db_before}.tmp" ]]; then
|
||||
echo "[mem_ceiling] FAIL [R4]: aborted run left a .db (or .db.tmp) file — store-integrity violation: $db_before" >&2
|
||||
FAILURES=$((FAILURES + 1))
|
||||
else
|
||||
echo "[mem_ceiling] PASS [R4]: aborted run left no .db file (extract-phase abort precedes any SQLite write)" >&2
|
||||
fi
|
||||
rm -f "$LAST_LOG" "$db_before" "${db_before}.tmp"
|
||||
|
||||
# ── Row 6 (synthetic large-file variant): generate a fixture that exceeds
|
||||
# a forced-low ceiling purely from cumulative parse RSS, independent of
|
||||
# whether the real repo happens to be big enough on this host ─────────
|
||||
echo "[mem_ceiling] Row 6 (synthetic): generating large-file repro to force the ceiling from cumulative RSS..." >&2
|
||||
WORKDIR="$(mktemp -d /tmp/cbm_mem_ceiling_repro.XXXXXX)"
|
||||
bash "$GEN" "$WORKDIR" 20 8 8 # 20 tiny + 8 large (~8MB each = ~64MB source; parse working
|
||||
# sets multiply this several-fold across concurrent workers)
|
||||
db_synth="$(db_path_for "$WORKDIR")"
|
||||
rm -f "$db_synth" "${db_synth}.tmp"
|
||||
run_index "$WORKDIR" CBM_MEM_CEILING_MB="2048" CBM_WORKERS="8"
|
||||
if [[ "$LAST_EXIT" -eq 0 ]]; then
|
||||
echo "[mem_ceiling] WARN [row6-synth]: synthetic fixture did not trip the forced ceiling on this host (IO's 31GB may absorb 8x8MB files even at 8 workers) — not counted as a failure since the real-repo variant above already proved the abort path; widen n_large/size_mb if this needs to be load-bearing" >&2
|
||||
else
|
||||
if grep -q "mem.ceiling.abort" "$LAST_LOG"; then
|
||||
echo "[mem_ceiling] PASS [row6-synth]: synthetic large-file repro tripped the ceiling with a diagnostic dump" >&2
|
||||
else
|
||||
echo "[mem_ceiling] FAIL [row6-synth]: synthetic fixture aborted (exit=$LAST_EXIT) but with no diagnostic dump" >&2
|
||||
# strictly before dump_and_persist_hashes()/cbm_gbuf_dump_to_sqlite(). Only
|
||||
# meaningful when the run actually aborted (LAST_EXIT != 0) — a completed
|
||||
# run legitimately leaves a .db behind, that is not a violation.
|
||||
if [[ "$LAST_EXIT" -ne 0 ]]; then
|
||||
if [[ -f "$db_synth" || -f "${db_synth}.tmp" ]]; then
|
||||
echo "[mem_ceiling] FAIL [R4]: aborted run left a .db (or .db.tmp) file — store-integrity violation: $db_synth" >&2
|
||||
FAILURES=$((FAILURES + 1))
|
||||
else
|
||||
echo "[mem_ceiling] PASS [R4]: aborted run left no .db file (extract-phase abort precedes any SQLite write)" >&2
|
||||
fi
|
||||
else
|
||||
echo "[mem_ceiling] SKIP [R4]: run did not abort, nothing to check (see row6/row9-low failure above)" >&2
|
||||
fi
|
||||
rm -f "$LAST_LOG" "$db_synth" "${db_synth}.tmp"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user