From 1167972031d427be6773b86ec6dc9903317360d7 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 13 Sep 2026 14:51:00 +0200 Subject: [PATCH] fix(mcp): give the over-budget error a retry value, and say the peak is not the requirement v0.10.9 introduces Decision A: an over-budget index attempt fails whole with a named error instead of thrashing. The error told the caller to "Raise CBM_MEM_BUDGET_MB" but not to what, so the obvious next move is to read peak_rss_mb and retry just above it. That move fails, every time, by construction. The abort fires WHEN resident memory crosses the budget, so peak_rss_mb is pinned just above the budget -- it is where indexing was stopped, not what the repository needs. Measured on the linux kernel (94521 files, 43.1M LOC) while benching v0.10.9: default budget 24576 MB -> aborted, peak_rss_mb 25622 completing the same index actually took 31.75 GB So the reported peak understates the real requirement by 24%, and 1.32x the budget was needed. A caller retrying at peak+10% would have burned another 30 seconds to fail again with a nearly identical message. The response now carries suggested_budget_mb (1.5x the current budget, which would have cleared the kernel case) and the hint says plainly that peak_rss_mb is a floor, not a target. Computed as (3*b+1)/2 rather than b + b/2: integer division makes the latter degenerate to b at b == 1, so the suggestion would have repeated the budget that just failed -- the extended test caught exactly that. Not changed: the abort itself, the backpressure ladder, and worker cleanup. A clean test confirmed workers do NOT survive the abort (process count 2 before, 2 after, both idle at 0% CPU), so there was nothing to fix there. tests/test_mcp.c now pins both halves -- a concrete value strictly greater than the budget, and the hint stating the peak is where indexing STOPPED. Verified against the real pre-fix output captured during the bench, which contains neither. mcp suite 317 passed, 0 failed, 4 skipped; lint-ci clean. Signed-off-by: Martin Vogel --- src/mcp/mcp.c | 33 ++++++++++++++++++++++++++++----- tests/test_mcp.c | 10 ++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index f80d97c6e..e4d6e7eb2 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11228,11 +11228,34 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { yyjson_mut_obj_add_str(doc, root, "previous_index", "preserved"); yyjson_mut_obj_add_int(doc, root, "budget_mb", budget_mb); yyjson_mut_obj_add_int(doc, root, "peak_rss_mb", peak_rss_mb); - yyjson_mut_obj_add_str(doc, root, "hint", - "Indexing stopped: resident memory stayed above the budget after " - "backpressure; no partial graph was published and the previous " - "index still serves. Raise CBM_MEM_BUDGET_MB, lower CBM_WORKERS, " - "or exclude large subtrees."); + /* A CONCRETE retry value, because "raise CBM_MEM_BUDGET_MB" alone makes + * the caller guess — and the obvious guess is wrong. peak_rss_mb is + * where the run was STOPPED (it is pinned just above the budget by + * construction), not what the repo needs, so retrying at peak+10% fails + * again. Measured 2026-09-13 on the linux kernel: aborted at 25622 MB + * against a 24576 MB budget, but completing it actually took 31.75 GB — + * 1.32x the budget, 1.24x the reported peak. Suggest 1.5x the budget so + * the first retry has a real chance, and say plainly that the peak is a + * floor rather than a requirement. */ + /* (3*b+1)/2 rather than b + b/2: integer division makes the latter + * degenerate to b for b == 1, so the "suggestion" would repeat the + * budget that just failed. Rounding up keeps it strictly larger for + * every positive budget. */ + int suggested_budget_mb = budget_mb > 0 ? (budget_mb * 3 + 1) / 2 : 0; + char hint_text[CBM_SZ_512]; + (void)snprintf(hint_text, sizeof(hint_text), + "Indexing stopped: resident memory stayed above the budget after " + "backpressure; no partial graph was published and the previous index " + "still serves. peak_rss_mb is where indexing was STOPPED, not what this " + "repo needs — the real requirement is higher, so retrying just above the " + "peak will fail again. Retry with CBM_MEM_BUDGET_MB=%d (1.5x the current " + "budget) if the machine has the RAM, or lower CBM_WORKERS, or exclude " + "large subtrees.", + suggested_budget_mb); + if (suggested_budget_mb > 0) { + yyjson_mut_obj_add_int(doc, root, "suggested_budget_mb", suggested_budget_mb); + } + yyjson_mut_obj_add_strcpy(doc, root, "hint", hint_text); } else if (rc == CBM_PIPELINE_ABORT_PRESERVE_DB) { /* The truthful abort message (#2020): the old generic "check repo_path" * hint sent people debugging a path that was fine, when the run diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 95f669852..974aad10e 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -17773,8 +17773,15 @@ TEST(index_repository_over_budget_reports_named_reason) { bool reason_named = second_reason && strcmp(second_reason, "over_memory_budget") == 0; bool previous_preserved = second_previous && strcmp(second_previous, "preserved") == 0; bool hint_names_knob = second_hint && strstr(second_hint, "CBM_MEM_BUDGET_MB") != NULL; + /* The hint must also say that peak_rss_mb is NOT the requirement. The abort + * fires when RSS crosses the budget, so the peak is pinned just above it by + * construction; a caller who retries at peak+10% fails again. Measured on + * the linux kernel 2026-09-13: aborted at 25622 MB against a 24576 MB + * budget, but completing it took 31.75 GB. */ + bool hint_warns_peak_is_not_need = second_hint && strstr(second_hint, "STOPPED") != NULL; int budget_mb = budget_doc_int(second_doc, "budget_mb", -1); int peak_rss_mb = budget_doc_int(second_doc, "peak_rss_mb", -1); + int suggested_mb = budget_doc_int(second_doc, "suggested_budget_mb", -1); yyjson_doc_free(second_doc); free(second); long db_size_after = (long)cbm_file_size(db_path); @@ -17817,8 +17824,11 @@ TEST(index_repository_over_budget_reports_named_reason) { ASSERT_TRUE(reason_named); ASSERT_TRUE(previous_preserved); ASSERT_TRUE(hint_names_knob); + ASSERT_TRUE(hint_warns_peak_is_not_need); ASSERT_EQ(budget_mb, 1); ASSERT_GT(peak_rss_mb, budget_mb); + /* A CONCRETE retry value, not just the knob name: 1.5x the budget. */ + ASSERT_GT(suggested_mb, budget_mb); /* Preserved on disk and still served: same file size, same node count. */ ASSERT_GT(db_size_before, 0L); ASSERT_EQ(db_size_after, db_size_before);